Feat/tcp connecotr hardening#179
Merged
Merged
Conversation
…rt and connection handling improvements
thaodt
approved these changes
Jul 12, 2026
thaodt
left a comment
thaodt
left a comment
Collaborator
There was a problem hiding this comment.
Approved with some small changes then we're good to go.
…prove time scaling in tests
…atchdog The loopback harness polls two non-terminating embassy-net stacks in the background, so a transport regression or a dropped crossover packet would leave `block_on` pending until the outer CI timeout instead of failing the test. Race the foreground/background `select` against a 20s wall-clock watchdog that re-arms its waker each poll (so the deadline is observed even when the stacks would otherwise park) and panics with a clear message. Addresses review feedback on #179. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed N=2 path The Embassy loopback concurrency test used two unrelated `TcpListener<1>` on different ports, so it never touched `with_buffers` construction or same-port fan-out. Rework it to stand up one `TcpListener::<2>::with_buffers(...)` on a single port and dial it with two clients. Reaching the N>1 pool from an integration test required a public accept: the `Listener` impl is `N=1`-only and the pooled path (`into_server_futures`/ `serve_socket_slot`) was private, reachable only through `TcpServer<N>` + the AimX session engine. Add `TcpListener::<N>::accept_on(index)` and factor the accept body — previously duplicated between the `N=1` `Listener` impl and the server workers — into a shared `accept_on_slot`. Side benefit: `with_buffers` can now be driven directly instead of only through `TcpServer`. Addresses review feedback on #179. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thaodt
requested changes
Jul 14, 2026
Contributor
Author
|
Your are right, but I think this change will lead to a broader api change. Let me check and come back! |
…ng in accept logic
Contributor
Author
|
Rather than reshaping the public API, I opted to feature-gate I also kept the transport-level fan-out test rather than replacing it with a full |
Collaborator
|
LGTM. Just need to update your branch and can merge bro :P |
thaodt
approved these changes
Jul 15, 2026
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
2 participants
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.
Changes
accept()errors.embassy-net'sTcpSocket::accept()can fail synchronously (a port-0local_endpointrejected asInvalidPort), and theErrarms ofserve_socket_slotand theListener<1>compat path had no await point — a tight loop with zero yields that starves the cooperative executor. Both nowembassy_futures::yield_now().await, degrading a static misconfig to a debuggable warn-loop. New optionalembassy-futuresdep._test-embassy-loopback). The socket pool is welded to a concreteembassy_net::tcp::TcpSocket, so its recycling and waker handoff were review-only. Now exercised over two realembassy-netstacks wired by an in-memory driver-channel crossover: recycle → re-accept, concurrent accept slots, and dialer redial after a failed connect / dropped link. Uses a wall-clock host time driver (the frozen stub clock stalls the delayed-ACK timer and hangs the firstflush()). Kept offembassy-runtimeand out of[dev-dependencies]; wired intomake check.TcpSocketSlotSAFETY comment (dialer/worker/listener-owned, not just "dialer-owned") and documented the intentional double-abort()on the recycle path.aimdb-tcp-connector/CHANGELOG.md(initial) + global changelog index link.