Making `gracefulClose` more robust?

[vdukhovni]

gracefulClose can still lose data: it stops reading before EOF

This is the follow-up promised in #617. Nothing here blocks that PR; the
problem below predates it and is unchanged by it.

1. Summary

gracefulClose returns after the first readable event on the socket, not after the peer's FIN. If the peer sends any data before its FIN, the first read consumes some of that data and gracefulClose proceeds to close. When our own write side still holds undelivered data at that point, the close aborts the connection and the remaining data may be lost, which is the very failure gracefulClose exists to prevent.

The proposed fix is conceptually simple: keep reading until EOF, under the existing timeout, with a reasonable limit on the total bytes read. A second part of this issue discusses where the cost of the longer wait should live, because the fix makes each graceful close potentially slower, and that has consequences for server worker pools.

2. What the function promises vs. what it does

The docstring says:

This sends TCP FIN and check if TCP FIN is received from the peer.

The implementation performs one recvBuf (under a timeout) and treats "socket became readable" as if it meant "FIN received". Data is not FIN. Any peer that sends something between our shutdown and its own close, which is normal in HTTP/2 (the peer's GOAWAY, PING, RST_STREAM frames) and in many other protocols, defeats the check.

The single read was a deliberate choice. The old event-manager code said:

In error cases, data is available. ... let's stop receiving to prevent
attacks.

That concern is valid: reading until EOF from a hostile peer is unbounded work. The strategy outlined below keeps the read bounded, so it answers the concern rather than ignoring it.

3. The failure mechanism

1. shutdown ShutdownSend queues the FIN behind whatever is still in the kernel send buffer. With a large final write, a slow reader, or a small receive window, the FIN may not even be transmitted yet, let alone acknowledged.
2. The peer sends data. It may already be queued in our receive buffer, or in flight, or in its send buffer if our read side is sufficiently far behind and the send window is full. Our socket becomes readable long before the peer's FIN arrives.
3. gracefulClose reads once, returns, and its finally calls close.
4. close on a socket with unread data in the receive buffer does not
   perform an orderly shutdown. Per RFC 1122 section 4.2.2.13 the kernel
   sends RST and discards the send buffer, including data never yet
   transmitted, and the queued FIN. The same happens if further peer
   data arrives just after close, while the socket is orphaned.
5. The peer receives RST. Our final data may never arrive.

Kernel specifics (Linux, FreeBSD, Windows)

Linux: tcp_close() checks for unread receive data before anything else (before SO_LINGER handling) and takes the tcp_send_active_reset() path, purging the write/retransmit queue.
Data arriving on an orphaned socket in FIN_WAIT_1/2 likewise produces RST and tears the connection down.

FreeBSD: tcp_disconnect() calls tcp_drop() (RST) when so_rcv is non-empty, with the same effect on unsent data.

Windows peers deserve a note: on Linux and BSD receivers, an incoming RST does not discard data already queued for the application, but on Windows it does. So when the peer is a Windows client, an RST race can also destroy data that had already arrived at the peer. Browsers on Windows are not an edge case for HTTP servers.

4. Proposed fix: bounded read to EOF

Replace the single read with a loop, under the same overall timeout, with a byte limit (should it be configurable, and passed to gracefulClose as an additional argument???).

recvEOFtimeout :: Socket -> Int -> Ptr Word8 -> IO ()
recvEOFtimeout s tmout0 buf =
    void $ timeout (tmout0 * 1000) $ loop drainLimit
  where
    loop limit = do
        n <- recvBuf s buf bufSize
        when (n > 0 && limit > n) $ loop (limit - n)

-- Maximum number of bytes to drain while waiting for the peer's FIN.
drainLimit :: Int
drainLimit = 128 * 1024

Behavior:

• Peer sends FIN (possibly after some data): loop reaches EOF, close is clean, no RST. This is the normal cooperative case and now works even when (up to 128K of) data precedes the FIN.
• Peer keeps sending past the byte limit, or the timeout expires: we give up and close, accepting the RST. Graceful close is a courtesy with a budget; a peer that will not stop sending, or will not close, does not get unbounded service.

The exported API is unchanged. The observable difference is that gracefulClose now does what its documentation already says.

5. Contract clarifications

Two documentation points, to make gracefulClose safe to build on:

1. Interruptibility. recvBuf blocks in threadWaitRead and timeout is exception-based, so gracefulClose can be interrupted by an asynchronous exception, and the finally still closes the socket.
   • This is true today but is an undocumented artefact of the implementation.
   • I propose documenting it: "may be interrupted by an asynchronous exception, in which case the socket is closed immediately."
   • External code (see section 6) needs this guarantee to cancel a slow close safely.
2. Docstring. Adjust to mention the byte limit, e.g. "reads and discards incoming data (up to a limit) until the peer's FIN is received or the timeout expires."

Tests worth adding:

• A client that sends data 10 ms after the server starts closing, then expects EOF rather than ECONNRESET (the current test, kept, since it now exercises the loop).
• A client that never closes, with an assertion that gracefulClose takes at least, and roughly, the requested time. This would have caught the ms/μs mixup fixed in reliable gracefulClose #617.

6. Bounding the cost: a graceful-close manager

The fix above makes graceful close correct, but potentially slow: every close may now legitimately block for up to the full timeout, and that time is spent on some thread. On a server, that thread is typically a worker. Under a burst of closes, workers accumulate in gracefulClose and the pool may starve for real requests. This is, I think, what the forkIO in the original #617 was reaching for; the problem is real, but detaching an unbounded thread inside the primitive imposes the cost on all callers and bounds nothing.

So a typical server needs a place to park pending closes, with limits. Every server framework would otherwise implement this independently. Sketch of a shared "graceful close manager" facility:

• Handoff by duplication. The application calls socketToFd (dup
  the descriptor, close the original Socket) and passes the new fd to
  the manager. The application's own resource handling (brackets,
  finalizers) is finished at that point and cannot interfere with the
  manager's copy.
