Making `gracefulClose` more robust?
- Dominant language
- Haskell
- Stars
- 366
- Forks
- 210
- Avg merge
- 22h 38m
- Merged PRs (30d)
- 5
Description
# `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???).
```haskell
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 #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?
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by locating the gracefulClose entry point, its docstring, and the existing test described in the issue; compare its single-read behavior with the proposed bounded drain-to-EOF loop and the timeout coverage from #617. Before coding, resolve the byte-limit and API questions and decide whether the graceful-close manager belongs in network. Done means the cooperative data-then-FIN case avoids ECONNRESET while timeout and byte-limit cases remain bounded.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- haskell
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100