From 70b5b06b57aa10f6ea980deba78d1331cf557cf9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 2 Oct 2026 03:41:56 +0000 Subject: [PATCH] Keep native client test descriptors alive until the client holds them, and correct a stale test comment Three native client tests stand in for the daemon: they send a socket to the client and then dropped their own copy straight away. Until the client read the message, the message was the socket's only reference, and Darwin's descriptor collector can flush such a socket, so the tests failed now and then on macOS. Each now keeps its copy until the client holds the descriptor, as the daemon itself does. Two of them also provoke a collection in that window, so a regression can show on macOS without waiting for a stray collection; in the bind test the stand-in daemon keeps its copy until the client closes the connection. A comment in the native API tests still described the earlier wait for a closed flow, which watched only the reader's close flag. It now says what the wait observes, a flagged or forgotten flow, and why neither means the release has been served yet. --- src/native/client/mod.rs | 20 +++++++++++++++++--- src/native/mod.rs | 10 ++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/native/client/mod.rs b/src/native/client/mod.rs index f02ec7f1..90dd9d64 100644 --- a/src/native/client/mod.rs +++ b/src/native/client/mod.rs @@ -1092,7 +1092,12 @@ mod tests { // already queued and no read here waits on anything. (&daemon).write_all(REFUSAL).unwrap(); fdpass::send_once(daemon.as_raw_fd(), CONNECT_REPLY, Some(passed.as_fd())).unwrap(); - drop(passed); + // `passed` stays alive until the client holds the descriptor: until + // then the message would be the socket's only reference, and Darwin's + // collector flushes such a socket. The provoked collection is there so + // a regression shows on macOS without waiting for a stray collection; + // whether it shows every run rests on `dgram_probe`'s fixed wait. + crate::native::dgram_probe::provoke_collection(); // **How many reads this takes is the platform's business, and asserting // it was wrong.** Linux coalesces the plain write with the sendmsg that @@ -1133,6 +1138,7 @@ mod tests { let (second, second_fd) = wire.line().unwrap(); assert!(second.starts_with(br#"{"status":"ok""#)); let received = second_fd.expect("the reply must carry the descriptor"); + drop(passed); // A live socket rather than merely a number: the far half sees it. let mut received = UnixStream::from(received); @@ -1207,7 +1213,6 @@ mod tests { let worker = thread::spawn(move || { let command = read_line(&daemon); fdpass::send_once(daemon.as_raw_fd(), LISTEN_REPLY, Some(passed.as_fd())).unwrap(); - drop(passed); // As for connect: setup is over, so the connection is over. A // listener that held one would take its flows down with it. // A deadline, or the defect this asserts against fails as a hang @@ -1220,6 +1225,9 @@ mod tests { let read = (&daemon) .read(&mut rest) .expect("the RPC connection outlived the setup call that opened it"); + // Only now: the client closes once it holds the descriptor, and + // until then the message would be the socket's only reference. + drop(passed); (command, read) }); @@ -1308,9 +1316,15 @@ mod tests { // client could read the arrival is already there when it holds it. (&held).write_all(b"opening").unwrap(); fdpass::send_once(theirs.as_raw_fd(), ARRIVAL, Some(passed.as_fd())).unwrap(); - drop(passed); + // `passed` stays alive until the client holds the descriptor: until + // then the message would be the socket's only reference, and Darwin's + // collector flushes such a socket. The provoked collection is there so + // a regression shows on macOS without waiting for a stray collection; + // whether it shows every run rests on `dgram_probe`'s fixed wait. + crate::native::dgram_probe::provoke_collection(); let (flow, peer) = listener.accept().unwrap(); + drop(passed); assert_eq!(peer.to_string(), format!("{PEER}:5001")); assert_eq!(flow.peer_addr(), peer); // From the arrival's own `node`, not from the listener: an accepted diff --git a/src/native/mod.rs b/src/native/mod.rs index 305c5760..9f599588 100644 --- a/src/native/mod.rs +++ b/src/native/mod.rs @@ -1595,10 +1595,12 @@ mod tests { drop(client); connection.settle_closed(flow).await; - // `settle_closed` observes the reader's flag, which it sets before it - // sends the release, so the registry may not have processed it yet. - // Retry rather than sleep: without the reclaim every attempt fails and - // the loop runs out, which is the failure this test exists to produce. + // `settle_closed` returns once the reader has flagged the flow closed or + // forgotten it. Neither means the registry has served the release: the + // flag is set before the release is queued, and the flow is forgotten + // once it is queued, not once it is processed. Retry rather than sleep: + // without the reclaim every attempt fails and the loop runs out, which + // is the failure this test exists to produce. let mut last = serde_json::Value::Null; for _ in 0..1000 { // Not `ask`: a connect that succeeds carries a descriptor, and that