From e207f2ba8dd41c2fa5de034764587b72cd2b030c Mon Sep 17 00:00:00 2001 From: mp0rta <3p0rta26@gmail.com> Date: Wed, 19 Aug 2026 11:16:58 +0900 Subject: [PATCH] fix(noq): resolve pending OpenPath on connection termination A pending OpenPath was never resolved on connection close or death: terminate() notified every other waiter class but did not drain the open_path watch senders, so the future hung forever and its ConnectionRef kept the connection state alive. Drain pending senders with ValidationFailed, matching the semantics used when PathEvent::Abandoned is received for a path that has not opened yet. Add a regression test that closes the connection while path validation is pending. --- noq/src/connection.rs | 9 ++++++ noq/src/tests.rs | 71 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/noq/src/connection.rs b/noq/src/connection.rs index fd1c909555..0fa97a08fc 100644 --- a/noq/src/connection.rs +++ b/noq/src/connection.rs @@ -1805,6 +1805,15 @@ impl State { } shared.handshake_confirmed.notify_waiters(); wake_all_notify(&mut self.stopped); + // Resolve pending `OpenPath` futures: a path that never finished + // validation cannot be established anymore, which semantically equals + // `ValidationFailed` (see the `PathEvent::Abandoned` handling in + // `forward_app_events`). Without this the futures would never + // resolve, keeping the connection state alive through their + // `ConnectionRef`s. + for (_, sender) in self.open_path.drain() { + sender.send_modify(|value| *value = Err(PathError::ValidationFailed)); + } shared.closed.notify_waiters(); // Send to the registered on_closed futures. if !self.on_closed.is_empty() { diff --git a/noq/src/tests.rs b/noq/src/tests.rs index adcd75682e..926e8461af 100755 --- a/noq/src/tests.rs +++ b/noq/src/tests.rs @@ -1557,6 +1557,77 @@ async fn close_path() -> TestResult { Ok(()) } +/// A pending [`OpenPath`] must resolve when the connection is closed; +/// previously it never resolved (the open-path watch senders were not +/// drained on terminate), keeping the connection state alive through the +/// future's `ConnectionRef`. +/// +/// [`OpenPath`]: crate::OpenPath +#[tokio::test] +async fn open_path_resolves_on_connection_close() -> TestResult { + let _logging = subscribe(); + let factory = EndpointFactory::new(); + + let mut transport_config = TransportConfig::default(); + transport_config.max_concurrent_multipath_paths(2); + let server = factory.endpoint_with_config("server", transport_config); + let server_addr = server.local_addr()?; + + let server_task = async move { + let conn = server.accept().await.ok_or("closed conn?")?.await?; + conn.closed().await; + TestResult::Ok(()) + } + .instrument(info_span!("server")); + + let mut transport_config = TransportConfig::default(); + transport_config.max_concurrent_multipath_paths(2); + let client = factory.endpoint_with_config("client", transport_config); + + // A socket that accepts datagrams but never answers: path validation + // towards it stays pending forever (no ICMP refusal, no PATH_RESPONSE). + let blackhole = std::net::UdpSocket::bind("127.0.0.1:0")?; + let blackhole_addr = blackhole.local_addr()?; + + let client_task = async move { + let conn = client.connect(server_addr, "localhost")?.await?; + + // Get a pending open: retry until the path id is allocated (right + // after the handshake the open may be rejected for missing CIDs). + let open = loop { + let open = conn.open_path( + FourTuple::from_remote(blackhole_addr), + PathStatus::Available, + ); + if open.path_id().is_some() { + break open; + } + match open.await { + Err(proto::PathError::RemoteCidsExhausted) => { + tokio::time::sleep(Duration::from_millis(20)).await; + } + Ok(_) => unreachable!("a blackholed path cannot validate"), + Err(err) => Err(err)?, + } + }; + + // Close the connection while the path is still validating; the + // pending future must resolve with an error instead of hanging. + conn.close(0u32.into(), b"bye"); + let resolved = tokio::time::timeout(Duration::from_secs(5), open) + .await + .map_err(|_| "OpenPath did not resolve after connection close")?; + assert!(resolved.is_err(), "a blackholed path cannot have validated"); + TestResult::Ok(()) + } + .instrument(info_span!("client")); + + let (server_res, client_res) = tokio::join!(server_task, client_task); + server_res?; + client_res?; + TestResult::Ok(()) +} + /// After `initiate_nat_traversal_round`, the connection driver should be /// woken so that the REACH_OUT frame is sent promptly. Without a wake, /// the frame sits pending until a timer or application data triggers