diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index 02cf356c..c581eedb 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -741,6 +741,42 @@ public async Task AnUnresponsiveOwnerTimesOutRatherThanHanging() listener.Stop(); } } + /// + /// Which socket failures mean the listener has stopped, as against one accept having failed. + /// + /// Returning on any SocketException gave the queue away for the life of the process: the + /// socket stays bound so nobody else can take it, and every later client lands in a backlog + /// nothing is draining. A peer that resets while its connection sits in that backlog is the + /// ordinary way to hit it - WSAECONNRESET on Windows, ECONNABORTED on BSD and macOS - and is + /// why Kestrel retries the same condition. + /// + /// + [Test] + [Arguments(SocketError.OperationAborted, true)] + [Arguments(SocketError.Interrupted, true)] + [Arguments(SocketError.ConnectionReset, false)] + [Arguments(SocketError.ConnectionAborted, false)] + [Arguments(SocketError.NetworkDown, false)] + public async Task SocketFailuresThatStopTheListener(SocketError error, bool expected) + { + var exception = new SocketException((int) error); + + await Assert.That(ViewerServer.IsStop(exception, default)).IsEqualTo(expected); + } + + /// + /// And a cancelled token means stop whatever the code says, since that is the ordinary way a + /// listener is shut down and the token may be observed before the exception is. + /// + [Test] + public async Task ACancelledTokenStopsTheListenerWhateverTheCode() + { + using var cancel = new CancelSource(); + await cancel.CancelAsync(); + + await Assert.That(ViewerServer.IsStop(new((int) SocketError.ConnectionReset), cancel.Token)).IsTrue(); + } + /// /// An owner that answers with an error is not an absent one. Collapsing the two into false /// meant a refused inline was read as "nobody is there", so a second viewer was launched, it diff --git a/src/DiffEngine/Protocol/ViewerServer.cs b/src/DiffEngine/Protocol/ViewerServer.cs index 93a6cfba..5c568708 100644 --- a/src/DiffEngine/Protocol/ViewerServer.cs +++ b/src/DiffEngine/Protocol/ViewerServer.cs @@ -76,10 +76,20 @@ public async Task Listen(Func handle, Cancel canc // Same, on the frameworks where a stopped listener reports it this way. return; } - catch (SocketException) + catch (SocketException exception) + when (IsStop(exception, cancel)) { return; } + catch (SocketException) + { + // A failure of one accept rather than of the listener. A peer that resets while + // its connection sits in the backlog surfaces exactly this way - WSAECONNRESET on + // Windows, ECONNABORTED on BSD and macOS - and returning gave the queue away for + // the life of the process: the socket stays bound, so nobody else can take it, + // and every later client lands in a backlog nothing is draining + continue; + } // Each connection on its own task, so one slow exchange does not stop the next from // being answered. Accepting an inline snapshot legitimately takes seconds, and a @@ -88,6 +98,15 @@ public async Task Listen(Func handle, Cancel canc } } + /// + /// Whether a socket failure means the listener itself has stopped, rather than one accept + /// having failed. Cancellation is the ordinary way that happens; the two error codes are how a + /// stopped listener reports itself when the token has not been observed yet. + /// + internal static bool IsStop(SocketException exception, Cancel cancel) => + cancel.IsCancellationRequested || + exception.SocketErrorCode is SocketError.OperationAborted or SocketError.Interrupted; + // ReSharper disable once ReplaceAsyncWithTaskReturn async Task Accept(Cancel cancel) {