0911172d

By: Tom Sydney Kerckhove <syd@cs-syd.eu>

warp: count a connection when it is accepted, not when its thread starts

fork hands the new connection to settingsFork and the accept loop goes
straight back to accept.  The counter was raised by onOpen, which runs in
the forked thread, so from accept returning until that thread is scheduled
the connection exists and the counter does not count it.

gracefulShutdown runs waitForZero on that counter once the accept loop has
ended.  A connection accepted in that gap reads as zero, so warp stops
while holding it: runSettings returns, and by then the timeout manager and
the caches that serving it would need have been torn down.  The caller's
next act is normally to exit, so that connection is over, unanswered.

This is reached whenever the listening socket outlives the process, as
under systemd socket activation, because connections keep being accepted
right up to the close and the process exits immediately after.  A service
of mine dropped six requests across twenty restarts when probed with
sixteen concurrent clients.  With this change the same probe over the same
number of restarts drops none.

increase now happens in fork before settingsFork, in the accept loop's own
thread, so a connection is counted from the moment accept hands it over
and there is no gap left for graceful shutdown to look into.

decrease now happens in a finally around the whole thread body rather than
in onClose, so every counted connection is uncounted exactly once however
it ends.  onClose would miss the connections that never reach onOpen,
which is any whose mkConn throws: under warp-tls, a failed handshake.
onOpen and onClose keep only the user callbacks.

This stops warp abandoning a connection it accepted.  It does not on its
own guarantee that connection's request is answered: once shutdown has
begun, makeGracefulRecv gives a synthetic EOF to any connection with no
Application in progress, and a connection accepted moments before the
close has none yet.  In practice its thread reaches connRecv first, which
is why the probe above goes to zero.  Whether a connection carrying an
unread request should count as idle is a separate question, untouched
here.

The two other readers of the counter shift with it.  waitForDecreased, the
EMFILE backoff, no longer reports NoConnections while warp is holding a
connection it has accepted but not yet begun.  getCount and
currentOpenConnections count from accept rather than from the connection
thread starting, so a connection appears in them a little earlier, and one
whose mkConn throws appears at all.

GracefulShutdownSpec gets the case as a test.  Master's suite is 121
examples and 0 failures; with this it is 122 and 0.  Keeping the test and
reverting only the change to fork gives 122 examples and exactly 1
failure, the new one.

Timeline