From 622e133a8a7c6690b1c7367ffd60807996d478e1 Mon Sep 17 00:00:00 2001
From: Andrew Stanton-Nurse <andrew@stanton-nurse.com>
Date: Wed, 2 May 2018 13:10:58 -0700
Subject: [PATCH] fix #2134 by disposing httpconnection if start fails (#2137)
 (#2188)

---
 .../HttpConnectionFactory.cs                  | 15 +++++++--
 .../HttpConnectionFactoryTests.cs             | 32 +++++++++++++++++++
 .../TestHttpMessageHandler.cs                 |  8 +++++
 3 files changed, 52 insertions(+), 3 deletions(-)
 create mode 100644 test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionFactoryTests.cs

diff --git a/src/Microsoft.AspNetCore.SignalR.Client/HttpConnectionFactory.cs b/src/Microsoft.AspNetCore.SignalR.Client/HttpConnectionFactory.cs
index 95a9dfe76e..99da56fbb4 100644
--- a/src/Microsoft.AspNetCore.SignalR.Client/HttpConnectionFactory.cs
+++ b/src/Microsoft.AspNetCore.SignalR.Client/HttpConnectionFactory.cs
@@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.SignalR.Client
             {
                 throw new ArgumentNullException(nameof(loggerFactory));
             }
-            
+
             _httpConnectionOptions = options.Value;
             _loggerFactory = loggerFactory;
         }
@@ -44,8 +44,17 @@ namespace Microsoft.AspNetCore.SignalR.Client
         public async Task<ConnectionContext> ConnectAsync(TransferFormat transferFormat, CancellationToken cancellationToken = default)
         {
             var connection = new HttpConnection(_httpConnectionOptions, _loggerFactory);
-            await connection.StartAsync(transferFormat, cancellationToken);
-            return connection;
+            try
+            {
+                await connection.StartAsync(transferFormat, cancellationToken);
+                return connection;
+            }
+            catch
+            {
+                // Make sure the connection is disposed, in case it allocated any resources before failing.
+                await connection.DisposeAsync();
+                throw;
+            }
         }
 
         /// <inheritdoc />
diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionFactoryTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionFactoryTests.cs
new file mode 100644
index 0000000000..dc9dcbb371
--- /dev/null
+++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionFactoryTests.cs
@@ -0,0 +1,32 @@
+using System;
+using System.Net.Http;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Connections;
+using Microsoft.AspNetCore.Http.Connections.Client;
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
+using Xunit;
+
+namespace Microsoft.AspNetCore.SignalR.Client.Tests
+{
+    public class HttpConnectionFactoryTests
+    {
+        [Fact]
+        public async Task ConnectionIsDisposedIfItFailsToStartAsync()
+        {
+            var testHandler = new TestHttpMessageHandler(autoNegotiate: false, handleFirstPoll: false);
+            testHandler.OnRequest((req, next, ct) => Task.FromException<HttpResponseMessage>(new Exception("BOOM")));
+
+            var factory = new HttpConnectionFactory(Options.Create(new HttpConnectionOptions() {
+                Url = new Uri("http://example.com"),
+                HttpMessageHandlerFactory = _ => testHandler
+            }), NullLoggerFactory.Instance);
+
+            // We don't care about the specific exception
+            await Assert.ThrowsAnyAsync<Exception>(() => factory.ConnectAsync(TransferFormat.Text));
+
+            // We care that the handler (and by extension the client) was disposed
+            Assert.True(testHandler.Disposed);
+        }
+    }
+}
diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/TestHttpMessageHandler.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/TestHttpMessageHandler.cs
index 93b234c43b..43251e4bc1 100644
--- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/TestHttpMessageHandler.cs
+++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/TestHttpMessageHandler.cs
@@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
 
         private List<Func<RequestDelegate, RequestDelegate>> _middleware = new List<Func<RequestDelegate, RequestDelegate>>();
 
+        public bool Disposed { get; private set; }
+
         public IReadOnlyList<HttpRequestMessage> ReceivedRequests
         {
             get
@@ -52,6 +54,12 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
             }
         }
 
+        protected override void Dispose(bool disposing)
+        {
+            Disposed = true;
+            base.Dispose(disposing);
+        }
+
         protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
             await Task.Yield();