From 29acebc150367e1bbda5d68a3da1842d2b318c63 Mon Sep 17 00:00:00 2001 From: Peter Jannesen Date: Fri, 13 Sep 2024 20:05:07 +0200 Subject: [PATCH] HttpListener fix: Operations that change non-concurrent collections must have exclusive access --- .../Net/Windows/HttpListener.Windows.cs | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs index 9315b0c511fb8e..00932b43ec228f 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs @@ -70,7 +70,9 @@ public bool UnsafeConnectionNtlmAuthentication { return; } - lock ((DisconnectResults as ICollection).SyncRoot) + + var disconnectResults = DisconnectResults; + lock ((disconnectResults as ICollection).SyncRoot) { if (_unsafeConnectionNtlmAuthentication == value) { @@ -79,7 +81,7 @@ public bool UnsafeConnectionNtlmAuthentication _unsafeConnectionNtlmAuthentication = value; if (!value) { - foreach (DisconnectAsyncResult result in DisconnectResults.Values) + foreach (DisconnectAsyncResult result in disconnectResults.Values) { result.AuthenticatedConnection = null; } @@ -694,7 +696,13 @@ public HttpListenerContext EndGetContext(IAsyncResult asyncResult) // assurance that we do this only for NTLM/Negotiate is not here, but in the // code that caches WindowsIdentity instances in the Dictionary. DisconnectAsyncResult? disconnectResult; - DisconnectResults.TryGetValue(connectionId, out disconnectResult); + + var disconnectResults = DisconnectResults; + lock ((disconnectResults as ICollection).SyncRoot) + { + disconnectResults.TryGetValue(connectionId, out disconnectResult); + } + if (UnsafeConnectionNtlmAuthentication) { if (authorizationHeader == null) @@ -1327,7 +1335,12 @@ private static void RegisterForDisconnectNotification(HttpListenerSession sessio // Need to make sure it's going to get returned before adding it to the hash. That way it'll be handled // correctly in HandleAuthentication's finally. disconnectResult = result; - session.Listener.DisconnectResults[connectionId] = disconnectResult; + + var disconnectResults = session.Listener.DisconnectResults; + lock ((disconnectResults as ICollection).SyncRoot) + { + disconnectResults[connectionId] = disconnectResult; + } } if (statusCode == Interop.HttpApi.ERROR_SUCCESS && HttpListener.SkipIOCPCallbackOnSuccess) @@ -1646,8 +1659,21 @@ private void HandleDisconnect() { HttpListener listener = _listenerSession.Listener; - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"DisconnectResults {listener.DisconnectResults} removing for _connectionId: {_connectionId}"); - listener.DisconnectResults.Remove(_connectionId); + var disconnectResults = listener.DisconnectResults; + if (NetEventSource.Log.IsEnabled()) + { + string? results; + lock ((disconnectResults as ICollection).SyncRoot) + { + results = disconnectResults.ToString(); + } + NetEventSource.Info(this, $"DisconnectResults {results} removing for _connectionId: {_connectionId}"); + } + + lock ((disconnectResults as ICollection).SyncRoot) + { + disconnectResults.Remove(_connectionId); + } // Cached identity is disposed with the session context Session?.Dispose();