Handle uninitialized connections in disposal (#1786) (#1794)

- We made a change to not initialize pipes up front
on connection creation. That change make it null ref in disposal because we didn't check if the pipes were initialized.
- Added a test
- Also fixed the EchoConnectionHandler in the functional ts tests.
This commit is contained in:
David Fowler 2018-03-30 15:51:48 -07:00 committed by GitHub
parent 9428f49b8e
commit 0293e53e11
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 31 deletions

View File

@ -11,35 +11,27 @@ namespace FunctionalTests
{ {
public async override Task OnConnectedAsync(ConnectionContext connection) public async override Task OnConnectedAsync(ConnectionContext connection)
{ {
var result = await connection.Transport.Input.ReadAsync(); while (true)
var buffer = result.Buffer;
try
{ {
if (!buffer.IsEmpty) var result = await connection.Transport.Input.ReadAsync();
var buffer = result.Buffer;
try
{ {
await connection.Transport.Output.WriteAsync(buffer.ToArray()); if (!buffer.IsEmpty)
{
await connection.Transport.Output.WriteAsync(buffer.ToArray());
}
else if (result.IsCompleted)
{
break;
}
}
finally
{
connection.Transport.Input.AdvanceTo(result.Buffer.End);
} }
} }
finally
{
connection.Transport.Input.AdvanceTo(result.Buffer.End);
}
// Wait for the user to close
var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
connection.Transport.Input.OnWriterCompleted((ex, state) =>
{
if (ex != null)
{
((TaskCompletionSource<object>)state).TrySetException(ex);
}
else
{
((TaskCompletionSource<object>)state).TrySetResult(null);
}
}, tcs);
await tcs.Task;
} }
} }
} }

View File

@ -143,21 +143,21 @@ namespace Microsoft.AspNetCore.Http.Connections
// If the application task is faulted, propagate the error to the transport // If the application task is faulted, propagate the error to the transport
if (ApplicationTask?.IsFaulted == true) if (ApplicationTask?.IsFaulted == true)
{ {
Transport.Output.Complete(ApplicationTask.Exception.InnerException); Transport?.Output.Complete(ApplicationTask.Exception.InnerException);
} }
else else
{ {
Transport.Output.Complete(); Transport?.Output.Complete();
} }
// If the transport task is faulted, propagate the error to the application // If the transport task is faulted, propagate the error to the application
if (TransportTask?.IsFaulted == true) if (TransportTask?.IsFaulted == true)
{ {
Application.Output.Complete(TransportTask.Exception.InnerException); Application?.Output.Complete(TransportTask.Exception.InnerException);
} }
else else
{ {
Application.Output.Complete(); Application?.Output.Complete();
} }
var applicationTask = ApplicationTask ?? Task.CompletedTask; var applicationTask = ApplicationTask ?? Task.CompletedTask;
@ -180,8 +180,8 @@ namespace Microsoft.AspNetCore.Http.Connections
// REVIEW: Should we move this to the read loops? // REVIEW: Should we move this to the read loops?
// Complete the reading side of the pipes // Complete the reading side of the pipes
Application.Input.Complete(); Application?.Input.Complete();
Transport.Input.Complete(); Transport?.Input.Complete();
} }
} }

View File

@ -190,6 +190,20 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests
Assert.Equal(HttpConnectionContext.ConnectionStatus.Disposed, connection.Status); Assert.Equal(HttpConnectionContext.ConnectionStatus.Disposed, connection.Status);
} }
[Fact]
public async Task DisposeInactiveConnectionWithNoPipes()
{
var connectionManager = CreateConnectionManager();
var connection = connectionManager.CreateConnection();
Assert.NotNull(connection.ConnectionId);
Assert.Null(connection.Transport);
Assert.Null(connection.Application);
await connection.DisposeAsync();
Assert.Equal(HttpConnectionContext.ConnectionStatus.Disposed, connection.Status);
}
[Fact] [Fact]
public void ScanAfterDisposeNoops() public void ScanAfterDisposeNoops()
{ {