From: David Shulman Date: Mon, 18 Mar 2019 00:13:31 +0000 (-0700) Subject: Fix WebSocketHttpListenerDuplexStream error handling (dotnet/corefx#36110) X-Git-Tag: submit/tizen/20210909.063632~11031^2~2157 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c5f2e8c2ea0dd82725842c15d667aa0c6cbf7333;p=platform%2Fupstream%2Fdotnet%2Fruntime.git 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 --- 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)