Fix WebSocketHttpListenerDuplexStream error handling (dotnet/corefx#36110)

An unobserved websocket task exception was happening when attempting to read/write on
non-existent connections. This code originally came from .NET Framework. It is specific
to Windows only and uses HttpListener as the server for the WebSocket. I was able to
reproduce the problem on .NET Framework as well.

The problem was due to the logic in WebSocketHttpListenerDuplexStream where it was always
creating a TaskCompletionSource and doing a TrySetException even if the error occured on
a synchronous codepath. The ReadAsyncFast (and WriteAsyncFast) methods are always calling
a callback to finish the operation and thus always setting an exception into the task. But
the task was never awaited because the exception was also rethrown from the ReadAsyncFast
method and skips the await.

The fix is to change the logic slightly so that ReadAsyncFast won't rethrow the exception.
It doesn't need to since the exception is already stored in the task. So, awaiting that task
will be fast since it is already completed (faulted with the exception).

In addition to the CI tests, I also verified the fix with the original repro from the issue.

Fixes dotnet/corefx#29005

Commit migrated from https://github.com/dotnet/corefx/commit/3633ea2c6bf9d52029681efeedd84fd7a06eb6ba

diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs
index 21d624e..40d0ef8 100644 (file)
--- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs
+++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs
@@ -197,7 +197,7 @@ namespace System.Net.WebSockets
 
         // return value indicates sync vs async completion
         // false: sync completion
-        // true: async completion
+        // true: async completion or error
         private unsafe bool ReadAsyncFast(HttpListenerAsyncEventArgs eventArgs)
         {
             if (NetEventSource.IsEnabled)
@@ -209,7 +209,7 @@ namespace System.Net.WebSockets
             eventArgs.StartOperationReceive();
 
             uint statusCode = 0;
-            bool completedAsynchronously = false;
+            bool completedAsynchronouslyOrWithError = false;
             try
             {
                 Debug.Assert(eventArgs.Buffer != null, "'BufferList' is not supported for read operations.");
@@ -278,11 +278,11 @@ namespace System.Net.WebSockets
                     // IO operation completed synchronously. No IO completion port callback is used because 
                     // it was disabled in SwitchToOpaqueMode()
                     eventArgs.FinishOperationSuccess((int)bytesReturned, true);
-                    completedAsynchronously = false;
+                    completedAsynchronouslyOrWithError = false;
                 }
                 else
                 {
-                    completedAsynchronously = true;
+                    completedAsynchronouslyOrWithError = true;
                 }
             }
             catch (Exception e)
@@ -291,17 +291,17 @@ namespace System.Net.WebSockets
                 _outputStream.SetClosedFlag();
                 _outputStream.InternalHttpContext.Abort();
 
-                throw;
+                completedAsynchronouslyOrWithError = true;
             }
             finally
             {
                 if (NetEventSource.IsEnabled)
                 {
-                    NetEventSource.Exit(this, completedAsynchronously);
+                    NetEventSource.Exit(this, completedAsynchronouslyOrWithError);
                 }
             }
 
-            return completedAsynchronously;
+            return completedAsynchronouslyOrWithError;
         }
 
         public override int ReadByte()
@@ -472,7 +472,7 @@ namespace System.Net.WebSockets
 
         // return value indicates sync vs async completion
         // false: sync completion
-        // true: async completion
+        // true: async completion or with error
         private unsafe bool WriteAsyncFast(HttpListenerAsyncEventArgs eventArgs)
         {
             if (NetEventSource.IsEnabled)
@@ -486,7 +486,7 @@ namespace System.Net.WebSockets
             eventArgs.StartOperationSend();
 
             uint statusCode;
-            bool completedAsynchronously = false;
+            bool completedAsynchronouslyOrWithError = false;
             try
             {
                 if (_outputStream.Closed ||
@@ -533,11 +533,11 @@ namespace System.Net.WebSockets
                 {
                     // IO operation completed synchronously - callback won't be called to signal completion.
                     eventArgs.FinishOperationSuccess((int)bytesSent, true);
-                    completedAsynchronously = false;
+                    completedAsynchronouslyOrWithError = false;
                 }
                 else
                 {
-                    completedAsynchronously = true;
+                    completedAsynchronouslyOrWithError = true;
                 }
             }
             catch (Exception e)
@@ -546,17 +546,17 @@ namespace System.Net.WebSockets
                 _outputStream.SetClosedFlag();
                 _outputStream.InternalHttpContext.Abort();
 
-                throw;
+                completedAsynchronouslyOrWithError = true;
             }
             finally
             {
                 if (NetEventSource.IsEnabled)
                 {
-                    NetEventSource.Exit(this, completedAsynchronously);
+                    NetEventSource.Exit(this, completedAsynchronouslyOrWithError);
                 }
             }
 
-            return completedAsynchronously;
+            return completedAsynchronouslyOrWithError;
         }
 
         public override void WriteByte(byte value)