reliable gracefulClose

[kazu-yamamoto]

Background: https://kazu-yamamoto.hatenablog.jp/entry/2019/09/20/165939

Since close is non-blocking, gracefulClose should also return immediately. To achieve this, the QUIC library offloads the termination logic to a separate thread using forkIO. Experience has shown this approach to be effective. Since there is no harm in the offloaded thread waiting for a long time, a simple timeout suffices—avoiding the complexity of having the IOManager and TimerManager compete. While this increases the number of threads, it ensures immediate responsiveness and reliability.

@khibino Would you review this?

[vdukhovni]

Thanks for working on this. I have one main concern with the forkIO change
(my thoughts, but the below prose framing the concern is partly AI-assisted)

gracefulClose becomes fire-and-forget, with no limit on pending closes.

Before this PR, gracefulClose blocked the calling thread. That gave a
natural limit: a server with N handler threads could have at most N closes
in progress.

With forkIO, that limit is gone. Each call starts a thread that may hold
the socket FD for the full timeout. Nothing bounds how many of these exist
at once.

Example: a server closing 1000 connections/sec with a 5-second timeout can
hold ~5000 extra FDs, invisible to the application. A malicious peer can
force the worst case cheaply: connect, trigger a close, and never send FIN.
Each such peer pins one FD for the whole timeout. Eventually accept()
fails with EMFILE for everyone.

There are also smaller costs of the detached thread: the caller can no
longer see exceptions (including the annotated ones), cannot cancel a stuck
close, and cannot know when the FD is really released.

Suggestions, in order of preference:

1. Keep gracefulClose synchronous. A caller who wants async behavior can
   write forkIO (gracefulClose s t) themselves, and then owns the thread.
2. If async behavior is the goal, return the ThreadId (or an Async) so
   the caller can wait or cancel.
3. If the fork stays internal, bound the number of pending closes.
   When the limit is reached, abort the oldest pending close with a
   plain close and admit the new one. The oldest close has already had
   the most time to finish, so canceling it loses the least. This also
   means stuck peers (that never send FIN) get recycled first instead of
   occupying slots forever. Graceful close is a courtesy; under pressure,
   shedding to abortive close is the right behavior, and peers must
   handle RST anyway.

The test change also hints at the problem: the added threadDelay after
gracefulClose is needed because the caller no longer knows when the close
finishes.

[ P.S. I have an additional material concern about the gracefulClose implementation, but it predates this PR, and applies equally to the prior state. I think it is likely best to focus on one such topic at a time. ]

[Zemyla]

I agree. Despite both calls eventually closing a socket, close and gracefulClose are not the same.

close doesn't block because it just tells the OS to close the socket. There's no application logic involved.

When you gracefulClose, you send and receive stuff before closing it. The OS doesn't handle that; you do. Therefore, it should block.

[kazu-yamamoto]

Thank you for your comment.

I agree with the observation that users can simply use forkIO to make the blocking gracefulClose non-blocking. Therefore, in this PR, I will limit the changes to simply simplifying the implementation of gracefulClose. Unlike the previous implementation, the timeout function spawns an additional thread only when a timeout occurs; however, I am prioritizing maintainability over that drawback.

[vdukhovni]

Thanks for removing the forkIO — the synchronous version with the
simplified timeout implementation looks good to me.

One bug in the simplification: a lost millisecond/microsecond conversion.

The docstring says the second argument of gracefulClose is in
milliseconds. System.Timeout.timeout takes microseconds. The old
event-manager path converted (tmout = tmout0 * 1000), but the old
recvEOFtimeout fallback did not — a pre-existing bug that only affected
Windows and the non-threaded RTS. This PR makes that path the only path,
so now a documented 5000 ms grace period is really 5 ms on all platforms.

I think this is also why the test needed changing: with the old code on
Linux, gracefulClose sock 3000 waited 3 seconds. With this PR it waits
3 ms, which is less than the client's 10 ms delay before "PING", so the
close happened first and the PING triggered RST. Raising the value to
30000 (= 30 ms) made the test pass again, but by accident, not by fixing
the units.

Suggested fix:

recvEOFtimeout s tmout0 buf =
    void $ timeout (tmout0 * 1000) $ recvBuf s buf bufSize

and the test value can go back to 3000. A test that would catch this
class of bug: a client that never sends FIN, with an assertion that
gracefulClose takes at least (and roughly) the requested time.

[kazu-yamamoto]

It's an embarrassing bug.

The commit list had become unnecessarily complex, so I squashed the commits.

[vdukhovni]

LGTM. The millisecond conversion is fixed.

As mentioned earlier, I have one more concern about gracefulClose, but it predates this PR and is unchanged by it, so it should not block landing this first.

I will open a separate issue for discussion, with a possible follow-on PR. Short version: the current code returns after the first readable event, not the peer's FIN. If our write side still has undelivered data, the early close aborts the connection and that data may be lost. The fix is to keep reading until EOF (with limits). Details in the upcoming separate issue, which merits a longer discussion.

[kazu-yamamoto]

Merged.
Thank you all for discussion.