HttpListener.Stop closes the request queue handle with CloseIoEx method (#684)

Currently, if HttpListener.Stop is called while the other thread is blocked inside a synchronous GetContext cal, it has no effect because Stop cannot dispose the request queue handle. This is caused by a reference counting mechanism implemented in SafeHandle which prevents disposal of the native handle due to the following reason.

GetContext method invokes the native HttpReceiveHttpRequest to start listening for incoming requests. HttpReceiveHttpRequest is called through P/Invoke and accepts SafeHandle as a request queue handle. In such case, runtime injects a special call to SafeHandle.DangerousAddRef incrementing the ref counter to prevent an accidental disposal from the managed side. In general, it's a good safety measure, but in the given scenario it prevents SafeHandle.Dispose from actually closing the underlying OS handle thus GetContext gets blocked until a next request arrives.

To overcome this limitation, PR adds a call to CloseIoEx before invoking Dispose to close the request queue and unblock GetContext.
Fixes dotnet/corefx#28169

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 08fe539..7d98e40 100644 (file)
--- 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
@@ -451,6 +451,18 @@ namespace System.Net
                 if (NetEventSource.IsEnabled) NetEventSource.Info($"Dispose ThreadPoolBoundHandle: {_requestQueueBoundHandle}");
                 _requestQueueBoundHandle?.Dispose();
                 _requestQueueHandle.Dispose();
+
+                // CancelIoEx is called after Dispose to prevent a race condition involving parallel GetContext and
+                // HttpReceiveHttpRequest calls. Otherwise, calling CancelIoEx before Dispose might block the synchronous
+                // GetContext call until the next request arrives.
+                try
+                {
+                    Interop.Kernel32.CancelIoEx(_requestQueueHandle, null); // This cancels the synchronous call to HttpReceiveHttpRequest
+                }
+                catch (ObjectDisposedException)
+                {
+                    // Ignore the exception since it only means that the queue handle has been successfully disposed
+                }
             }
         }
 

diff --git a/src/libraries/System.Net.HttpListener/tests/HttpListenerTests.cs b/src/libraries/System.Net.HttpListener/tests/HttpListenerTests.cs
index 1a6e963..cf22e8c 100644 (file)
--- a/src/libraries/System.Net.HttpListener/tests/HttpListenerTests.cs
+++ b/src/libraries/System.Net.HttpListener/tests/HttpListenerTests.cs
@@ -6,6 +6,7 @@ using System.Net.Http;
 using System.Net.Sockets;
 using System.Security.Authentication.ExtendedProtection;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -155,5 +156,19 @@ namespace System.Net.Tests
                 Assert.Throws<InvalidOperationException>(() => listener.EndGetContext(beginGetContextResult));
             }
         }
+
+        [Fact]
+        [OuterLoop]
+        public async Task GetContext_StopIsCalled_GetContextUnblocked()
+        {
+            using var listenerFactory = new HttpListenerFactory();
+            var listener = listenerFactory.GetListener();
+            listener.Start();
+            var listenerTask = Task.Run(() => Assert.Throws<HttpListenerException>(() => listener.GetContext()));
+            await Task.Delay(1000).TimeoutAfter(10000); // Wait for listenerTask to call GetContext.
+            listener.Stop();
+            listener.Close();
+            await listenerTask.TimeoutAfter(10000);
+        }
     }
 }