fix #1199 by suppressing ODE in received callback (#1395)

src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnection.cs

@@ -97,7 +97,18 @@ namespace Microsoft.AspNetCore.SignalR.Client
             if (_needKeepAlive)
             {
                 _logger.ResettingKeepAliveTimer();
-                _timeoutTimer.Change(ServerTimeout, Timeout.InfiniteTimeSpan);
+
+                // If the connection is disposed while this callback is firing, or if the callback is fired after dispose
+                // (which can happen because of some races), this will throw ObjectDisposedException. That's OK, because
+                // we don't need the timer anyway.
+                try
+                {
+                    _timeoutTimer.Change(ServerTimeout, Timeout.InfiniteTimeSpan);
+                }
+                catch (ObjectDisposedException)
+                {
+                    // This is OK!
+                }
             }
         }
 
@@ -157,8 +168,10 @@ namespace Microsoft.AspNetCore.SignalR.Client
 
         private async Task DisposeAsyncCore()
         {
-            _timeoutTimer.Dispose();
             await _connection.DisposeAsync();
+
+            // Dispose the timer AFTER shutting down the connection.
+            _timeoutTimer.Dispose();
         }
 
         // TODO: Client return values/tasks?

test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs

@@ -207,6 +207,21 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
             Assert.Equal("Server timeout (100.00ms) elapsed without receiving a message from the server.", exception.Message);
         }
 
+        [Fact]
+        public async Task OnReceivedAfterTimerDisposedDoesNotThrow()
+        {
+            var connection = new TestConnection();
+            var hubConnection = new HubConnection(connection, new JsonHubProtocol(), new LoggerFactory());
+            await hubConnection.StartAsync().OrTimeout();
+            await hubConnection.DisposeAsync().OrTimeout();
+
+            // Fire callbacks, they shouldn't fail
+            foreach (var registration in connection.Callbacks)
+            {
+                await registration.InvokeAsync(new byte[0]);
+            }
+        }
+
         // Moq really doesn't handle out parameters well, so to make these tests work I added a manual mock -anurse
         private class MockHubProtocol : IHubProtocol
         {

test/Microsoft.AspNetCore.SignalR.Client.Tests/TestConnection.cs

@@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
         private bool _closed;
         private object _closedLock = new object();
 
-        private readonly List<ReceiveCallback> _callbacks = new List<ReceiveCallback>();
+        public List<ReceiveCallback> Callbacks { get; } = new List<ReceiveCallback>();
 
         public IFeatureCollection Features { get; } = new FeatureCollection();
 
@@ -130,9 +130,9 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
                         while (_receivedMessages.Reader.TryRead(out var message))
                         {
                             ReceiveCallback[] callbackCopies;
-                            lock (_callbacks)
+                            lock (Callbacks)
                             {
-                                callbackCopies = _callbacks.ToArray();
+                                callbackCopies = Callbacks.ToArray();
                             }
 
                             foreach (var callback in callbackCopies)
@@ -170,14 +170,14 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests
         public IDisposable OnReceived(Func<byte[], object, Task> callback, object state)
         {
             var receiveCallBack = new ReceiveCallback(callback, state);
-            lock (_callbacks)
+            lock (Callbacks)
             {
-                _callbacks.Add(receiveCallBack);
+                Callbacks.Add(receiveCallBack);
             }
-            return new Subscription(receiveCallBack, _callbacks);
+            return new Subscription(receiveCallBack, Callbacks);
         }
 
-        private class ReceiveCallback
+        public class ReceiveCallback
         {
             private readonly Func<byte[], object, Task> _callback;
             private readonly object _state;