fix(io): retry interrupted receives #111

Merged
Lochnair merged 2 commits from fix/linux-receive-eintr into main 2026-08-16 17:35:20 +00:00
10 changed files with 448 additions and 32 deletions
Showing only changes of commit ca2389174f - Show all commits

No files matched your search

+24 -7
View File
@@ -4,7 +4,7 @@
mod in_tree_server; mod in_tree_server;
use std::{ use std::{
io::Read, io::{ErrorKind, Read},
net::{SocketAddr, UdpSocket}, net::{SocketAddr, UdpSocket},
process::{Command, Output, Stdio}, process::{Command, Output, Stdio},
sync::mpsc, sync::mpsc,
@@ -1177,18 +1177,35 @@ fn start_open_failure_server_inner(
FakeServer { addr, done } FakeServer { addr, done }
chatgpt-codex-connector[bot] commented 2026-08-16 17:28:56 +00:00 (Migrated from github.com)
Review

P2 Badge Preserve the timeout deadline across test-server interrupts

When this helper runs under repeated signals spaced closer than its configured 500 ms socket timeout, every Interrupted result starts a new recv_from with the full SO_RCVTIMEO, so the loop has no absolute deadline. start_interruptible_echo_server subsequently joins this thread synchronously, meaning sustained process-group signals can hang the test suite instead of letting the fake peer become quiet. Keep one absolute deadline and apply only the remaining timeout on each retry, as the production blocking receive now does.

AGENTS.md reference: AGENTS.md:L143-L150

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Preserve the timeout deadline across test-server interrupts** When this helper runs under repeated signals spaced closer than its configured 500 ms socket timeout, every `Interrupted` result starts a new `recv_from` with the full `SO_RCVTIMEO`, so the loop has no absolute deadline. `start_interruptible_echo_server` subsequently joins this thread synchronously, meaning sustained process-group signals can hang the test suite instead of letting the fake peer become quiet. Keep one absolute deadline and apply only the remaining timeout on each retry, as the production blocking receive now does. AGENTS.md reference: [AGENTS.md:L143-L150](https://github.com/Lochnair/irtt-rs/blob/ca2389174fd0e103fe645fd3a9f6ee45b3c69429/AGENTS.md#L143-L150) Useful? React with 👍 / 👎.
Lochnair commented 2026-08-16 17:32:59 +00:00 (Migrated from github.com)
Review

Fixed: recv_request/recv_request_timeout now share a recv_from_retrying_interrupted helper that computes one absolute deadline from the socket's configured read timeout and re-applies only the remaining time on each interrupted retry, restoring the configured timeout before returning. Applied the same fix to the equivalent helpers in irtt-client/tests/support/mod.rs. See a598dbd.

🤖 Addressed by Claude Code

Fixed: `recv_request`/`recv_request_timeout` now share a `recv_from_retrying_interrupted` helper that computes one absolute deadline from the socket's configured read timeout and re-applies only the remaining time on each interrupted retry, restoring the configured timeout before returning. Applied the same fix to the equivalent helpers in `irtt-client/tests/support/mod.rs`. See a598dbd. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
} }
/// A fake server's `recv_from`, retrying an interrupted syscall
/// (`EINTR`/`ErrorKind::Interrupted`) instead of panicking on it: real
/// signal delivery (e.g. process-group signals during CI) can interrupt a
/// blocking receive here without there being anything actually wrong.
fn recv_request(socket: &UdpSocket) -> (Vec<u8>, SocketAddr) { fn recv_request(socket: &UdpSocket) -> (Vec<u8>, SocketAddr) {
let mut buf = [0_u8; 2048]; let mut buf = [0_u8; 2048];
let (size, peer) = socket.recv_from(&mut buf).unwrap(); loop {
(buf[..size].to_vec(), peer) match socket.recv_from(&mut buf) {
Ok((size, peer)) => return (buf[..size].to_vec(), peer),
Err(err) if err.kind() == ErrorKind::Interrupted => continue,
Err(err) => panic!("fake server recv_from failed: {err}"),
}
}
} }
/// Like [`recv_request`], but treats any non-interrupting error (including a
/// configured read timeout) as "nothing more to receive" rather than a
/// failure, since callers use this to detect the peer going quiet. An
/// interrupted syscall carries no such information and is retried instead of
/// being misclassified as quiet.
fn recv_request_timeout(socket: &UdpSocket) -> Option<(Vec<u8>, SocketAddr)> { fn recv_request_timeout(socket: &UdpSocket) -> Option<(Vec<u8>, SocketAddr)> {
let mut buf = [0_u8; 2048]; let mut buf = [0_u8; 2048];
socket loop {
.recv_from(&mut buf) match socket.recv_from(&mut buf) {
.ok() Ok((size, peer)) => return Some((buf[..size].to_vec(), peer)),
.map(|(size, peer)| (buf[..size].to_vec(), peer)) Err(err) if err.kind() == ErrorKind::Interrupted => continue,
Err(_) => return None,
}
}
} }
fn open_reply(flags: u8, token: u64, params: &Params) -> Vec<u8> { fn open_reply(flags: u8, token: u64, params: &Params) -> Vec<u8> {
+70 -3
View File
@@ -41,6 +41,19 @@ use crate::{
/// so one managed target cannot retain the current-thread runtime. /// so one managed target cannot retain the current-thread runtime.
const OPEN_POLL_WORK_BUDGET: usize = 64; const OPEN_POLL_WORK_BUDGET: usize = 64;
/// Maximum immediately available receive work performed by one
/// [`AsyncClient::poll_recv`] poll.
///
/// A `WouldBlock` readiness false positive is naturally bounded: `try_io`
/// clears the reactor's readiness on `WouldBlock`, so the next
/// `poll_recv_ready` genuinely awaits a fresh edge. An `Interrupted`
/// (`EINTR`) nonblocking `recvmsg` does not clear that readiness — the
/// syscall was interrupted, not told there was nothing to read — so without
/// an explicit bound a sustained signal delivery rate could otherwise retry
/// in a tight loop on this executor thread. Bounding both cases the same way
/// this crate already bounds open-attempt work keeps that impossible.
const RECV_POLL_WORK_BUDGET: usize = 64;
#[cfg(test)] #[cfg(test)]
use crate::client::ProbeSendTimestamps; use crate::client::ProbeSendTimestamps;
@@ -174,6 +187,11 @@ struct AsyncClientTestHooks {
sends: RefCell<VecDeque<InjectedSend>>, sends: RefCell<VecDeque<InjectedSend>>,
send_attempts: Cell<usize>, send_attempts: Cell<usize>,
receive_would_block: Cell<usize>, receive_would_block: Cell<usize>,
/// Number of subsequent receive attempts (`poll_open` or `poll_recv`)
/// that should report `io::ErrorKind::Interrupted` instead of performing
/// a real nonblocking receive, deterministically exercising interrupted-
/// syscall retry behavior.
inject_recv_interrupted: Cell<usize>,
fail_open_dscp: Cell<bool>, fail_open_dscp: Cell<bool>,
fail_cleanup_send: Cell<bool>, fail_cleanup_send: Cell<bool>,
fail_peer_close_dscp: Cell<bool>, fail_peer_close_dscp: Cell<bool>,
@@ -205,6 +223,18 @@ impl AsyncClientTestHooks {
fn take_probe_timestamps(&self) -> Option<ProbeSendTimestamps> { fn take_probe_timestamps(&self) -> Option<ProbeSendTimestamps> {
self.probe_timestamps.borrow_mut().pop_front() self.probe_timestamps.borrow_mut().pop_front()
} }
/// Consumes one pending injected interruption, if any, reporting whether
/// the caller should report `io::ErrorKind::Interrupted` instead of
/// attempting a real receive.
fn take_injected_interrupted(&self) -> bool {
let remaining = self.inject_recv_interrupted.get();
if remaining == 0 {
return false;
}
self.inject_recv_interrupted.set(remaining - 1);
true
}
} }
/// Low-level Tokio IRTT client for one connected UDP target. /// Low-level Tokio IRTT client for one connected UDP target.
@@ -408,9 +438,26 @@ impl AsyncClient {
Poll::Ready(Ok(true)) => {} Poll::Ready(Ok(true)) => {}
} }
#[cfg(test)]
if self.test_hooks.take_injected_interrupted() {
continue;
}
let datagram = match try_recv_tokio_datagram(&self.socket, &mut state.buffer) { let datagram = match try_recv_tokio_datagram(&self.socket, &mut state.buffer) {
Ok(datagram) => datagram, Ok(datagram) => datagram,
Err(error) if error.kind() == io::ErrorKind::WouldBlock => continue, // WouldBlock is a readiness false positive; Interrupted
// (EINTR) is a transient nonblocking-recvmsg interruption.
// Both retry through the surrounding `OPEN_POLL_WORK_BUDGET`-
// bounded loop above, which re-checks readiness before the
// next attempt and yields back to the executor once that
// budget is exhausted.
Err(error)
if matches!(
error.kind(),
io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted
) =>
{
continue
}
Err(error) => return Poll::Ready(Err(ClientError::Socket(error))), Err(error) => return Poll::Ready(Err(ClientError::Socket(error))),
}; };
if datagram.received_at.mono > state.deadline() { if datagram.received_at.mono > state.deadline() {
@@ -613,7 +660,7 @@ impl AsyncClient {
return Poll::Ready(Err(error)); return Poll::Ready(Err(error));
} }
loop { for _ in 0..RECV_POLL_WORK_BUDGET {
match self.socket.poll_recv_ready(cx) { match self.socket.poll_recv_ready(cx) {
Poll::Pending => return Poll::Pending, Poll::Pending => return Poll::Pending,
Poll::Ready(Ok(())) => {} Poll::Ready(Ok(())) => {}
@@ -628,9 +675,23 @@ impl AsyncClient {
.set(self.test_hooks.receive_would_block.get() - 1); .set(self.test_hooks.receive_would_block.get() - 1);
continue; continue;
} }
#[cfg(test)]
if self.test_hooks.take_injected_interrupted() {
continue;
}
let datagram = match try_recv_tokio_datagram(&self.socket, &mut self.recv_buffer) { let datagram = match try_recv_tokio_datagram(&self.socket, &mut self.recv_buffer) {
Ok(datagram) => datagram, Ok(datagram) => datagram,
Err(error) if error.kind() == io::ErrorKind::WouldBlock => continue, // See `RECV_POLL_WORK_BUDGET`: both a readiness false
// positive and an interrupted nonblocking recvmsg retry
// within this bounded loop rather than failing the receive.
Err(error)
if matches!(
error.kind(),
io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted
) =>
{
continue
}
Err(error) => return Poll::Ready(Err(ClientError::Socket(error))), Err(error) => return Poll::Ready(Err(ClientError::Socket(error))),
}; };
@@ -659,6 +720,12 @@ impl AsyncClient {
} }
return Poll::Ready(Ok(events)); return Poll::Ready(Ok(events));
} }
// Truncated known immediate receive work, exactly as `poll_open`
// does: arrange for another poll even if no new socket-readiness
// edge occurs, rather than spinning this executor thread further.
cx.waker().wake_by_ref();
Poll::Pending
} }
pub(crate) fn poll_close( pub(crate) fn poll_close(
@@ -883,6 +883,53 @@ fn receive_retries_false_readiness_and_shares_packet_classification() {
assert_eq!(server.finish().len(), 2); assert_eq!(server.finish().len(), 2);
} }
#[test]
fn receive_retries_interrupted_syscall_and_shares_packet_classification() {
let server = start_server(move |socket, tx| {
let (params, peer) = open_session(&socket, &tx, None);
let (probe_packet, _) = recv_packet(&socket, &tx);
let sequence = echo_request_sequence(&probe_packet, None);
socket
.send_to(
&echo_reply(&params, sequence, TOKEN, flags::FLAG_REPLY, None),
peer,
)
.unwrap();
});
runtime().block_on(async {
let mut client = opened_client(server.addr, 0).await;
client.send_probe().await.unwrap();
client.test_hooks.inject_recv_interrupted.set(2);
assert!(matches!(
client.recv().await.unwrap().as_slice(),
[ClientEvent::EchoReply { seq: 0, .. }]
));
});
assert_eq!(server.finish().len(), 2);
}
#[test]
fn open_retries_interrupted_syscall_within_the_same_attempt() {
let server = start_open_server(None, 0);
runtime().block_on(async {
let mut client = AsyncClient::connect(config(server.addr, None, 0))
.await
.unwrap();
client.test_hooks.inject_recv_interrupted.set(3);
assert!(matches!(
client.open().await,
Ok(OpenOutcome::Started { .. })
));
// Only one open request reached the server: interruptions were
// absorbed inside the single open attempt, with no retransmit.
assert_eq!(client.test_hooks.send_attempts.get(), 1);
});
server.finish();
}
#[test] #[test]
fn peer_close_dscp_failure_preserves_events_and_logical_ownership() { fn peer_close_dscp_failure_preserves_events_and_logical_ownership() {
let server = start_peer_close_server(); let server = start_peer_close_server();
+115 -14
View File
@@ -14,7 +14,7 @@ use crate::{
config::{ClientConfig, RecvBudget}, config::{ClientConfig, RecvBudget},
error::ClientError, error::ClientError,
event::{ClientEvent, OpenOutcome}, event::{ClientEvent, OpenOutcome},
receive::{drain_tx_timestamps, recv_datagram, try_enable_tx_timestamping}, receive::{drain_tx_timestamps, recv_datagram, try_enable_tx_timestamping, ReceivedDatagram},
session::machine::{ session::machine::{
recv_buffer_size, OpenDatagramDisposition, PreparedOpenAcceptance, ProbeSent, recv_buffer_size, OpenDatagramDisposition, PreparedOpenAcceptance, ProbeSent,
SessionMachine, MAX_OPEN_PACKET_SIZE, SessionMachine, MAX_OPEN_PACKET_SIZE,
@@ -69,6 +69,10 @@ struct ClientTestHooks {
fail_dscp_restore: Cell<bool>, fail_dscp_restore: Cell<bool>,
last_restored_read_timeout: Cell<Option<Duration>>, last_restored_read_timeout: Cell<Option<Duration>>,
prepared_active_session_before_dscp: Cell<bool>, prepared_active_session_before_dscp: Cell<bool>,
/// Number of subsequent receive attempts that should report
/// `io::ErrorKind::Interrupted` before a real receive is attempted.
/// Decremented on every injected receive; `0` means inject nothing.
inject_recv_interrupted: Cell<usize>,
} }
#[cfg(test)] #[cfg(test)]
@@ -264,18 +268,40 @@ impl Client {
self.recv_once_inner() self.recv_once_inner()
} }
fn recv_once_inner(&mut self) -> Result<Vec<ClientEvent>, ClientError> { /// Receive one datagram into `self.recv_buffer`. In test builds, a
let datagram = match recv_datagram(&self.socket, &mut self.recv_buffer) { /// pending `inject_recv_interrupted` count reports
Ok(datagram) => datagram, /// `io::ErrorKind::Interrupted` instead of performing a real receive,
Err(err) /// deterministically exercising interrupted-syscall retry behavior.
if matches!( fn recv_datagram_once(&mut self) -> Result<ReceivedDatagram, io::Error> {
err.kind(), #[cfg(test)]
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut {
) => let remaining = self.test_hooks.inject_recv_interrupted.get();
{ if remaining > 0 {
return Ok(vec![]); self.test_hooks.inject_recv_interrupted.set(remaining - 1);
return Err(io::Error::from(io::ErrorKind::Interrupted));
} }
Err(err) => return Err(ClientError::Socket(err)), }
recv_datagram(&self.socket, &mut self.recv_buffer)
}
/// Same as [`Self::recv_datagram_once`], receiving into a caller-supplied
/// buffer instead of `self.recv_buffer` (used before the negotiated
/// receive buffer size is known, i.e. during Open).
fn recv_datagram_once_into(&self, buf: &mut [u8]) -> Result<ReceivedDatagram, io::Error> {
#[cfg(test)]
{
let remaining = self.test_hooks.inject_recv_interrupted.get();
if remaining > 0 {
self.test_hooks.inject_recv_interrupted.set(remaining - 1);
return Err(io::Error::from(io::ErrorKind::Interrupted));
}
}
recv_datagram(&self.socket, buf)
}
fn recv_once_inner(&mut self) -> Result<Vec<ClientEvent>, ClientError> {
let Some(datagram) = self.recv_datagram_retrying_interrupted()? else {
return Ok(vec![]);
}; };
// One final bounded drain before processing the reply, so a TX // One final bounded drain before processing the reply, so a TX
@@ -297,6 +323,71 @@ impl Client {
Ok(events) Ok(events)
} }
/// Receive one datagram, transparently retrying an interrupted syscall
/// (`io::ErrorKind::Interrupted`, e.g. `EINTR`) without treating it as a
/// genuine socket error, timeout, or empty receive.
///
/// One logical receive keeps one timeout budget: when a receive timeout
/// is configured, an absolute deadline is computed once here and each
/// retry after an interruption restores the socket's remaining time
/// against that same deadline, so repeated interruptions cannot extend
/// this receive past its configured timeout. An unconfigured timeout
/// (`None`) keeps blocking indefinitely after an interruption, matching
/// its no-timeout contract. The socket's read timeout is restored to the
/// caller-configured value before returning, so a later call in the same
/// `recv_available` budget is unaffected by an earlier interruption here.
fn recv_datagram_retrying_interrupted(
&mut self,
) -> Result<Option<ReceivedDatagram>, ClientError> {
let configured_timeout = self.runtime.config().socket_config.recv_timeout;
let deadline = match configured_timeout {
Some(timeout) => Some(
Instant::now()
.checked_add(timeout)
.ok_or(ClientError::DurationOverflow)?,
),
None => None,
};
let mut timeout_narrowed = false;
let result = loop {
match self.recv_datagram_once() {
Ok(datagram) => break Ok(Some(datagram)),
Err(err)
if matches!(
err.kind(),
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut
) =>
{
break Ok(None);
}
Err(err) if err.kind() == io::ErrorKind::Interrupted => {
if let Some(deadline) = deadline {
let Some(remaining) = deadline.checked_duration_since(Instant::now())
else {
break Ok(None);
};
if remaining.is_zero() {
break Ok(None);
}
if let Err(err) = self.socket.set_read_timeout(Some(remaining)) {
break Err(ClientError::Socket(err));
}
timeout_narrowed = true;
}
}
Err(err) => break Err(ClientError::Socket(err)),
}
};
if timeout_narrowed {
if let Err(err) = self.socket.set_read_timeout(configured_timeout) {
return Err(ClientError::Socket(err));
}
}
result
}
/// Receive and classify datagrams until a receive produces no events or the /// Receive and classify datagrams until a receive produces no events or the
/// receive budget is exhausted. /// receive budget is exhausted.
pub fn recv_available(&mut self, budget: RecvBudget) -> Result<Vec<ClientEvent>, ClientError> { pub fn recv_available(&mut self, budget: RecvBudget) -> Result<Vec<ClientEvent>, ClientError> {
@@ -496,12 +587,22 @@ impl Client {
} }
self.socket.set_read_timeout(Some(remaining))?; self.socket.set_read_timeout(Some(remaining))?;
let datagram = match recv_datagram(&self.socket, &mut buf) { let datagram = match self.recv_datagram_once_into(&mut buf) {
Ok(datagram) => datagram, Ok(datagram) => datagram,
// WouldBlock/TimedOut mean the attempt's own deadline
// elapsed; Interrupted (EINTR) means the syscall was
// merely interrupted and carries no timing information of
// its own. Both retry through the same loop, which
// recomputes `remaining` and re-applies it as the read
// timeout on every iteration — so an interruption resumes
// within this same absolute deadline rather than
// resetting or extending it.
Err(err) Err(err)
if matches!( if matches!(
err.kind(), err.kind(),
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut io::ErrorKind::WouldBlock
| io::ErrorKind::TimedOut
| io::ErrorKind::Interrupted
) => ) =>
{ {
if Instant::now() >= deadline { if Instant::now() >= deadline {
@@ -64,6 +64,63 @@ fn open_retries_after_first_timeout() {
server.join(); server.join();
} }
#[test]
fn open_survives_interrupted_receive_within_same_attempt() {
let server = start_fake_server(|socket, tx| {
let (first, peer) = recv_request(&socket, &tx);
let params = Params::decode(&first[4..]).unwrap();
let reply = open_reply(FLAG_OPEN | FLAG_REPLY, TOKEN, &params, None);
socket.send_to(&reply, peer).unwrap();
});
let config = ClientConfig {
open_timeouts: vec![Duration::from_millis(500)],
..default_test_config(server.addr)
};
let mut client = Client::connect(config).unwrap();
// Several interrupted receive attempts before the real reply arrives
// must not be treated as a timeout, must not retransmit the open
// request, and must not advance to a next configured attempt.
client.test_hooks.inject_recv_interrupted.set(5);
let outcome = client.open().unwrap();
assert_open_started(outcome);
// Exactly one request reached the server: interruptions were absorbed
// inside the single configured attempt.
assert_eq!(server.rx.iter().take(1).count(), 1);
server.join();
}
#[test]
fn open_interrupted_receive_does_not_extend_the_attempt_deadline() {
// A fake server that never replies: with a short attempt timeout, the
// real question is whether interruptions can push the effective wait
// past the configured deadline. They must not: the deadline stays
// absolute across any number of interruptions.
let server = start_fake_server(|socket, tx| {
let _ = recv_request(&socket, &tx);
});
let config = ClientConfig {
open_timeouts: vec![Duration::from_millis(200)],
..default_test_config(server.addr)
};
let mut client = Client::connect(config).unwrap();
// Each interruption carries no timing information of its own, so the
// loop must keep re-checking the real deadline rather than treating an
// interruption as "more time available" or a reason to fail outright.
client.test_hooks.inject_recv_interrupted.set(20);
let started = Instant::now();
let result = client.open();
let elapsed = started.elapsed();
assert!(matches!(result, Err(ClientError::OpenTimeout)));
assert!(
elapsed < Duration::from_millis(800),
"interrupted receives must not extend the attempt deadline, took {elapsed:?}"
);
server.join();
}
#[test] #[test]
fn open_timeout_after_all_timeouts() { fn open_timeout_after_all_timeouts() {
let server = timeout_server(Duration::from_millis(700)); let server = timeout_server(Duration::from_millis(700));
@@ -0,0 +1,80 @@
use super::*;
/// An interrupted receive (`io::ErrorKind::Interrupted`, e.g. `EINTR`) must be
/// invisible to the caller: retried transparently, with the eventual reply
/// classified exactly as if no interruption had occurred.
#[test]
fn interrupted_receive_is_retried_and_the_reply_is_still_classified() {
let params = default_params();
let server = start_fake_server(move |socket, tx| {
let (_, peer) = recv_request(&socket, &tx);
let reply = open_reply(FLAG_OPEN | FLAG_REPLY, TOKEN, &params, None);
socket.send_to(&reply, peer).unwrap();
let mut buf = [0_u8; 2048];
let (size, _) = socket.recv_from(&mut buf).unwrap();
let seq = u32::from_le_bytes(buf[12..16].try_into().unwrap());
let _ = size;
let ts = TimestampFields::default();
let reply_packet = echo_reply_packet(TOKEN, seq, &params, &ts, None);
socket.send_to(&reply_packet, peer).unwrap();
});
let config = ClientConfig {
socket_config: crate::SocketConfig {
recv_timeout: Some(Duration::from_secs(2)),
..Default::default()
},
..default_test_config(server.addr)
};
let mut client = Client::connect(config).unwrap();
assert_open_started(client.open().unwrap());
client.send_probe().unwrap();
thread::sleep(Duration::from_millis(50));
client.test_hooks.inject_recv_interrupted.set(10);
let events = client.recv_once().unwrap();
assert_eq!(events.len(), 1);
assert!(
matches!(&events[0], ClientEvent::EchoReply { seq: 0, .. }),
"expected a normal EchoReply despite interrupted receive attempts, got {:?}",
events[0]
);
client.close().unwrap();
server.join();
}
/// One logical receive keeps one timeout budget: repeated interruptions must
/// not extend a `recv_once` call past its configured receive timeout.
#[test]
fn interrupted_receive_does_not_extend_the_configured_timeout() {
let params = default_params();
let server = silent_open_server(params);
let config = ClientConfig {
socket_config: crate::SocketConfig {
recv_timeout: Some(Duration::from_millis(100)),
..Default::default()
},
..default_test_config(server.addr)
};
let mut client = Client::connect(config).unwrap();
assert_open_started(client.open().unwrap());
// No reply is ever coming; only interrupted attempts occur before the
// real socket takes over and legitimately times out.
client.test_hooks.inject_recv_interrupted.set(20);
let started = Instant::now();
let events = client.recv_once().unwrap();
let elapsed = started.elapsed();
assert!(events.is_empty());
assert!(
elapsed < Duration::from_millis(500),
"interrupted receives must not extend the configured receive timeout, took {elapsed:?}"
);
client.close().unwrap();
server.join();
}
@@ -20,6 +20,7 @@ fn kernel_rx_timestamp_unavailable_skip(test_name: &str) {
} }
mod burst_recv; mod burst_recv;
mod interrupted_receive;
mod packet_length; mod packet_length;
mod probe_schedule; mod probe_schedule;
mod reply_classification; mod reply_classification;
@@ -153,7 +153,14 @@ impl ErrorQueueControlBuffer {
/// Nonblocking drain of a single record from `fd`'s `MSG_ERRQUEUE`. /// Nonblocking drain of a single record from `fd`'s `MSG_ERRQUEUE`.
/// ///
/// Returns `Ok(None)` when the queue is empty (`EAGAIN`/`EWOULDBLOCK`). /// Returns `Ok(None)` when the queue is empty (`EAGAIN`/`EWOULDBLOCK`) or
/// when this one read was interrupted (`EINTR`). TX timestamp capture is
/// optional best-effort metadata, not a network operation: an interrupted
/// read here is not a socket/network error and must not become one, so it is
/// treated as no record obtained on this drain rather than retried in a
/// loop. The bounded caller in [`super::drain_tx_timestamps`] naturally
/// revisits the queue on its next call, which is how every other record is
/// picked up too.
/// `SOF_TIMESTAMPING_OPT_TSONLY` notifications carry no meaningful payload, /// `SOF_TIMESTAMPING_OPT_TSONLY` notifications carry no meaningful payload,
/// so a zero-length iovec is enough: correlation is by `id`, never by /// so a zero-length iovec is enough: correlation is by `id`, never by
/// payload content. /// payload content.
@@ -174,7 +181,7 @@ pub(crate) fn try_recv_error_queue_record(fd: RawFd) -> io::Result<Option<ErrorQ
.map_err(|_| io::Error::from(io::ErrorKind::InvalidData))?; .map_err(|_| io::Error::from(io::ErrorKind::InvalidData))?;
chatgpt-codex-connector[bot] commented 2026-08-16 17:28:56 +00:00 (Migrated from github.com)
Review

P2 Badge Continue the bounded drain after an interrupted read

When Linux recvmsg(MSG_ERRQUEUE) is interrupted during the final drain immediately before an echo reply is processed, mapping EINTR to None makes drain_tx_timestamps stop as though the queue were empty. The timestamp record remains queued, but processing the reply removes its pending probe, so a later drain cannot associate the record and the upstream one-way-delay measurement permanently falls back to the less accurate userspace timestamp. Treat the interruption as a skipped attempt within the existing 32-iteration work budget rather than as an empty queue.

AGENTS.md reference: crates/irtt-client/AGENTS.md:L77-L86

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Continue the bounded drain after an interrupted read** When Linux `recvmsg(MSG_ERRQUEUE)` is interrupted during the final drain immediately before an echo reply is processed, mapping `EINTR` to `None` makes `drain_tx_timestamps` stop as though the queue were empty. The timestamp record remains queued, but processing the reply removes its pending probe, so a later drain cannot associate the record and the upstream one-way-delay measurement permanently falls back to the less accurate userspace timestamp. Treat the interruption as a skipped attempt within the existing 32-iteration work budget rather than as an empty queue. AGENTS.md reference: [crates/irtt-client/AGENTS.md:L77-L86](https://github.com/Lochnair/irtt-rs/blob/ca2389174fd0e103fe645fd3a9f6ee45b3c69429/crates/irtt-client/AGENTS.md#L77-L86) Useful? React with 👍 / 👎.
Lochnair commented 2026-08-16 17:32:52 +00:00 (Migrated from github.com)
Review

Fixed: an interrupted MSG_ERRQUEUE read now reports ErrorQueueRecord::Ignored instead of None, so it consumes one attempt of the drain's existing per-call work budget and retries the same slot rather than ending the drain early. See a598dbd.

🤖 Addressed by Claude Code

Fixed: an interrupted `MSG_ERRQUEUE` read now reports `ErrorQueueRecord::Ignored` instead of `None`, so it consumes one attempt of the drain's existing per-call work budget and retries the same slot rather than ending the drain early. See a598dbd. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
Ok(Some(classify(cmsgs))) Ok(Some(classify(cmsgs)))
} }
Err(Errno::EWOULDBLOCK) => Ok(None), Err(Errno::EWOULDBLOCK | Errno::EINTR) => Ok(None),
Err(errno) => Err(io::Error::from(errno)), Err(errno) => Err(io::Error::from(errno)),
} }
} }
+20 -5
View File
@@ -4,6 +4,7 @@ mod in_tree_server;
mod real_irtt; mod real_irtt;
use std::{ use std::{
io::ErrorKind,
net::{SocketAddr, UdpSocket}, net::{SocketAddr, UdpSocket},
process::Command, process::Command,
sync::mpsc, sync::mpsc,
@@ -447,17 +448,31 @@ where
FakeServer { addr, rx, done } FakeServer { addr, rx, done }
} }
/// A fake server's `recv_from`, retrying an interrupted syscall
/// (`EINTR`/`ErrorKind::Interrupted`) instead of panicking on it.
fn recv_request(socket: &UdpSocket) -> (Vec<u8>, SocketAddr) { fn recv_request(socket: &UdpSocket) -> (Vec<u8>, SocketAddr) {
let mut buf = [0_u8; 8192]; let mut buf = [0_u8; 8192];
let (size, peer) = socket.recv_from(&mut buf).unwrap(); loop {
(buf[..size].to_vec(), peer) match socket.recv_from(&mut buf) {
Ok((size, peer)) => return (buf[..size].to_vec(), peer),
Err(err) if err.kind() == ErrorKind::Interrupted => continue,
Err(err) => panic!("fake server recv_from failed: {err}"),
}
}
} }
/// Like [`recv_request`], but any non-interrupting error (including a
/// configured read timeout) means "nothing more to receive" rather than a
/// failure. An interrupted syscall carries no such information and is
/// retried instead of being misclassified as quiet.
fn recv_request_timeout(socket: &UdpSocket) -> Option<Vec<u8>> { fn recv_request_timeout(socket: &UdpSocket) -> Option<Vec<u8>> {
let mut buf = [0_u8; 8192]; let mut buf = [0_u8; 8192];
match socket.recv_from(&mut buf) { loop {
Ok((size, _)) => Some(buf[..size].to_vec()), match socket.recv_from(&mut buf) {
Err(_) => None, Ok((size, _)) => return Some(buf[..size].to_vec()),
Err(err) if err.kind() == ErrorKind::Interrupted => continue,
Err(_) => return None,
}
} }
} }
+25 -1
View File
@@ -152,6 +152,18 @@ pub(super) async fn receive_bound(
receive_ancillary(socket, buffer, false).await receive_ancillary(socket, buffer, false).await
} }
/// Consecutive interrupted (`EINTR`) nonblocking receives tolerated before
/// this receive loop explicitly yields to the runtime.
///
/// `try_io` only clears the reactor's readiness state on `WouldBlock`; an
/// `Interrupted` result leaves it untouched, so `socket.readable().await`
/// can resolve immediately again without a genuine new readiness edge. Under
/// a sustained signal delivery rate that could otherwise retry in a tight
/// loop without ever suspending this task. Yielding periodically bounds that
/// without treating the interruption as any kind of failure or terminating
/// the listener.
const INTERRUPTED_YIELD_THRESHOLD: u32 = 64;
/// `require_destination` is the listener's own wildcard-ness. It decides both /// `require_destination` is the listener's own wildcard-ness. It decides both
/// what this receive must recover and how severe a truncated control buffer is. /// what this receive must recover and how severe a truncated control buffer is.
async fn receive_ancillary( async fn receive_ancillary(
@@ -159,12 +171,24 @@ async fn receive_ancillary(
buffer: &mut [u8], buffer: &mut [u8],
require_destination: bool, require_destination: bool,
) -> io::Result<Option<ReceivedDatagram>> { ) -> io::Result<Option<ReceivedDatagram>> {
let mut consecutive_interrupted = 0_u32;
loop { loop {
socket.readable().await?; socket.readable().await?;
match socket.try_io(Interest::READABLE, || { match socket.try_io(Interest::READABLE, || {
receive_once(socket, buffer, require_destination) receive_once(socket, buffer, require_destination)
}) { }) {
Err(error) if error.kind() == io::ErrorKind::WouldBlock => continue, Err(error) if error.kind() == io::ErrorKind::WouldBlock => {
consecutive_interrupted = 0;
continue;
}
Err(error) if error.kind() == io::ErrorKind::Interrupted => {
consecutive_interrupted += 1;
if consecutive_interrupted >= INTERRUPTED_YIELD_THRESHOLD {
consecutive_interrupted = 0;
tokio::task::yield_now().await;
}
continue;
}
result => return result, result => return result,
} }
} }