From 7c2d9e87e9c55e84265d9b99b6087c8a79d039e0 Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Tue, 3 Apr 2018 17:08:04 -0700 Subject: [PATCH] Clean up pipe pair on transport start failure (#1836) --- .../HttpConnection.cs | 6 +++ ...HttpConnectionTests.ConnectionLifecycle.cs | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs b/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs index c589561b84..8521df7d82 100644 --- a/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Http.Connections.Client/HttpConnection.cs @@ -345,6 +345,12 @@ namespace Microsoft.AspNetCore.Http.Connections.Client catch (Exception ex) { Log.ErrorStartingTransport(_logger, transport, ex); + + // Clean up pipes and null out transport when we fail to start. + pair.Transport.Input.Complete(); + pair.Transport.Output.Complete(); + pair.Application.Input.Complete(); + pair.Application.Output.Complete(); _transport = null; throw; } diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs index 1d7ef85ac8..fb6bf5d9c1 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs @@ -159,6 +159,28 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests } } + [Fact] + public async Task PipesAreDisposedAfterTransportFailsToStart() + { + using (StartLog(out var loggerFactory)) + { + var writerTcs = new TaskCompletionSource(); + var readerTcs = new TaskCompletionSource(); + await WithConnectionAsync( + CreateConnection( + loggerFactory: loggerFactory, + transport: new FakeTransport(writerTcs, readerTcs)), + async (connection) => + { + var ex = await Assert.ThrowsAsync(() => connection.StartAsync(TransferFormat.Text)); + Assert.Equal("Unable to connect to the server with any of the available transports.", ex.Message); + + Assert.True(writerTcs.Task.IsCompleted); + Assert.True(readerTcs.Task.IsCompleted); + }); + } + } + [Fact] public async Task CanDisposeUnstartedConnection() { @@ -356,6 +378,35 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests } } + private class FakeTransport : ITransport + { + private IDuplexPipe _application; + private TaskCompletionSource _writerTcs; + private TaskCompletionSource _readerTcs; + + public FakeTransport(TaskCompletionSource writerTcs, TaskCompletionSource readerTcs) + { + _writerTcs = writerTcs; + _readerTcs = readerTcs; + } + + public Task StartAsync(Uri url, IDuplexPipe application, TransferFormat transferFormat, IConnection connection) + { + _application = application; + Action onCompletedCallback = (ex, tcs) => { ((TaskCompletionSource)tcs).TrySetResult(null); }; + _application.Input.OnWriterCompleted(onCompletedCallback, _writerTcs); + _application.Output.OnReaderCompleted(onCompletedCallback, _readerTcs); + throw new Exception(); + } + + public Task StopAsync() + { + _application.Output.Complete(); + _application.Input.Complete(); + return Task.CompletedTask; + } + } + private static async Task AssertDisposedAsync(HttpConnection connection) { var exception =