• Bounded pending set, oldest evicted first. The manager keeps a
  priority queue of pending closes keyed by deadline, holding the
  ThreadId of the worker draining each socket. At the cap, it cancels
  the oldest via throwTo; the worker's finally closes the socket
  (abortive if needed). The oldest close has had the most time already,
  so cancelling it loses the least, and peers that never send FIN are
  recycled first.
• Natural back-pressure under fd exhaustion. If dup fails the socket is closed immediately anyway. Under fd pressure, shedding to abortive close is the right behaviour, and it frees a descriptor exactly when descriptors are scarce.
• Waitable shutdown. The manager exposes a handle that main (or the framework's shutdown path) can wait on, so that connection draining can actually complete before the process exits.
• Optional optimization: detect FIN acknowledged. After shutdown, the connection sits in FIN_WAIT_1 and moves to FIN_WAIT_2 exactly when the peer acknowledges our FIN, i.e. when all our data has been delivered to the peer's kernel. TCP_INFO (Linux, FreeBSD; macOS spells it TCP_CONNECTION_INFO, Windows SIO_TCP_INFO) exposes the connection state, so one cheap periodic getsockopt tells the manager when delivery is complete, and the same call returns the kernel's RTT estimate. Once in FIN_WAIT_2 the manager can shorten the remaining time and byte budget: delivery to the peer's kernel is not consumption by the peer's application, so closing immediately would re-open a small RST window (which matters for Windows peers, per section 3), but a settle delay of a few RTTs covers the cooperative case. Byte-count ioctls (SIOCOUTQ, SO_NWRITE, FIONWRITE) could serve as fallbacks, but the state check is simpler and more portable. This needs new cbits and is strictly an optimization; nothing else depends on it.

None of this requires private parts of network; a prototype can be built out-of-tree on the public API (except the ioctl probe). But see the placement question below.

7. Open questions

1. Where does the manager live? The facility is protocol-neutral: HTTP/1 lingering close, HTTP/2 GOAWAY, SMTP after QUIT, TLS close_notify all have the same shape. That argues for network (or a sibling package that network blesses) rather than warp, which would strand every non-HTTP server. The counterargument is that network has mostly stayed a thin socket IO wrapper. Note that the ioctl probe is the one piece that genuinely wants to live in network, since that is where the per-platform portability machinery already is. A reasonable path: agree on the API here, prototype out-of-tree, upstream if the design pans out.
2. The byte limit. Is a constant acceptable, and what value? Or should it be an argument of gracefulClose (API addition), with the current signature keeping a default?
3. Reporting the outcome. gracefulClose returns () whether it saw EOF, timed out, or hit the byte limit. A variant returning PeerClosed | TimedOut | Truncated would help servers keep
   statistics and tune the limits. Worth adding, or noise?

[kazu-yamamoto]

I think the direction of the changes is correct, and the implementation also appears correct at first glance, but the test is failing. I gave it some thought, but I couldn't figure out what was wrong.

[kazu-yamamoto]

Ah, OK. The server in the gracefulClose test uses spends more time than the upper limit.