fix(p2p): harden transport availability - #71
Open
trvon wants to merge 1 commit into
Open
Conversation
Signed-off-by: trvon <git@trevon.dev>
Deploying yams with
|
| Latest commit: |
8e602fa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://98fba8e5.yams.pages.dev |
| Branch Preview URL: | https://fix-p2p-transport-availabili.yams.pages.dev |
There was a problem hiding this comment.
🟡 Changes recommended
The current YAMS_DAEMON_TEST_HOOKS_IMPL scoping in p2p_manager.cpp can produce inconsistent header views within the same program (ODR/UB risk) and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Hardens the direct-P2P transport against availability/lifecycle issues by separating unauthenticated handshake capacity from authenticated sessions, enforcing an aggregate session I/O deadline, and containing thread-boundary exceptions in listener and reconnect loops.
Changes:
- Add hard caps for handshake/session concurrency plus bounded operation/session timeout validation.
- Enforce a single aggregate session deadline across TLS handshake and framed I/O; serialize socket cancellation onto the owning I/O context.
- Expand Catch2 coverage for capacity separation, deadline behavior, and listener/manager lifecycle edge cases; update docs.
File summaries
| File | Description |
|---|---|
| tests/unit/daemon/p2p_transport_catch2_test.cpp | Adds tests for handshake vs session capacity, aggregate deadline enforcement, and listener shutdown/lifecycle behaviors. |
| tests/unit/daemon/p2p_manager_catch2_test.cpp | Adds tests for reconnect interval ceiling and reconnect-thread exception/stop serialization (with hooks). |
| src/daemon/p2p/p2p_transport.cpp | Implements handshake/session pool separation, aggregate deadlines, cancellation serialization, and exception containment; adds test hooks. |
| src/daemon/p2p/p2p_manager.cpp | Threads session timeout into transport usage, caps reconnect interval, contains reconnect-loop exceptions; adds test hook support. |
| include/yams/daemon/p2p/p2p_transport.h | Introduces new timeout/capacity constants and options, exposes session counters and test-hook entry points. |
| include/yams/daemon/p2p/p2p_manager.h | Adds reconnect ceiling constant, sessionTimeout option, and reconnect-loop test hook entry point. |
| docs/p2p.md | Documents the new transport capacity pools, aggregate deadline semantics, and lifecycle hardening. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+509
to
+512
| retry.nextAttempt = | ||
| now + std::min(delay + jitter, | ||
| std::chrono::duration_cast<std::chrono::milliseconds>( | ||
| std::chrono::minutes(5))); |
Comment on lines
+47
to
+53
| bool waitUntil(std::function<bool()> predicate, std::chrono::milliseconds timeout = 2s) { | ||
| const auto deadline = std::chrono::steady_clock::now() + timeout; | ||
| while (!predicate() && std::chrono::steady_clock::now() < deadline) { | ||
| std::this_thread::yield(); | ||
| } | ||
| return predicate(); | ||
| } |
Comment on lines
+4
to
+11
| #define YAMS_DAEMON_TEST_HOOKS_IMPL 1 | ||
| // pi-lens-ignore: fatal error | ||
| #include <yams/daemon/p2p/p2p_manager.h> | ||
| #undef YAMS_DAEMON_TEST_HOOKS_IMPL | ||
|
|
||
| // pi-lens-ignore: fatal error | ||
| #include <yams/daemon/p2p/p2p_delta.h> | ||
| #include <yams/daemon/p2p/p2p_transport.h> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked draft child of #70. This change closes the disclosed direct-P2P transport availability and socket-lifecycle blockers without merging or deploying anything.
Handler-local computation between I/O calls remains cooperative and must preserve its own operation bounds. The transport does not claim to forcibly terminate arbitrary C++ handler work.
Before the fix:
vector::push_backcould terminate under allocation failureKnown stack blockers
This draft child is not independently mergeable or deployable. The umbrella still has separately tracked bootstrap-trust defaults, replicated-apply serialization, operator-status semantics, fuzzing, and final live gates.
AI disclosure
Extensive AI assistance was used for implementation, test generation, and adversarial review. All changes were locally inspected and validated; remaining stack blockers are disclosed above.
Stack
fix/p2p-bounded-catchup/ fix(p2p): resume bounded history catch-up #70fix/p2p-transport-availabilityat8e602fa96f58bc5edeb6bd434fbf83dcd8901180Do not merge or deploy this child independently.