Manage connection state with a Monitor instead of a SemaphoreSlim

Author: halter73

src/SignalR/common/Http.Connections/ref/Microsoft.AspNetCore.Http.Connections.netcoreapp3.0.cs

@@ -89,9 +89,9 @@ public HttpConnectionContext(string id, System.IO.Pipelines.IDuplexPipe transpor
         public Microsoft.AspNetCore.Http.HttpContext HttpContext { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public override System.Collections.Generic.IDictionary<object, object> Items { get { throw null; } set { } }
         public System.DateTime LastSeenUtc { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public System.DateTime? LastSeenUtcIfInactive { get { throw null; } }
         public System.Threading.Tasks.Task PreviousPollTask { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
-        public System.Threading.SemaphoreSlim StateLock { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
-        public Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus Status { get { throw null; } set { } }
+        public Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus Status { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public Microsoft.AspNetCore.Connections.TransferFormat SupportedFormats { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public override System.IO.Pipelines.IDuplexPipe Transport { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public System.Threading.Tasks.Task TransportTask { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
@@ -100,9 +100,10 @@ public HttpConnectionContext(string id, System.IO.Pipelines.IDuplexPipe transpor
         public System.Threading.SemaphoreSlim WriteLock { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         [System.Diagnostics.DebuggerStepThroughAttribute]
         public System.Threading.Tasks.Task DisposeAsync(bool closeGracefully = false) { throw null; }
+        public void MarkInactive() { }
         public void OnHeartbeat(System.Action<object> action, object state) { }
         public void TickHeartbeat() { }
-        public bool TryChangeState(Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus from, Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus to) { throw null; }
+        public Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus TryChangeState(Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus from, Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionStatus to) { throw null; }
     }
     public partial class HttpConnectionDispatcher
     {
@@ -122,7 +123,6 @@ public void CloseConnections() { }
         [System.Diagnostics.DebuggerStepThroughAttribute]
         public System.Threading.Tasks.Task DisposeAndRemoveAsync(Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionContext connection, bool closeGracefully) { throw null; }
         public void RemoveConnection(string id) { }
-        [System.Diagnostics.DebuggerStepThroughAttribute]
         public System.Threading.Tasks.Task ScanAsync() { throw null; }
         public void Start() { }
         public bool TryGetConnection(string id, out Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionContext connection) { throw null; }

src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs

@@ -28,14 +28,14 @@ public class HttpConnectionContext : ConnectionContext,
                                          IHttpTransportFeature,
                                          IConnectionInherentKeepAliveFeature
     {
+        private readonly object _stateLock = new object();
         private readonly object _itemsLock = new object();
         private readonly object _heartbeatLock = new object();
         private List<(Action<object> handler, object state)> _heartbeatHandlers;
         private readonly ILogger _logger;
         private PipeWriterStream _applicationStream;
         private IDuplexPipe _application;
         private IDictionary<object, object> _items;
-        private int _status = (int)HttpConnectionStatus.Inactive;
 
         // This tcs exists so that multiple calls to DisposeAsync all wait asynchronously
         // on the same task
@@ -83,7 +83,6 @@ public HttpConnectionContext(string id, IDuplexPipe transport, IDuplexPipe appli
         public HttpTransportType TransportType { get; set; }
 
         public SemaphoreSlim WriteLock { get; } = new SemaphoreSlim(1, 1);
-        public SemaphoreSlim StateLock { get; } = new SemaphoreSlim(1, 1);
 
         // Used for testing only
         internal Task DisposeAndRemoveTask { get; set; }
@@ -96,7 +95,18 @@ public HttpConnectionContext(string id, IDuplexPipe transport, IDuplexPipe appli
 
         public DateTime LastSeenUtc { get; set; }
 
-        public HttpConnectionStatus Status { get => (HttpConnectionStatus)_status; set => Interlocked.Exchange(ref _status, (int)value); }
+        public DateTime? LastSeenUtcIfInactive
+        {
+            get
+            {
+                lock (_stateLock)
+                {
+                    return Status == HttpConnectionStatus.Inactive ? (DateTime?)LastSeenUtc : null;
+                }
+            }
+        }
+
+        public HttpConnectionStatus Status { get; set; } = HttpConnectionStatus.Inactive;
 
         public override string ConnectionId { get; set; }
 
@@ -184,29 +194,29 @@ public async Task DisposeAsync(bool closeGracefully = false)
         {
             Task disposeTask;
 
-            await StateLock.WaitAsync();
             try
             {
-                if (Status == HttpConnectionStatus.Disposed)
+                lock (_stateLock)
                 {
-                    disposeTask = _disposeTcs.Task;
-                }
-                else
-                {
-                    Status = HttpConnectionStatus.Disposed;
+                    if (Status == HttpConnectionStatus.Disposed)
+                    {
+                        disposeTask = _disposeTcs.Task;
+                    }
+                    else
+                    {
+                        Status = HttpConnectionStatus.Disposed;
 
-                    Log.DisposingConnection(_logger, ConnectionId);
+                        Log.DisposingConnection(_logger, ConnectionId);
 
-                    var applicationTask = ApplicationTask ?? Task.CompletedTask;
-                    var transportTask = TransportTask ?? Task.CompletedTask;
+                        var applicationTask = ApplicationTask ?? Task.CompletedTask;
+                        var transportTask = TransportTask ?? Task.CompletedTask;
 
-                    disposeTask = WaitOnTasks(applicationTask, transportTask, closeGracefully);
+                        disposeTask = WaitOnTasks(applicationTask, transportTask, closeGracefully);
+                    }
                 }
             }
             finally
             {
-                StateLock.Release();
-
                 Cancellation?.Dispose();
 
                 Cancellation = null;
@@ -310,9 +320,31 @@ private async Task WaitOnTasks(Task applicationTask, Task transportTask, bool cl
             }
         }
 
-        public bool TryChangeState(HttpConnectionStatus from, HttpConnectionStatus to)
+        public HttpConnectionStatus TryChangeState(HttpConnectionStatus from, HttpConnectionStatus to)
         {
-            return Interlocked.CompareExchange(ref _status, (int)to, (int)from) == (int)from;
+            lock (_stateLock)
+            {
+                if (Status == from)
+                {
+                    Status = to;
+                    return from;
+                }
+
+                return Status;
+            }
+        }
+
+        public void MarkInactive()
+        {
+            lock (_stateLock)
+            {
+                LastSeenUtc = DateTime.UtcNow;
+
+                if (Status == HttpConnectionStatus.Active)
+                {
+                    Status = HttpConnectionStatus.Inactive;
+                }
+            }
         }
 
         private static class Log