Fixed race in sockets transport (#2279)

- Based on the changes you made earlier (f4d27e6), we trigger OnConnectionClosed before the socket is disposed in the SocketTransport. This moves the call to Output.Complete to happen after and thus fixes the race.
This commit is contained in:
David Fowler 2018-01-29 12:41:45 -08:00 committed by GitHub
parent bbe8851f32
commit 50b396cec6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 11 additions and 5 deletions

View File

@ -57,13 +57,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
public async Task StartAsync(IConnectionHandler connectionHandler)
{
Exception sendError = null;
try
{
connectionHandler.OnConnection(this);
// Spawn send and receive logic
Task receiveTask = DoReceive();
Task sendTask = DoSend();
Task<Exception> sendTask = DoSend();
// If the sending task completes then close the receive
// We don't need to do this in the other direction because the kestrel
@ -76,7 +77,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
// Now wait for both to complete
await receiveTask;
await sendTask;
sendError = await sendTask;
// Dispose the socket(should noop if already called)
_socket.Dispose();
@ -85,6 +86,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
{
_trace.LogError(0, ex, $"Unexpected exception in {nameof(SocketConnection)}.{nameof(StartAsync)}.");
}
finally
{
// Complete the output after disposing the socket
Output.Complete(sendError);
}
}
private async Task DoReceive()
@ -181,7 +187,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
}
}
private async Task DoSend()
private async Task<Exception> DoSend()
{
Exception error = null;
@ -233,8 +239,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
}
finally
{
Output.Complete(error);
// Make sure to close the connection only after the _aborted flag is set.
// Without this, the RequestsCanBeAbortedMidRead test will sometimes fail when
// a BadHttpRequestException is thrown instead of a TaskCanceledException.
@ -242,6 +246,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal
_trace.ConnectionWriteFin(ConnectionId);
_socket.Shutdown(SocketShutdown.Both);
}
return error;
}
}
}