Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/DiffEngine.Tests/ViewerProtocolTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,42 @@ public async Task AnUnresponsiveOwnerTimesOutRatherThanHanging()
listener.Stop();
}
}
/// <summary>
/// Which socket failures mean the listener has stopped, as against one accept having failed.
/// <para>
/// 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.
/// </para>
/// </summary>
[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);
}

/// <summary>
/// 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.
/// </summary>
[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();
}

/// <summary>
/// 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
Expand Down
21 changes: 20 additions & 1 deletion src/DiffEngine/Protocol/ViewerServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,20 @@ public async Task Listen(Func<ViewerMessage, ViewerResponse> 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
Expand All @@ -88,6 +98,15 @@ public async Task Listen(Func<ViewerMessage, ViewerResponse> handle, Cancel canc
}
}

/// <summary>
/// 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.
/// </summary>
internal static bool IsStop(SocketException exception, Cancel cancel) =>
cancel.IsCancellationRequested ||
exception.SocketErrorCode is SocketError.OperationAborted or SocketError.Interrupted;

// ReSharper disable once ReplaceAsyncWithTaskReturn
async Task<TcpClient> Accept(Cancel cancel)
{
Expand Down
Loading