diff --git a/docs/how-to/use-the-native-datagram-api.md b/docs/how-to/use-the-native-datagram-api.md index adc87334..355ba093 100644 --- a/docs/how-to/use-the-native-datagram-api.md +++ b/docs/how-to/use-the-native-datagram-api.md @@ -286,10 +286,11 @@ port from one dropped because a client was not reading fast enough. For the response shape, see [../reference/control-socket.md](../reference/control-socket.md#read-only-queries). -Reach for this when datagrams go missing. **Four places lose data with +Reach for this when datagrams go missing. **Five places lose data with nothing reported to your program**: a full per-flow queue, a listener that does not accept fast enough, an outbound datagram sent before a session -exists, and an outbound datagram after the transport MTU has fallen. +exists, an outbound datagram after the transport MTU has fallen, and, on +Linux, an empty datagram sent just before a close. [../reference/native-api.md](../reference/native-api.md#where-data-disappears) describes each and what bounds it. diff --git a/docs/how-to/write-a-native-api-client.md b/docs/how-to/write-a-native-api-client.md index bde73efa..a128874a 100644 --- a/docs/how-to/write-a-native-api-client.md +++ b/docs/how-to/write-a-native-api-client.md @@ -85,13 +85,15 @@ that costs a real payload if you get it wrong. A client that sends an empty datagram, then a message, then closes, leaves both queued, and a reader that trusts `POLLHUP` alone discards the message. -**One case has no answer, and you should design around it rather than solve -it.** A zero-length datagram that is the last message before a close is -indistinguishable from the close: reading it drains the queue, and `FIONREAD` -then reports zero because a zero-length message contributes no bytes. If your -protocol gives a zero-length payload a meaning, do not send it as a zero-length -socket message. Carry a one-byte discriminator, and keep the zero-byte read for -end of file alone. +**One case has no answer on Linux, where the pair is `SOCK_SEQPACKET`, and you +should design around it rather than solve it.** A zero-length datagram that is +the last message before a close is indistinguishable from the close: reading it +drains the queue, and `FIONREAD` then reports zero because a zero-length message +contributes no bytes. If your protocol gives a zero-length payload a meaning, do +not send it as a zero-length socket message. Carry a one-byte discriminator, and +keep the zero-byte read for end of file alone. On macOS and FreeBSD the pair is +`SOCK_DGRAM`: the empty datagram reads as zero bytes, and the close is reported +by the read after it. Both directions of the mistake are real. Reading an empty datagram as a close lets a peer tear down a live flow by sending nothing, and presents as a diff --git a/docs/reference/native-api.md b/docs/reference/native-api.md index 61fe1748..cea846d8 100644 --- a/docs/reference/native-api.md +++ b/docs/reference/native-api.md @@ -260,13 +260,15 @@ because it latches while messages are still queued: a read that trusted it would report the close early and discard whatever was waiting. The `EPIPE` that `recv` returns means the daemon went away, never that a peer finished. -**One case is reported as `EPIPE` although it is a datagram.** A zero-length -datagram that is the last message before a close is indistinguishable from the -close, because reading it drains the queue and `FIONREAD` then reports zero: a -zero-length message contributes no bytes. Do not give a zero-length payload a -meaning of its own on this API. Carry a one-byte discriminator instead. -Separating the two needs a payload that is never zero bytes on the wire, which -is a protocol change rather than a receive-path one. +**On Linux, one case is reported as `EPIPE` although it is a datagram.** A +zero-length datagram that is the last message before a close is +indistinguishable from the close, because reading it drains the queue and +`FIONREAD` then reports zero: a zero-length message contributes no bytes. macOS +and FreeBSD carry the flow on `SOCK_DGRAM`, where the datagram is delivered as +`Ok(0)` and the close follows it. Do not give a zero-length payload a meaning +of its own on this API. Carry a one-byte discriminator instead. Separating the +two needs a payload that is never zero bytes on the wire, which is a protocol +change rather than a receive-path one. **`peer_addr()`** and **`local_addr()`** return `FipsAddr`, not `io::Result`, unlike their `TcpStream` counterparts. These are field @@ -438,7 +440,7 @@ There is no acknowledgement, no retransmission, no ordering guarantee and no flow control between the two ends. A program that needs confirmation gets it from the peer, in the payload. -Four places lose data with nothing reported to the client. +Five places lose data with nothing reported to the client. **A full per-flow queue.** Inbound datagrams beyond `pending_per_flow` are dropped with a trace log and no client-visible signal. A program that stops @@ -467,6 +469,14 @@ snapshot taken at setup. The daemon re-checks each outbound datagram against the node's current limit and drops it silently if the transport MTU has since fallen. +**An empty datagram sent just before a close, on Linux.** A program that sends +a zero-length datagram and then drops its `FipsStream` may have the daemon read +that datagram as the close: the daemon frees the flow and the empty datagram +never reaches the peer. In the other direction, an empty datagram from the peer +that the daemon delivers immediately before its own half of the flow closes, +as when the daemon stops, reaches `recv` as `EPIPE` rather than `Ok(0)`. macOS +and FreeBSD are not affected. See `recv` above. + ### The drop causes, and what they mean An inbound datagram can be refused for eight reasons, which render as seven diff --git a/src/native/client/mod.rs b/src/native/client/mod.rs index 1fb85681..ee4c8ade 100644 --- a/src/native/client/mod.rs +++ b/src/native/client/mod.rs @@ -488,10 +488,12 @@ impl FipsStream { /// [`bytes_queued`](super::seqpacket::bytes_queued) reports nothing behind /// it. The daemon half applies the identical rule, on the same pair. /// - /// One case survives both checks: a zero-length datagram that is the last - /// message before the close is indistinguishable from the close, because - /// reading it drains the queue and a zero-length message contributes no - /// bytes. Do not give a zero-length payload a meaning of its own. + /// One case survives both checks on Linux, where the pair is + /// `SOCK_SEQPACKET`: a zero-length datagram that is the last message before + /// the close is indistinguishable from the close, because reading it drains + /// the queue and a zero-length message contributes no bytes, so it is + /// reported as `EPIPE`. On macOS and FreeBSD it is delivered as `Ok(0)` and + /// the close follows. Do not give a zero-length payload a meaning of its own. /// /// A datagram longer than `buf` is truncated and the remainder discarded, /// which is `SOCK_SEQPACKET` behaviour. Size `buf` at diff --git a/src/native/seqpacket.rs b/src/native/seqpacket.rs index 7e17e2cb..d2537a27 100644 --- a/src/native/seqpacket.rs +++ b/src/native/seqpacket.rs @@ -46,7 +46,11 @@ use tokio::io::unix::AsyncFd; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Received { /// A datagram of this many bytes. Zero is a legitimate value: a client may - /// send an empty datagram, and that is not the same as closing. + /// send an empty datagram, and while it keeps its half open that is not the + /// same as closing. On Linux, where the pair is `SOCK_SEQPACKET`, an empty + /// datagram that is the last thing a client sends before it closes reads as + /// [`Received::Eof`] instead; `recv_once` says why that one case cannot be + /// told apart. Datagram(usize), /// The peer closed its half of the pair. Eof, @@ -308,14 +312,18 @@ impl Seqpacket { /// message behind it is never taken. /// /// This matters because reading an empty datagram as a close would let a client -/// tear down its own flow by sending nothing, and the defect would present as a -/// spurious disconnect. +/// tear down its own flow by sending nothing while it is still connected, and the +/// defect would present as a spurious disconnect. /// -/// **One case survives and cannot be fixed here.** A zero-length datagram that -/// is the last message before a close is indistinguishable from the close: -/// reading it drains the queue, and every observation then matches a bare end of -/// file. Separating those needs a payload that is never zero bytes on the wire, -/// which is a protocol change rather than a receive-path one. +/// **One case survives on `SOCK_SEQPACKET`, which is Linux, and cannot be fixed +/// here.** A zero-length datagram that is the last message before a close is +/// indistinguishable from the close: reading it drains the queue, and every +/// observation then matches a bare end of file. The flow ends early and the empty +/// datagram is never delivered. Separating those needs a payload that is never +/// zero bytes on the wire, which is a protocol change rather than a receive-path +/// one. On `SOCK_DGRAM`, which macOS and FreeBSD use, the empty datagram is +/// delivered and the close is reported by the read after it; the tests below +/// pin both behaviours. /// /// **`ECONNRESET` is treated as end of file too**, for the platform whose /// datagram sockets report a close that way rather than through `POLLHUP`. Both @@ -671,6 +679,45 @@ mod tests { assert_eq!(recv_bounded(&daemon, &mut buf).await, Received::Eof); } + #[tokio::test] + async fn a_trailing_empty_datagram_before_a_close_reads_as_the_close_only_on_seqpacket() { + // The case recv_once documents as unfixable, pinned per socket type so + // the documentation cannot drift from what the kernel does. On + // SOCK_SEQPACKET the empty datagram is the last thing queued when the + // client closes, so reading it drains the queue with POLLHUP latched + // and it is indistinguishable from the close. On SOCK_DGRAM it is + // delivered and the close is reported by the read after it. The branch + // is on SOCK_TYPE rather than on the OS, because the socket type is the + // property the limitation depends on. + let (daemon, theirs) = pair().unwrap(); + let daemon = Seqpacket::new(daemon).unwrap(); + let theirs = client(theirs); + + // SAFETY: the descriptor is open and owned by `theirs`. + let sent = unsafe { libc::send(theirs.as_raw_fd(), std::ptr::null(), 0, 0) }; + assert_eq!(sent, 0, "{}", io::Error::last_os_error()); + drop(theirs); + + let mut buf = [0u8; 64]; + if SOCK_TYPE == libc::SOCK_SEQPACKET { + assert_eq!( + recv_bounded(&daemon, &mut buf).await, + Received::Eof, + "a trailing empty datagram on SOCK_SEQPACKET was delivered rather than \ + read as the close; the documented limitation no longer holds on this \ + kernel, so re-measure it and correct the docs that describe it" + ); + } else { + assert_eq!( + recv_bounded(&daemon, &mut buf).await, + Received::Datagram(0), + "a trailing empty datagram on SOCK_DGRAM was not delivered; the docs \ + say only SOCK_SEQPACKET platforms lose it" + ); + assert_eq!(recv_bounded(&daemon, &mut buf).await, Received::Eof); + } + } + #[test] fn both_halves_of_a_pair_are_close_on_exec() { // Asserted rather than assumed because the platforms disagree on how it