Skip to content

Commit 337d202

Browse files
testclaude
andcommitted
feat(tcp-connector): expose TcpListener::accept_on and cover the pooled 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>
1 parent 83df852 commit 337d202

2 files changed

Lines changed: 97 additions & 52 deletions

File tree

aimdb-tcp-connector/src/embassy_transport.rs

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,23 @@ impl<const N: usize> TcpListener<N> {
292292
}
293293
}
294294

295+
/// Accept one connection on pooled socket `index`, recycling that socket
296+
/// back into its slot when the returned connection drops.
297+
///
298+
/// The [`Listener`] impl (`N = 1`) and the `TcpServer` workers each accept a
299+
/// single slot at a time; this exposes the same per-slot accept so a caller
300+
/// can keep several pooled sockets in `accept()` on one endpoint at once
301+
/// (the concurrent-listener property `with_buffers` exists for). Panics if
302+
/// `index >= N`.
303+
pub fn accept_on(
304+
&self,
305+
index: usize,
306+
) -> impl Future<Output = TransportResult<Box<dyn Connection>>> + Send + '_ {
307+
let slot = self.slots[index].clone();
308+
let local_endpoint = self.local_endpoint;
309+
SendFutureWrapper(async move { accept_on_slot(&slot, local_endpoint).await })
310+
}
311+
295312
fn into_server_futures(
296313
self,
297314
codec: Arc<AimxCodec>,
@@ -316,25 +333,33 @@ impl<const N: usize> TcpListener<N> {
316333

317334
impl Listener for TcpListener<1> {
318335
fn accept(&mut self) -> BoxFut<'_, TransportResult<Box<dyn Connection>>> {
319-
Box::pin(SendFutureWrapper(async move {
320-
let slot = &self.slots[0];
321-
let mut socket = poll_fn(|cx| slot.poll_take(cx)).await;
336+
Box::pin(self.accept_on(0))
337+
}
338+
}
339+
340+
/// Take the socket from `slot`, accept one inbound connection on it, and hand
341+
/// back a recyclable [`TcpConnection`]. Shared by [`TcpListener::accept_on`] and
342+
/// the per-slot server workers so all pooled accept paths behave identically.
343+
async fn accept_on_slot(
344+
slot: &Arc<TcpSocketSlot>,
345+
local_endpoint: IpListenEndpoint,
346+
) -> TransportResult<Box<dyn Connection>> {
347+
let mut socket = poll_fn(|cx| slot.poll_take(cx)).await;
348+
socket.abort();
349+
match socket.accept(local_endpoint).await {
350+
Ok(()) => {
351+
Ok(Box::new(TcpConnection::reusable(socket, slot.clone())) as Box<dyn Connection>)
352+
}
353+
Err(_) => {
322354
socket.abort();
323-
match socket.accept(self.local_endpoint).await {
324-
Ok(()) => {
325-
Ok(Box::new(TcpConnection::reusable(socket, slot.clone()))
326-
as Box<dyn Connection>)
327-
}
328-
Err(_) => {
329-
socket.abort();
330-
slot.put(socket);
331-
// Same synchronous-failure guard as `serve_socket_slot`:
332-
// `serve()` re-enters `accept()` immediately on `Err`, so yield.
333-
yield_now().await;
334-
Err(TransportError::Io)
335-
}
336-
}
337-
}))
355+
slot.put(socket);
356+
// `accept()` can fail synchronously (e.g. port-0 `InvalidPort`);
357+
// without this await the caller re-enters `accept()` immediately with
358+
// no yield point, starving the executor. Yield so a misconfig
359+
// warn-loops instead of hanging.
360+
yield_now().await;
361+
Err(TransportError::Io)
362+
}
338363
}
339364
}
340365

@@ -346,23 +371,10 @@ async fn serve_socket_slot(
346371
config: SessionConfig,
347372
) {
348373
loop {
349-
let mut socket = poll_fn(|cx| slot.poll_take(cx)).await;
350-
socket.abort();
351-
match socket.accept(local_endpoint).await {
352-
Ok(()) => {
353-
let conn =
354-
Box::new(TcpConnection::reusable(socket, slot.clone())) as Box<dyn Connection>;
355-
run_session(conn, codec.as_ref(), dispatch.as_ref(), &config).await;
356-
}
357-
Err(_) => {
358-
socket.abort();
359-
slot.put(socket);
360-
// `accept()` can fail synchronously (e.g. port-0 `InvalidPort`);
361-
// without this await the loop retakes the slot and retries with no
362-
// yield point, starving the executor. Yield so a misconfig
363-
// warn-loops instead of hanging.
364-
yield_now().await;
365-
}
374+
// `accept_on_slot` already yields on the synchronous-failure path, so a
375+
// misconfig warn-loops here instead of starving the executor.
376+
if let Ok(conn) = accept_on_slot(&slot, local_endpoint).await {
377+
run_session(conn, codec.as_ref(), dispatch.as_ref(), &config).await;
366378
}
367379
}
368380
}

aimdb-tcp-connector/tests/embassy_loopback.rs

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
//! `TcpListener`/`TcpDialer`/`TcpConnection` triple under `block_on`:
88
//!
99
//! - recycle: accept -> exchange -> drop -> re-accept (`recycle_then_reaccept`);
10-
//! - concurrency: two sessions, each with its own accept slot, at once
11-
//! (`two_concurrent_sessions`);
10+
//! - concurrency: one pooled `N = 2` listener keeps both sockets in `accept()`
11+
//! on a single port while two clients dial it (`two_concurrent_sessions`);
1212
//! - redial: after a failed connect and after a dropped link
1313
//! (`dialer_redials_after_failure_and_drop`).
1414
#![cfg(feature = "_test-embassy-loopback")]
@@ -201,6 +201,30 @@ async fn exchange(server: &mut dyn Connection, client: &mut dyn Connection, tag:
201201
assert_eq!(got, b"pong", "reply framing round-trip");
202202
}
203203

204+
/// Receive one frame and echo it straight back. Pairing-agnostic: with same-port
205+
/// fan-out the stack, not the test, decides which pooled server socket a given
206+
/// client lands on, so the server can only echo whatever frame arrives on it.
207+
async fn echo_once(server: &mut dyn Connection) {
208+
let got = server
209+
.recv()
210+
.await
211+
.expect("server recv")
212+
.expect("server frame");
213+
server.send(&got).await.expect("server echo");
214+
}
215+
216+
/// Send `tag`, then assert the echo comes back byte-for-byte — so each client
217+
/// verifies its own session regardless of which server socket served it.
218+
async fn send_and_verify(client: &mut dyn Connection, tag: &[u8]) {
219+
client.send(tag).await.expect("client send");
220+
let got = client
221+
.recv()
222+
.await
223+
.expect("client recv")
224+
.expect("client frame");
225+
assert_eq!(got, tag, "echo round-trip");
226+
}
227+
204228
/// accept -> exchange -> disconnect -> socket recycled -> re-accept works.
205229
#[test]
206230
fn recycle_then_reaccept() {
@@ -228,33 +252,42 @@ fn recycle_then_reaccept() {
228252
});
229253
}
230254

231-
/// Two client/server sessions, each with its own accept slot, live at once — the
232-
/// concurrent-accept-slots property the pool exists for. (The single-port
233-
/// `TcpListener<N>` fan-out runs the AimX session engine, covered elsewhere.)
255+
/// One pooled `N = 2` listener keeps both of its sockets in `accept()` on a
256+
/// single port while two clients dial that port at once — the same-port fan-out
257+
/// and pooled worker creation that `TcpListener::<N>::with_buffers` exists for.
258+
/// Each client lands on a distinct pooled slot; a broken pool (only one socket
259+
/// accepting, or both racing to the same slot) would hang the second session and
260+
/// trip the watchdog. (Wiring the pool into the AimX session engine via
261+
/// `TcpServer<N>` is covered elsewhere; this stays at the transport layer.)
234262
#[test]
235263
fn two_concurrent_sessions() {
236264
drive(|server_stack, client_stack| async move {
237-
let mut listener_a = TcpListener::new(server_stack, 7001u16, buf(), buf());
238-
let mut listener_b = TcpListener::new(server_stack, 7002u16, buf(), buf());
265+
// One pooled listener, two sockets, both bound to port 7001.
266+
let listener =
267+
TcpListener::<2>::with_buffers(server_stack, 7001u16, [buf(), buf()], [buf(), buf()]);
239268
let dialer_a = TcpDialer::new(client_stack, endpoint(7001), buf(), buf());
240-
let dialer_b = TcpDialer::new(client_stack, endpoint(7002), buf(), buf());
269+
let dialer_b = TcpDialer::new(client_stack, endpoint(7001), buf(), buf());
241270

242-
// Bring both sessions up concurrently, then exchange on both while both
243-
// are still open.
244-
let (a_srv, a_cli, b_srv, b_cli) = futures::join!(
245-
listener_a.accept(),
271+
// Both pooled sockets accept on 7001 while both clients dial it; each
272+
// client establishes against a separate slot.
273+
let (a_srv, b_srv, a_cli, b_cli) = futures::join!(
274+
listener.accept_on(0),
275+
listener.accept_on(1),
246276
dialer_a.connect(),
247-
listener_b.accept(),
248277
dialer_b.connect(),
249278
);
250-
let mut a_srv = a_srv.expect("accept A");
279+
let mut a_srv = a_srv.expect("accept slot 0");
280+
let mut b_srv = b_srv.expect("accept slot 1");
251281
let mut a_cli = a_cli.expect("connect A");
252-
let mut b_srv = b_srv.expect("accept B");
253282
let mut b_cli = b_cli.expect("connect B");
254283

284+
// Drive both sessions at once. Servers echo (the stack picks the pairing);
285+
// each client verifies its own tag returns.
255286
futures::join!(
256-
exchange(a_srv.as_mut(), a_cli.as_mut(), b"aaa"),
257-
exchange(b_srv.as_mut(), b_cli.as_mut(), b"bbb"),
287+
echo_once(a_srv.as_mut()),
288+
echo_once(b_srv.as_mut()),
289+
send_and_verify(a_cli.as_mut(), b"aaa"),
290+
send_and_verify(b_cli.as_mut(), b"bbb"),
258291
);
259292
});
260293
}

0 commit comments

Comments
 (0)