From 524ea303b3793a8e570bcf3ebb4159a3bf7e82bc Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Thu, 11 Jan 2024 16:57:39 +0100 Subject: [PATCH 1/9] rebase --- .../Net/WebSockets/WebSocketValidate.cs | 11 +- .../Handlers/EchoWebSocketHandler.cs | 17 +- .../BrowserWebSockets/BrowserInterop.cs | 42 +- .../BrowserWebSockets/BrowserWebSocket.cs | 632 +++++++++--------- .../tests/CloseTest.cs | 73 +- .../tests/SendReceiveTest.cs | 4 +- .../System.Net.WebSockets.Client.Tests.csproj | 3 - ...me.InteropServices.JavaScript.Tests.csproj | 1 + .../JavaScript/WebWorkerTest.cs | 97 +++ src/mono/browser/runtime/web-socket.ts | 34 +- 10 files changed, 539 insertions(+), 375 deletions(-) diff --git a/src/libraries/Common/src/System/Net/WebSockets/WebSocketValidate.cs b/src/libraries/Common/src/System/Net/WebSockets/WebSocketValidate.cs index 7c97c082d26f89..e087677be4608e 100644 --- a/src/libraries/Common/src/System/Net/WebSockets/WebSocketValidate.cs +++ b/src/libraries/Common/src/System/Net/WebSockets/WebSocketValidate.cs @@ -23,10 +23,15 @@ internal static partial class WebSocketValidate internal const int MaxDeflateWindowBits = 15; internal const int MaxControlFramePayloadLength = 123; +#if TARGET_BROWSER + private const int ValidCloseStatusCodesFrom = 3000; + private const int ValidCloseStatusCodesTo = 4999; +#else private const int CloseStatusCodeAbort = 1006; private const int CloseStatusCodeFailedTLSHandshake = 1015; private const int InvalidCloseStatusCodesFrom = 0; private const int InvalidCloseStatusCodesTo = 999; +#endif // [0x21, 0x7E] except separators "()<>@,;:\\\"/[]?={} ". private static readonly SearchValues s_validSubprotocolChars = @@ -84,11 +89,15 @@ internal static void ValidateCloseStatus(WebSocketCloseStatus closeStatus, strin } int closeStatusCode = (int)closeStatus; - +#if TARGET_BROWSER + // as defined in https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/close#code + if (closeStatus != WebSocketCloseStatus.NormalClosure && (closeStatusCode < ValidCloseStatusCodesFrom || closeStatusCode > ValidCloseStatusCodesTo)) +#else if ((closeStatusCode >= InvalidCloseStatusCodesFrom && closeStatusCode <= InvalidCloseStatusCodesTo) || closeStatusCode == CloseStatusCodeAbort || closeStatusCode == CloseStatusCodeFailedTLSHandshake) +#endif { // CloseStatus 1006 means Aborted - this will never appear on the wire and is reflected by calling WebSocket.Abort throw new ArgumentException(SR.Format(SR.net_WebSockets_InvalidCloseStatusCode, diff --git a/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoWebSocketHandler.cs b/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoWebSocketHandler.cs index f4e5562600015d..8304f2d1156072 100644 --- a/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoWebSocketHandler.cs +++ b/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoWebSocketHandler.cs @@ -24,11 +24,11 @@ public static async Task InvokeAsync(HttpContext context) if (context.Request.QueryString.HasValue && context.Request.QueryString.Value.Contains("delay10sec")) { - Thread.Sleep(10000); + await Task.Delay(10000); } else if (context.Request.QueryString.HasValue && context.Request.QueryString.Value.Contains("delay20sec")) { - Thread.Sleep(20000); + await Task.Delay(20000); } try @@ -124,14 +124,15 @@ await socket.CloseAsync( } bool sendMessage = false; + string receivedMessage = null; if (receiveResult.MessageType == WebSocketMessageType.Text) { - string receivedMessage = Encoding.UTF8.GetString(receiveBuffer, 0, offset); + receivedMessage = Encoding.UTF8.GetString(receiveBuffer, 0, offset); if (receivedMessage == ".close") { await socket.CloseAsync(WebSocketCloseStatus.NormalClosure, receivedMessage, CancellationToken.None); } - if (receivedMessage == ".shutdown") + else if (receivedMessage == ".shutdown") { await socket.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, receivedMessage, CancellationToken.None); } @@ -161,6 +162,14 @@ await socket.SendAsync( !replyWithPartialMessages, CancellationToken.None); } + if (receivedMessage == ".closeafter") + { + await socket.CloseAsync(WebSocketCloseStatus.NormalClosure, receivedMessage, CancellationToken.None); + } + else if (receivedMessage == ".shutdownafter") + { + await socket.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, receivedMessage, CancellationToken.None); + } } } } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs index a03a0d6941190d..3382103b404f78 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs @@ -16,6 +16,24 @@ internal static partial class BrowserInterop return protocol; } + public static WebSocketCloseStatus? GetCloseStatus(JSObject? webSocket) + { + if (webSocket == null || webSocket.IsDisposed) return null; + if (!webSocket.HasProperty("close_status")) + { + return null; + } + int status = webSocket.GetPropertyAsInt32("close_status"); + return (WebSocketCloseStatus)status; + } + + public static string? GetCloseStatusDescription(JSObject? webSocket) + { + if (webSocket == null || webSocket.IsDisposed) return null; + string? description = webSocket.GetPropertyAsString("close_status_description"); + return description; + } + public static int GetReadyState(JSObject? webSocket) { if (webSocket == null || webSocket.IsDisposed) return -1; @@ -28,16 +46,14 @@ public static int GetReadyState(JSObject? webSocket) public static partial JSObject WebSocketCreate( string uri, string?[]? subProtocols, - IntPtr responseStatusPtr, - [JSMarshalAs>] Action onClosed); + IntPtr responseStatusPtr); public static unsafe JSObject UnsafeCreate( string uri, string?[]? subProtocols, - MemoryHandle responseHandle, - [JSMarshalAs>] Action onClosed) + MemoryHandle responseHandle) { - return WebSocketCreate(uri, subProtocols, (IntPtr)responseHandle.Pointer, onClosed); + return WebSocketCreate(uri, subProtocols, (IntPtr)responseHandle.Pointer); } [JSImport("INTERNAL.ws_wasm_open")] @@ -52,19 +68,9 @@ public static partial Task WebSocketOpen( int messageType, bool endOfMessage); - public static unsafe Task? UnsafeSendSync(JSObject jsWs, ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage) + public static unsafe Task? UnsafeSend(JSObject jsWs, MemoryHandle pinBuffer, int length, WebSocketMessageType messageType, bool endOfMessage) { - if (buffer.Count == 0) - { - return WebSocketSend(jsWs, IntPtr.Zero, 0, (int)messageType, endOfMessage); - } - - var span = buffer.AsSpan(); - // we can do this because the bytes in the buffer are always consumed synchronously (not later with Task resolution) - fixed (void* spanPtr = span) - { - return WebSocketSend(jsWs, (IntPtr)spanPtr, buffer.Count, (int)messageType, endOfMessage); - } + return WebSocketSend(jsWs, (IntPtr)pinBuffer.Pointer, length, (int)messageType, endOfMessage); } [JSImport("INTERNAL.ws_wasm_receive")] @@ -73,7 +79,7 @@ public static partial Task WebSocketOpen( IntPtr bufferPtr, int bufferLength); - public static unsafe Task? ReceiveUnsafeSync(JSObject jsWs, MemoryHandle pinBuffer, int length) + public static unsafe Task? ReceiveUnsafe(JSObject jsWs, MemoryHandle pinBuffer, int length) { return WebSocketReceive(jsWs, (IntPtr)pinBuffer.Pointer, length); } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 68120a5cf8f7ef..8c588fcaeca9a6 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -14,16 +14,17 @@ namespace System.Net.WebSockets /// internal sealed class BrowserWebSocket : WebSocket { -#if FEATURE_WASM_THREADS - private readonly object _thisLock = new object(); -#endif + private readonly object _lockObject = new object(); private WebSocketCloseStatus? _closeStatus; private string? _closeStatusDescription; private JSObject? _innerWebSocket; private WebSocketState _state; + private bool _closeSent; + private bool _closeReceived; private bool _disposed; private bool _aborted; + private bool _cancelled; private int[] responseStatus = new int[3]; private MemoryHandle? responseStatusHandle; @@ -33,29 +34,30 @@ public override WebSocketState State { get { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif - if (_innerWebSocket == null || _disposed || (_state != WebSocketState.Connecting && _state != WebSocketState.Open && _state != WebSocketState.CloseSent)) + if (_innerWebSocket == null || _disposed || _state == WebSocketState.Aborted || _state == WebSocketState.Closed) { return _state; } -#if FEATURE_WASM_THREADS - } //lock -#endif - -#if FEATURE_WASM_THREADS - return FastState = _innerWebSocket!.SynchronizationContext.Send(static (BrowserWebSocket self) => - { - lock (self._thisLock) + var st = GetReadyState(_innerWebSocket!); + if (st == WebSocketState.Closed || st == WebSocketState.CloseSent) { - return GetReadyState(self._innerWebSocket!); - } //lock - }, this); -#else - return FastState = GetReadyState(_innerWebSocket!); -#endif + if (_closeReceived && _closeSent) + { + st = WebSocketState.Closed; + } + else if (_closeReceived && !_closeSent) + { + st = WebSocketState.CloseReceived; + } + else if (!_closeReceived && _closeSent) + { + st = WebSocketState.CloseSent; + } + } + return FastState = st; + } // lock } } @@ -63,50 +65,68 @@ private WebSocketState FastState { get { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif return _state; -#if FEATURE_WASM_THREADS - } //lock -#endif + } // lock } set { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif _state = value; -#if FEATURE_WASM_THREADS - } //lock -#endif + } // lock } } - public override WebSocketCloseStatus? CloseStatus => _closeStatus; - public override string? CloseStatusDescription => _closeStatusDescription; - public override string? SubProtocol + public override WebSocketCloseStatus? CloseStatus { get { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif - ThrowIfDisposed(); - if (_innerWebSocket == null) throw new InvalidOperationException(SR.net_WebSockets_NotConnected); -#if FEATURE_WASM_THREADS - } //lock -#endif + if (_closeStatus != null) + { + return _closeStatus; + } + if (_disposed || _aborted || _cancelled) + { + return null; + } + return GetCloseStatus(); + } + } + } -#if FEATURE_WASM_THREADS - return _innerWebSocket.SynchronizationContext.Send(BrowserInterop.GetProtocol, _innerWebSocket); -#else - return BrowserInterop.GetProtocol(_innerWebSocket); -#endif + public override string? CloseStatusDescription + { + get + { + lock (_lockObject) + { + if (_closeStatusDescription != null) + { + return _closeStatusDescription; + } + if (_disposed || _aborted || _cancelled) + { + return null; + } + return GetCloseStatusDescription(); + } + } + } + public override string? SubProtocol + { + get + { + ThrowIfDisposed(); + lock (_lockObject) + { + if (_innerWebSocket == null) throw new InvalidOperationException(SR.net_WebSockets_NotConnected); + return BrowserInterop.GetProtocol(_innerWebSocket); + } // lock } } @@ -114,41 +134,26 @@ public override string? SubProtocol internal Task ConnectAsync(Uri uri, List? requestedSubProtocols, CancellationToken cancellationToken) { - ThrowIfDisposed(); - if (FastState != WebSocketState.None) - { - throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted); - } -#if FEATURE_WASM_THREADS - JSHost.CurrentOrMainJSSynchronizationContext.Send(_ => + lock (_lockObject) { - lock (_thisLock) + if (FastState != WebSocketState.None) { - ThrowIfDisposed(); - FastState = WebSocketState.Connecting; - CreateCore(uri, requestedSubProtocols); + throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted); } - }, null); - - return JSHost.CurrentOrMainJSSynchronizationContext.Post(() => - { - return ConnectAsyncCore(cancellationToken); - }); -#else - FastState = WebSocketState.Connecting; + ThrowIfDisposed(); + FastState = WebSocketState.Connecting; + } // lock CreateCore(uri, requestedSubProtocols); return ConnectAsyncCore(cancellationToken); -#endif } public override Task SendAsync(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); - // fast check of previous _state instead of GetReadyState(), the readyState would be validated on JS side - if (FastState != WebSocketState.Open) + var fastState = FastState; + if (fastState != WebSocketState.Open && fastState != WebSocketState.CloseReceived) { throw new InvalidOperationException(SR.net_WebSockets_NotConnected); } @@ -166,21 +171,8 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); -#if FEATURE_WASM_THREADS - return _innerWebSocket!.SynchronizationContext.Post(() => - { - Task promise; - lock (_thisLock) - { - ThrowIfDisposed(); - promise = SendAsyncCore(buffer, messageType, endOfMessage, cancellationToken); - } //lock will unlock synchronously before promise is resolved! - - return promise; - }); -#else - return SendAsyncCore(buffer, messageType, endOfMessage, cancellationToken); -#endif + ThrowIfDisposed(); + return SendAsyncCore(buffer, messageType, endOfMessage, cancellationToken, fastState); } public override Task ReceiveAsync(ArraySegment buffer, CancellationToken cancellationToken) @@ -189,7 +181,7 @@ public override Task ReceiveAsync(ArraySegment buf { return Task.FromException(new OperationCanceledException(cancellationToken)); } - ThrowIfDisposed(); + // fast check of previous _state instead of GetReadyState(), the readyState would be validated on JS side var fastState = FastState; if (fastState != WebSocketState.Open && fastState != WebSocketState.CloseSent) @@ -199,26 +191,13 @@ public override Task ReceiveAsync(ArraySegment buf WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); -#if FEATURE_WASM_THREADS - return _innerWebSocket!.SynchronizationContext.Post(() => - { - Task promise; - lock (_thisLock) - { - ThrowIfDisposed(); - promise = ReceiveAsyncCore(buffer, cancellationToken); - } //lock will unlock synchronously before task is resolved! - return promise; - }); -#else - return ReceiveAsyncCore(buffer, cancellationToken); -#endif + ThrowIfDisposed(); + return ReceiveAsyncCore(buffer, cancellationToken, fastState); } public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); var fastState = FastState; @@ -227,37 +206,24 @@ public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, fastState, "Connecting, Open, CloseSent, Aborted")); } -#if FEATURE_WASM_THREADS - return _innerWebSocket!.SynchronizationContext.Post(() => + lock (_lockObject) { - Task promise; - lock (_thisLock) + var state = State; + if (state == WebSocketState.None || state == WebSocketState.Closed) { - ThrowIfDisposed(); -#endif - var state = State; - if (state == WebSocketState.None || state == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); - } - if (state != WebSocketState.Open && state != WebSocketState.Connecting && state != WebSocketState.Aborted) - { - return Task.CompletedTask; - } -#if FEATURE_WASM_THREADS - promise = CloseAsyncCore(closeStatus, statusDescription, false, cancellationToken); - } //lock will unlock synchronously before task is resolved! - return promise; - }); -#else - return CloseAsyncCore(closeStatus, statusDescription, false, cancellationToken); -#endif + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); + } + if (_closeSent) + { + return Task.CompletedTask; + } + } // lock + return CloseAsyncCore(closeStatus, statusDescription, false, cancellationToken, fastState); } public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - ThrowIfDisposed(); WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); @@ -266,118 +232,82 @@ public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? status { throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, fastState, "Connecting, Open, CloseSent, Aborted")); } - -#if FEATURE_WASM_THREADS - return _innerWebSocket!.SynchronizationContext.Post(() => + WebSocketState state; + lock (_lockObject) { - Task promise; - lock (_thisLock) + state = State; + if (state == WebSocketState.Closed) { - ThrowIfDisposed(); -#endif - var state = State; - if (state == WebSocketState.None || state == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); - } - if (state != WebSocketState.Open && state != WebSocketState.Connecting && state != WebSocketState.Aborted && state != WebSocketState.CloseSent) - { - return Task.CompletedTask; - } + return Task.CompletedTask; + } + if (state == WebSocketState.None || state == WebSocketState.Closed) + { + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); + } -#if FEATURE_WASM_THREADS - promise = CloseAsyncCore(closeStatus, statusDescription, state != WebSocketState.Aborted, cancellationToken); - } //lock will unlock synchronously before task is resolved! - return promise; - }); -#else - return CloseAsyncCore(closeStatus, statusDescription, state != WebSocketState.Aborted, cancellationToken); -#endif + } // lock + return CloseAsyncCore(closeStatus, statusDescription, state != WebSocketState.Aborted, cancellationToken, fastState); } public override void Abort() { -#if FEATURE_WASM_THREADS - if (_disposed) + WebSocketState state; + lock (_lockObject) { - return; - } - _innerWebSocket?.SynchronizationContext.Send(static (BrowserWebSocket self) => - { - lock (self._thisLock) + if (_disposed) { - AbortCore(self, self.State); + return; } - }, this); -#else - AbortCore(this, this.State); -#endif + state = State; + } // lock + AbortCore(this, state); } public override void Dispose() { -#if FEATURE_WASM_THREADS - if (_disposed) - { - return; - } - _innerWebSocket?.SynchronizationContext.Send(static (BrowserWebSocket self) => + WebSocketState state; + lock (_lockObject) { - lock (self._thisLock) + if (_disposed) { - DisposeCore(self); + return; } - }, this); -#else - DisposeCore(this); -#endif - } + _disposed = true; + state = State; + } // lock - private void ThrowIfDisposed() - { -#if FEATURE_WASM_THREADS - lock (_thisLock) + if (state < WebSocketState.Closed && state != WebSocketState.None) { -#endif - ObjectDisposedException.ThrowIf(_disposed, this); -#if FEATURE_WASM_THREADS - } //lock -#endif + AbortCore(this, state); + } + if (state != WebSocketState.Aborted) + { + FastState = WebSocketState.Closed; + } + _innerWebSocket?.Dispose(); + responseStatusHandle?.Dispose(); } - #region methods always called on one thread only, exclusively - - private static void DisposeCore(BrowserWebSocket self) + private void ThrowIfDisposed() { - if (!self._disposed) + lock (_lockObject) { - var state = self.State; - self._disposed = true; - if (state < WebSocketState.Aborted && state != WebSocketState.None) - { - AbortCore(self, state); - } - if (self.FastState != WebSocketState.Aborted) - { - self.FastState = WebSocketState.Closed; - } - self._innerWebSocket?.Dispose(); - self._innerWebSocket = null; - self.responseStatusHandle?.Dispose(); - } + ObjectDisposedException.ThrowIf(_disposed, this); + } // lock } private static void AbortCore(BrowserWebSocket self, WebSocketState currentState) { - if (!self._disposed && currentState != WebSocketState.Closed) + lock (self._lockObject) { self.FastState = WebSocketState.Aborted; self._aborted = true; - if (self._innerWebSocket != null) + if (self._disposed || currentState == WebSocketState.Closed || currentState == WebSocketState.Aborted) { - BrowserInterop.WebSocketAbort(self._innerWebSocket!); + return; } } + BrowserInterop.WebSocketAbort(self._innerWebSocket!); } private void CreateCore(Uri uri, List? requestedSubProtocols) @@ -385,28 +315,11 @@ private void CreateCore(Uri uri, List? requestedSubProtocols) try { string[]? subProtocols = requestedSubProtocols?.ToArray(); - var onClose = (int code, string reason) => - { - _closeStatus = (WebSocketCloseStatus)code; - _closeStatusDescription = reason; -#if FEATURE_WASM_THREADS - lock (_thisLock) - { -#endif - WebSocketState state = State; - if (state == WebSocketState.Connecting || state == WebSocketState.Open || state == WebSocketState.CloseSent) - { - FastState = WebSocketState.Closed; - } -#if FEATURE_WASM_THREADS - } //lock -#endif - }; Memory responseMemory = new Memory(responseStatus); responseStatusHandle = responseMemory.Pin(); - _innerWebSocket = BrowserInterop.UnsafeCreate(uri.ToString(), subProtocols, responseStatusHandle.Value, onClose); + _innerWebSocket = BrowserInterop.UnsafeCreate(uri.ToString(), subProtocols, responseStatusHandle.Value); } catch (Exception) { @@ -417,51 +330,42 @@ private void CreateCore(Uri uri, List? requestedSubProtocols) private async Task ConnectAsyncCore(CancellationToken cancellationToken) { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif if (_aborted) { FastState = WebSocketState.Closed; throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure); } ThrowIfDisposed(); -#if FEATURE_WASM_THREADS - } //lock -#endif + } // lock try { var openTask = BrowserInterop.WebSocketOpen(_innerWebSocket!); - await CancelationHelper(openTask!, cancellationToken, FastState).ConfigureAwait(true); -#if FEATURE_WASM_THREADS - lock (_thisLock) + await CancellationHelper(openTask!, cancellationToken, WebSocketState.Connecting).ConfigureAwait(false); + + lock (_lockObject) { -#endif - if (State == WebSocketState.Connecting) + WebSocketState state = State; + if (state == WebSocketState.Connecting) { FastState = WebSocketState.Open; } -#if FEATURE_WASM_THREADS - } //lock -#endif + } // lock } catch (OperationCanceledException ex) { -#if FEATURE_WASM_THREADS - lock (_thisLock) + lock (_lockObject) { -#endif FastState = WebSocketState.Closed; if (_aborted) { throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, ex); } -#if FEATURE_WASM_THREADS - } //lock -#endif + } // lock + throw; } catch (Exception) @@ -471,124 +375,159 @@ private async Task ConnectAsyncCore(CancellationToken cancellationToken) } } - private async Task SendAsyncCore(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken) + private async Task SendAsyncCore(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken, WebSocketState previousState) { try { - var sendTask = BrowserInterop.UnsafeSendSync(_innerWebSocket!, buffer, messageType, endOfMessage); - if (sendTask != null) + Task? sendTask; + if (buffer.Count == 0) { - await CancelationHelper(sendTask, cancellationToken, FastState).ConfigureAwait(true); + sendTask = BrowserInterop.WebSocketSend(_innerWebSocket!, IntPtr.Zero, 0, (int)messageType, endOfMessage); + if (sendTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + { + await CancellationHelper(sendTask, cancellationToken, previousState).ConfigureAwait(false); + } } -#if FEATURE_WASM_THREADS - // return synchronously, not supported with MT else { - Environment.FailFast("BrowserWebSocket.SendAsyncCore: Unexpected synchronous result"); + Memory bufferMemory = buffer.AsMemory(); + MemoryHandle pinBuffer = bufferMemory.Pin(); + try + { + sendTask = BrowserInterop.UnsafeSend(_innerWebSocket!, pinBuffer, bufferMemory.Length, messageType, endOfMessage); + if (sendTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + { + await CancellationHelper(sendTask, cancellationToken, previousState, pinBuffer).ConfigureAwait(false); + } + } + finally + { + // must be after await! + pinBuffer.Dispose(); + } } -#endif } catch (JSException ex) { if (ex.Message.StartsWith("InvalidState:")) { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, State, "Open"), ex); + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, FastState, "Open"), ex); } throw new WebSocketException(WebSocketError.NativeError, ex); } } - private async Task ReceiveAsyncCore(ArraySegment buffer, CancellationToken cancellationToken) + private async Task ReceiveAsyncCore(ArraySegment buffer, CancellationToken cancellationToken, WebSocketState previousState) { + Memory bufferMemory = buffer.AsMemory(); + MemoryHandle pinBuffer = bufferMemory.Pin(); try { - Memory bufferMemory = buffer.AsMemory(); - using (MemoryHandle pinBuffer = bufferMemory.Pin()) - { - var receiveTask = BrowserInterop.ReceiveUnsafeSync(_innerWebSocket!, pinBuffer, bufferMemory.Length); - if (receiveTask != null) - { - await CancelationHelper(receiveTask, cancellationToken, FastState).ConfigureAwait(true); - } -#if FEATURE_WASM_THREADS - // return synchronously, not supported with MT - else - { - Environment.FailFast("BrowserWebSocket.ReceiveAsyncCore: Unexpected synchronous result"); - } -#endif - + var receiveTask = BrowserInterop.ReceiveUnsafe(_innerWebSocket!, pinBuffer, bufferMemory.Length); -#if FEATURE_WASM_THREADS - lock (_thisLock) - { -#endif - return ConvertResponse(this); -#if FEATURE_WASM_THREADS - } //lock -#endif + if (receiveTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + { + await CancellationHelper(receiveTask, cancellationToken, previousState, pinBuffer).ConfigureAwait(false); } + + return ConvertResponse(); } catch (JSException ex) { if (ex.Message.StartsWith("InvalidState:")) { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, State, "Open, CloseSent"), ex); + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, previousState, "Open, CloseSent"), ex); } throw new WebSocketException(WebSocketError.NativeError, ex); } + finally + { + // must be after await! + pinBuffer.Dispose(); + } } - private static WebSocketReceiveResult ConvertResponse(BrowserWebSocket self) + private WebSocketReceiveResult ConvertResponse() { const int countIndex = 0; const int typeIndex = 1; const int endIndex = 2; - WebSocketMessageType messageType = (WebSocketMessageType)self.responseStatus[typeIndex]; + int count; + WebSocketMessageType messageType; + bool isEnd = responseStatus[endIndex] != 0; + lock (_lockObject) + { + messageType = (WebSocketMessageType)responseStatus[typeIndex]; + count = responseStatus[countIndex]; + if (messageType == WebSocketMessageType.Close) + { + _closeReceived = true; + FastState = _closeSent ? WebSocketState.Closed : WebSocketState.CloseReceived; + } + } // lock + if (messageType == WebSocketMessageType.Close) { - return new WebSocketReceiveResult(self.responseStatus[countIndex], messageType, self.responseStatus[endIndex] != 0, self.CloseStatus, self.CloseStatusDescription); + ForceReadCloseStatus(); + switch (_closeStatus ?? WebSocketCloseStatus.NormalClosure) + { + case WebSocketCloseStatus.NormalClosure: + case WebSocketCloseStatus.Empty: + return new WebSocketReceiveResult(count, messageType, isEnd, _closeStatus, _closeStatusDescription); + case WebSocketCloseStatus.InvalidMessageType: + case WebSocketCloseStatus.InvalidPayloadData: + throw new WebSocketException(WebSocketError.InvalidMessageType, _closeStatusDescription); + case WebSocketCloseStatus.EndpointUnavailable: + throw new WebSocketException(WebSocketError.NotAWebSocket, _closeStatusDescription); + case WebSocketCloseStatus.ProtocolError: + throw new WebSocketException(WebSocketError.UnsupportedProtocol, _closeStatusDescription); + case WebSocketCloseStatus.InternalServerError: + throw new WebSocketException(WebSocketError.Faulted, _closeStatusDescription); + default: + throw new WebSocketException(WebSocketError.NativeError, (int)_closeStatus!.Value, _closeStatusDescription); + } } - return new WebSocketReceiveResult(self.responseStatus[countIndex], messageType, self.responseStatus[endIndex] != 0); + return new WebSocketReceiveResult(count, messageType, isEnd); } - private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, bool waitForCloseReceived, CancellationToken cancellationToken) + private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, bool waitForCloseReceived, CancellationToken cancellationToken, WebSocketState previousState) { - _closeStatus = closeStatus; - _closeStatusDescription = statusDescription; - - var closeTask = BrowserInterop.WebSocketClose(_innerWebSocket!, (int)closeStatus, statusDescription, waitForCloseReceived); - if (closeTask != null) + lock (_lockObject) { - await CancelationHelper(closeTask, cancellationToken, FastState).ConfigureAwait(true); + if (!_closeReceived) + { + _closeStatus = closeStatus; + _closeStatusDescription = statusDescription; + } + _closeSent = true; } -#if FEATURE_WASM_THREADS - // return synchronously, not supported with MT - else + var closeTask = BrowserInterop.WebSocketClose(_innerWebSocket!, (int)closeStatus, statusDescription, waitForCloseReceived); + if (closeTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts { - Environment.FailFast("BrowserWebSocket.CloseAsyncCore: Unexpected synchronous result"); + await CancellationHelper(closeTask, cancellationToken, previousState).ConfigureAwait(false); } -#endif - -#if FEATURE_WASM_THREADS - lock (_thisLock) + if (waitForCloseReceived) { -#endif - var state = State; - if (state == WebSocketState.Open || state == WebSocketState.Connecting || state == WebSocketState.CloseSent) + lock (_lockObject) { - FastState = waitForCloseReceived ? WebSocketState.Closed : WebSocketState.CloseSent; + _closeReceived = true; + ForceReadCloseStatus(); + _ = State; } -#if FEATURE_WASM_THREADS - } //lock -#endif + } } - private async Task CancelationHelper(Task jsTask, CancellationToken cancellationToken, WebSocketState previousState) + private async Task CancellationHelper(Task promise, CancellationToken cancellationToken, WebSocketState previousState, IDisposable? disposable = null) { - if (jsTask.IsCompletedSuccessfully) + if (cancellationToken.IsCancellationRequested) { + disposable?.Dispose(); + cancellationToken.ThrowIfCancellationRequested(); + } + if (promise.IsCompletedSuccessfully) + { + disposable?.Dispose(); return; } try @@ -596,35 +535,87 @@ private async Task CancelationHelper(Task jsTask, CancellationToken cancellation using (var receiveRegistration = cancellationToken.Register(static s => { CancelablePromise.CancelPromise((Task)s!); - }, jsTask)) + }, promise)) { - await jsTask.ConfigureAwait(true); + await promise.ConfigureAwait(false); return; } } catch (JSException ex) { - if (State == WebSocketState.Aborted) + lock (_lockObject) { - throw new OperationCanceledException(nameof(WebSocketState.Aborted), ex); + var state = State; + if (state == WebSocketState.Aborted) + { + ForceReadCloseStatus(); + throw new OperationCanceledException(nameof(WebSocketState.Aborted), ex); + } + + if (state != WebSocketState.Closed && cancellationToken.IsCancellationRequested) + { + FastState = WebSocketState.Aborted; + _cancelled = true; + throw new OperationCanceledException(cancellationToken); + } + if (state != WebSocketState.Closed && ex.Message == "Error: OperationCanceledException") + { + FastState = WebSocketState.Aborted; + _cancelled = true; + throw new OperationCanceledException("The operation was cancelled.", ex, cancellationToken); + } + if (previousState == WebSocketState.Connecting) + { + ForceReadCloseStatus(); + throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, ex); + } + throw new WebSocketException(WebSocketError.NativeError, ex); } + } + finally + { + disposable?.Dispose(); + } + } - if (cancellationToken.IsCancellationRequested) + private void ForceReadCloseStatus() + { + lock (_lockObject) + { + if (!_disposed && _closeStatus == null) { - FastState = WebSocketState.Aborted; - throw new OperationCanceledException(cancellationToken); + GetCloseStatus(); + GetCloseStatusDescription(); } - if (ex.Message == "Error: OperationCanceledException") + } + } + + private WebSocketCloseStatus? GetCloseStatus() + { + ThrowIfDisposed(); + var closeStatus = BrowserInterop.GetCloseStatus(_innerWebSocket); + lock (_lockObject) + { + if (closeStatus != null && _closeStatus == null) { - FastState = WebSocketState.Aborted; - throw new OperationCanceledException("The operation was cancelled.", ex, cancellationToken); + _closeStatus = closeStatus; } - if (previousState == WebSocketState.Connecting) + } + return _closeStatus; + } + + private string? GetCloseStatusDescription() + { + ThrowIfDisposed(); + var closeStatusDescription = BrowserInterop.GetCloseStatusDescription(_innerWebSocket); + lock (_lockObject) + { + if (closeStatusDescription != null && _closeStatusDescription == null) { - throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, ex); + _closeStatusDescription = closeStatusDescription; } - throw new WebSocketException(WebSocketError.NativeError, ex); } + return _closeStatusDescription; } private static WebSocketState GetReadyState(JSObject innerWebSocket) @@ -641,6 +632,5 @@ private static WebSocketState GetReadyState(JSObject innerWebSocket) }; } - #endregion } } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs index 26dfd9a4fd01ba..c319b8f6c686cc 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs @@ -33,12 +33,12 @@ public class CloseTest : ClientWebSocketTestBase public CloseTest(ITestOutputHelper output) : base(output) { } - [ActiveIssue("https://github.com/dotnet/runtime/issues/28957")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/28957", typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServersAndBoolean))] public async Task CloseAsync_ServerInitiatedClose_Success(Uri server, bool useCloseOutputAsync) { - const string closeWebSocketMetaCommand = ".close"; + const string closeWebSocketMetaCommand = ".shutdown"; using (ClientWebSocket cws = await GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) { @@ -69,9 +69,10 @@ await cws.SendAsync( // Send back close message to acknowledge server-initiated close. _output.WriteLine("Close starting."); + var closeStatus = PlatformDetection.IsNotBrowser ? WebSocketCloseStatus.InvalidMessageType : (WebSocketCloseStatus)3210; await (useCloseOutputAsync ? - cws.CloseOutputAsync(WebSocketCloseStatus.InvalidMessageType, string.Empty, cts.Token) : - cws.CloseAsync(WebSocketCloseStatus.InvalidMessageType, string.Empty, cts.Token)); + cws.CloseOutputAsync(closeStatus, string.Empty, cts.Token) : + cws.CloseAsync(closeStatus, string.Empty, cts.Token)); _output.WriteLine("Close done."); Assert.Equal(WebSocketState.Closed, cws.State); @@ -224,7 +225,6 @@ await Assert.ThrowsAnyAsync(async () => [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] - [SkipOnPlatform(TestPlatforms.Browser, "This never really worked for browser, it was just lucky timing that browser's `close` event was executed in next browser tick, for this test. See also https://github.com/dotnet/runtime/issues/45538")] public async Task CloseOutputAsync_ClientInitiated_CanReceive_CanClose(Uri server) { string message = "Hello WebSockets!"; @@ -233,7 +233,7 @@ public async Task CloseOutputAsync_ClientInitiated_CanReceive_CanClose(Uri serve { var cts = new CancellationTokenSource(TimeOutMilliseconds); - var closeStatus = WebSocketCloseStatus.InvalidPayloadData; + var closeStatus = PlatformDetection.IsNotBrowser ? WebSocketCloseStatus.InvalidPayloadData : (WebSocketCloseStatus)3210; string closeDescription = "CloseOutputAsync_Client_InvalidPayloadData"; await cws.SendAsync(WebSocketData.GetBufferFromText(message), WebSocketMessageType.Text, true, cts.Token); @@ -261,7 +261,64 @@ public async Task CloseOutputAsync_ClientInitiated_CanReceive_CanClose(Uri serve } } - [ActiveIssue("https://github.com/dotnet/runtime/issues/28957")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/28957", typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))] + [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] + public async Task CloseOutputAsync_ServerInitiated_CanReceive(Uri server) + { + string message = "Hello WebSockets!"; + var expectedCloseStatus = WebSocketCloseStatus.NormalClosure; + var expectedCloseDescription = ".shutdownafter"; + + using (ClientWebSocket cws = await GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) + { + var cts = new CancellationTokenSource(TimeOutMilliseconds); + + await cws.SendAsync( + WebSocketData.GetBufferFromText(expectedCloseDescription), + WebSocketMessageType.Text, + true, + cts.Token); + + // Should be able to receive the message echoed by the server. + var recvBuffer = new byte[100]; + var segmentRecv = new ArraySegment(recvBuffer); + WebSocketReceiveResult recvResult = await cws.ReceiveAsync(segmentRecv, cts.Token); + Assert.Equal(expectedCloseDescription.Length, recvResult.Count); + segmentRecv = new ArraySegment(segmentRecv.Array, 0, recvResult.Count); + Assert.Equal(expectedCloseDescription, WebSocketData.GetTextFromBuffer(segmentRecv)); + Assert.Null(recvResult.CloseStatus); + Assert.Null(recvResult.CloseStatusDescription); + + // Should be able to receive a shutdown message. + segmentRecv = new ArraySegment(recvBuffer); + recvResult = await cws.ReceiveAsync(segmentRecv, cts.Token); + Assert.Equal(0, recvResult.Count); + Assert.Equal(expectedCloseStatus, recvResult.CloseStatus); + Assert.Equal(expectedCloseDescription, recvResult.CloseStatusDescription); + + // Verify WebSocket state + Assert.Equal(expectedCloseStatus, cws.CloseStatus); + Assert.Equal(expectedCloseDescription, cws.CloseStatusDescription); + + Assert.Equal(WebSocketState.CloseReceived, cws.State); + + // Should be able to send. + await cws.SendAsync(WebSocketData.GetBufferFromText(message), WebSocketMessageType.Text, true, cts.Token); + + // Cannot change the close status/description with the final close. + var closeStatus = PlatformDetection.IsNotBrowser ? WebSocketCloseStatus.InvalidPayloadData : (WebSocketCloseStatus)3210; + var closeDescription = "CloseOutputAsync_Client_Description"; + + await cws.CloseAsync(closeStatus, closeDescription, cts.Token); + + Assert.Equal(expectedCloseStatus, cws.CloseStatus); + Assert.Equal(expectedCloseDescription, cws.CloseStatusDescription); + Assert.Equal(WebSocketState.Closed, cws.State); + } + } + + [ActiveIssue("https://github.com/dotnet/runtime/issues/28957", typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] public async Task CloseOutputAsync_ServerInitiated_CanSend(Uri server) @@ -298,7 +355,7 @@ await cws.SendAsync( await cws.SendAsync(WebSocketData.GetBufferFromText(message), WebSocketMessageType.Text, true, cts.Token); // Cannot change the close status/description with the final close. - var closeStatus = WebSocketCloseStatus.InvalidPayloadData; + var closeStatus = PlatformDetection.IsNotBrowser ? WebSocketCloseStatus.InvalidPayloadData : (WebSocketCloseStatus)3210; var closeDescription = "CloseOutputAsync_Client_Description"; await cws.CloseAsync(closeStatus, closeDescription, cts.Token); diff --git a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs index 2016f4f7285a87..357dcb0945d665 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs @@ -249,12 +249,12 @@ public async Task SendAsync_MultipleOutstandingSendOperations_Throws(Uri server) [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))] [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))] // This will also pass when no exception is thrown. Current implementation doesn't throw. - [ActiveIssue("https://github.com/dotnet/runtime/issues/83517", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/83517", typeof(PlatformDetection), nameof(PlatformDetection.IsNodeJS))] public async Task ReceiveAsync_MultipleOutstandingReceiveOperations_Throws(Uri server) { using (ClientWebSocket cws = await GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) { - var cts = new CancellationTokenSource(TimeOutMilliseconds); + var cts = new CancellationTokenSource(PlatformDetection.LocalEchoServerIsNotAvailable ? TimeOutMilliseconds : 200); Task[] tasks = new Task[2]; diff --git a/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj b/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj index 54be463b694e12..b797fcf1894599 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj +++ b/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj @@ -17,9 +17,6 @@ 01:15:00 - - - <_XUnitBackgroundExec>false diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj index 3e6c61c71a1419..ab0e7ac77251ef 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj @@ -1,4 +1,5 @@ + $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) true diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs index 31352864753af6..cd8e24d6343e70 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/WebWorkerTest.cs @@ -503,5 +503,102 @@ public async Task JSObject_CapturesAffinity(Executor executor1, Executor executo } #endregion + + #region WebSocket + + [Theory, MemberData(nameof(GetTargetThreads))] + public async Task WebSocketClient_ContentInSameThread(Executor executor) + { + var cts = new CancellationTokenSource(TimeoutMilliseconds); + + var uri = new Uri(WebWorkerTestHelper.LocalWsEcho + "?guid=" + Guid.NewGuid()); + var message = "hello"; + var send = Encoding.UTF8.GetBytes(message); + var receive = new byte[100]; + + await executor.Execute(async () => + { + using var client = new ClientWebSocket(); + await client.ConnectAsync(uri, CancellationToken.None); + await client.SendAsync(send, WebSocketMessageType.Text, true, CancellationToken.None); + + var res = await client.ReceiveAsync(receive, CancellationToken.None); + Assert.Equal(WebSocketMessageType.Text, res.MessageType); + Assert.True(res.EndOfMessage); + Assert.Equal(send.Length, res.Count); + Assert.Equal(message, Encoding.UTF8.GetString(receive, 0, res.Count)); + }, cts.Token); + } + + + [Theory, MemberData(nameof(GetTargetThreads2x))] + public Task WebSocketClient_ResponseCloseInDifferentThread(Executor executor1, Executor executor2) + { + var cts = new CancellationTokenSource(TimeoutMilliseconds); + + var uri = new Uri(WebWorkerTestHelper.LocalWsEcho + "?guid=" + Guid.NewGuid()); + var message = "hello"; + var send = Encoding.UTF8.GetBytes(message); + var receive = new byte[100]; + + var e1Job = async (Task e2done, TaskCompletionSource e1State) => + { + using var client = new ClientWebSocket(); + await client.ConnectAsync(uri, CancellationToken.None); + await client.SendAsync(send, WebSocketMessageType.Text, true, CancellationToken.None); + + // share the state with the E2 continuation + e1State.SetResult(client); + await e2done; + }; + + var e2Job = async (ClientWebSocket client) => + { + var res = await client.ReceiveAsync(receive, CancellationToken.None); + Assert.Equal(WebSocketMessageType.Text, res.MessageType); + Assert.True(res.EndOfMessage); + Assert.Equal(send.Length, res.Count); + Assert.Equal(message, Encoding.UTF8.GetString(receive, 0, res.Count)); + + await client.CloseAsync(WebSocketCloseStatus.NormalClosure, "bye", CancellationToken.None); + }; + + return ActionsInDifferentThreads(executor1, executor2, e1Job, e2Job, cts); + } + + [Theory, MemberData(nameof(GetTargetThreads2x))] + public Task WebSocketClient_CancelInDifferentThread(Executor executor1, Executor executor2) + { + var cts = new CancellationTokenSource(TimeoutMilliseconds); + + var uri = new Uri(WebWorkerTestHelper.LocalWsEcho + "?guid=" + Guid.NewGuid()); + var message = ".delay5sec"; // this will make the loopback server slower + var send = Encoding.UTF8.GetBytes(message); + var receive = new byte[100]; + + var e1Job = async (Task e2done, TaskCompletionSource e1State) => + { + using var client = new ClientWebSocket(); + await client.ConnectAsync(uri, CancellationToken.None); + await client.SendAsync(send, WebSocketMessageType.Text, true, CancellationToken.None); + + // share the state with the E2 continuation + e1State.SetResult(client); + await e2done; + }; + + var e2Job = async (ClientWebSocket client) => + { + CancellationTokenSource cts2 = new CancellationTokenSource(); + var resTask = client.ReceiveAsync(receive, cts2.Token); + cts2.Cancel(); + var ex = await Assert.ThrowsAsync(() => resTask); + Assert.Equal(cts2.Token, ex.CancellationToken); + }; + + return ActionsInDifferentThreads(executor1, executor2, e1Job, e2Job, cts); + } + + #endregion } } diff --git a/src/mono/browser/runtime/web-socket.ts b/src/mono/browser/runtime/web-socket.ts index ec88b12d9ff14e..34e32490c0350a 100644 --- a/src/mono/browser/runtime/web-socket.ts +++ b/src/mono/browser/runtime/web-socket.ts @@ -11,7 +11,6 @@ import { VoidPtr } from "./types/emscripten"; import { PromiseController } from "./types/internal"; import { mono_log_warn } from "./logging"; import { viewOrCopy, utf8ToStringRelaxed, stringToUTF8 } from "./strings"; -import { IDisposable } from "./marshal"; import { wrap_as_cancelable } from "./cancelable-promise"; import { assert_js_interop } from "./invoke-js"; @@ -25,8 +24,8 @@ const wasm_ws_pending_open_promise_used = Symbol.for("wasm wasm_ws_pending_open_ const wasm_ws_pending_close_promises = Symbol.for("wasm ws_pending_close_promises"); const wasm_ws_pending_send_promises = Symbol.for("wasm ws_pending_send_promises"); const wasm_ws_is_aborted = Symbol.for("wasm ws_is_aborted"); -const wasm_ws_on_closed = Symbol.for("wasm ws_on_closed"); const wasm_ws_receive_status_ptr = Symbol.for("wasm ws_receive_status_ptr"); + let mono_wasm_web_socket_close_warning = false; const ws_send_buffer_blocking_threshold = 65536; const emptyBuffer = new Uint8Array(); @@ -43,11 +42,10 @@ function verifyEnvironment() { } } -export function ws_wasm_create(uri: string, sub_protocols: string[] | null, receive_status_ptr: VoidPtr, onClosed: (code: number, reason: string) => void): WebSocketExtension { +export function ws_wasm_create(uri: string, sub_protocols: string[] | null, receive_status_ptr: VoidPtr): WebSocketExtension { verifyEnvironment(); assert_js_interop(); mono_assert(uri && typeof uri === "string", () => `ERR12: Invalid uri ${typeof uri}`); - mono_assert(typeof onClosed === "function", () => `ERR12: Invalid onClosed ${typeof onClosed}`); const ws = new globalThis.WebSocket(uri, sub_protocols || undefined) as WebSocketExtension; const { promise_control: open_promise_control } = createPromiseController(); @@ -58,7 +56,6 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece ws[wasm_ws_pending_send_promises] = []; ws[wasm_ws_pending_close_promises] = []; ws[wasm_ws_receive_status_ptr] = receive_status_ptr; - ws[wasm_ws_on_closed] = onClosed as any; ws.binaryType = "arraybuffer"; const local_on_open = () => { if (ws[wasm_ws_is_aborted]) return; @@ -77,7 +74,8 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece if (ws[wasm_ws_is_aborted]) return; if (loaderHelpers.is_exited()) return; - onClosed(ev.code, ev.reason); + ws["close_status"] = ev.code; + ws["close_status_description"] = ev.reason; // this reject would not do anything if there was already "open" before it. open_promise_control.reject(new Error(ev.reason)); @@ -94,9 +92,6 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece setI32(receive_status_ptr + 8, 1);// end_of_message: true receive_promise_control.resolve(); }); - - // cleanup the delegate proxy - ws[wasm_ws_on_closed].dispose(); }; const local_on_error = (ev: any) => { if (ws[wasm_ws_is_aborted]) return; @@ -147,11 +142,6 @@ export function ws_wasm_receive(ws: WebSocketExtension, buffer_ptr: VoidPtr, buf const receive_event_queue = ws[wasm_ws_pending_receive_event_queue]; const receive_promise_queue = ws[wasm_ws_pending_receive_promise_queue]; - const readyState = ws.readyState; - if (readyState != WebSocket.OPEN && readyState != WebSocket.CLOSING) { - throw new Error(`InvalidState: ${readyState} The WebSocket is not connected.`); - } - if (receive_event_queue.getLength()) { mono_assert(receive_promise_queue.getLength() == 0, "ERR20: Invalid WS state"); @@ -159,6 +149,16 @@ export function ws_wasm_receive(ws: WebSocketExtension, buffer_ptr: VoidPtr, buf return resolvedPromise(); } + + const readyState = ws.readyState; + if (readyState == WebSocket.CLOSED) { + const receive_status_ptr = ws[wasm_ws_receive_status_ptr]; + setI32(receive_status_ptr, 0); // count + setI32(receive_status_ptr + 4, 2); // type:close + setI32(receive_status_ptr + 8, 1);// end_of_message: true + return resolvedPromise(); + } + const { promise, promise_control } = createPromiseController(); const receive_promise_control = promise_control as ReceivePromiseControl; receive_promise_control.buffer_ptr = buffer_ptr; @@ -206,9 +206,6 @@ export function ws_wasm_abort(ws: WebSocketExtension): void { ws[wasm_ws_is_aborted] = true; reject_promises(ws, new Error("OperationCanceledException")); - // cleanup the delegate proxy - ws[wasm_ws_on_closed]?.dispose(); - try { // this is different from Managed implementation ws.close(1000, "Connection was aborted."); @@ -416,11 +413,12 @@ type WebSocketExtension = WebSocket & { [wasm_ws_pending_send_promises]: PromiseController[] [wasm_ws_pending_close_promises]: PromiseController[] [wasm_ws_is_aborted]: boolean - [wasm_ws_on_closed]: IDisposable [wasm_ws_receive_status_ptr]: VoidPtr [wasm_ws_pending_send_buffer_offset]: number [wasm_ws_pending_send_buffer_type]: number [wasm_ws_pending_send_buffer]: Uint8Array | null + ["close_status"]: number | undefined + ["close_status_description"]: string | undefined dispose(): void } From 13438ca93b9c6b538471e5558aa43ed112bb53dd Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 15 Jan 2024 15:09:38 +0100 Subject: [PATCH 2/9] feedback - calling methods from inside of the lock, but awaiting outside - made Dispose->Abort async to target thread - optimized CancellationHelper - added wasm_ws_close_received and wasm_ws_close_received on JS side - added extra state validation on JS side - removed mono_wasm_web_socket_close_warning --- .../BrowserWebSockets/BrowserWebSocket.cs | 319 +++++++++--------- .../tests/CloseTest.cs | 10 +- src/mono/browser/runtime/web-socket.ts | 38 ++- 3 files changed, 202 insertions(+), 165 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 8c588fcaeca9a6..8d9e9ebd873506 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -24,6 +24,7 @@ internal sealed class BrowserWebSocket : WebSocket private bool _closeReceived; private bool _disposed; private bool _aborted; + private bool _shouldAbort; private bool _cancelled; private int[] responseStatus = new int[3]; private MemoryHandle? responseStatusHandle; @@ -40,7 +41,7 @@ public override WebSocketState State { return _state; } - var st = GetReadyState(_innerWebSocket!); + var st = GetReadyStateLocked(_innerWebSocket!); if (st == WebSocketState.Closed || st == WebSocketState.CloseSent) { if (_closeReceived && _closeSent) @@ -93,7 +94,7 @@ public override WebSocketCloseStatus? CloseStatus { return null; } - return GetCloseStatus(); + return GetCloseStatusLocked(); } } } @@ -112,7 +113,7 @@ public override string? CloseStatusDescription { return null; } - return GetCloseStatusDescription(); + return GetCloseStatusDescriptionLocked(); } } } @@ -136,11 +137,13 @@ internal Task ConnectAsync(Uri uri, List? requestedSubProtocols, Cancell { lock (_lockObject) { + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + if (FastState != WebSocketState.None) { throw new InvalidOperationException(SR.net_WebSockets_AlreadyStarted); } - ThrowIfDisposed(); FastState = WebSocketState.Connecting; } // lock CreateCore(uri, requestedSubProtocols); @@ -149,15 +152,7 @@ internal Task ConnectAsync(Uri uri, List? requestedSubProtocols, Cancell public override Task SendAsync(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - - // fast check of previous _state instead of GetReadyState(), the readyState would be validated on JS side - var fastState = FastState; - if (fastState != WebSocketState.Open && fastState != WebSocketState.CloseReceived) - { - throw new InvalidOperationException(SR.net_WebSockets_NotConnected); - } - + // this validation should be synchronous if (messageType != WebSocketMessageType.Binary && messageType != WebSocketMessageType.Text) { throw new ArgumentException(SR.Format(SR.net_WebSockets_Argument_InvalidMessageType, @@ -172,95 +167,61 @@ public override Task SendAsync(ArraySegment buffer, WebSocketMessageType m WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); ThrowIfDisposed(); - return SendAsyncCore(buffer, messageType, endOfMessage, cancellationToken, fastState); + return SendAsyncCore(buffer, messageType, endOfMessage, cancellationToken); } public override Task ReceiveAsync(ArraySegment buffer, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromException(new OperationCanceledException(cancellationToken)); - } - - // fast check of previous _state instead of GetReadyState(), the readyState would be validated on JS side - var fastState = FastState; - if (fastState != WebSocketState.Open && fastState != WebSocketState.CloseSent) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, fastState, "Open, CloseSent")); - } - + // this validation should be synchronous WebSocketValidate.ValidateArraySegment(buffer, nameof(buffer)); ThrowIfDisposed(); - return ReceiveAsyncCore(buffer, cancellationToken, fastState); + return ReceiveAsyncCore(buffer, cancellationToken); } public override Task CloseOutputAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - + // this validation should be synchronous WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); - var fastState = FastState; - if (fastState == WebSocketState.None || fastState == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, fastState, "Connecting, Open, CloseSent, Aborted")); - } - lock (_lockObject) - { - var state = State; - if (state == WebSocketState.None || state == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); - } - if (_closeSent) - { - return Task.CompletedTask; - } - } // lock - return CloseAsyncCore(closeStatus, statusDescription, false, cancellationToken, fastState); + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + + return CloseAsyncCore(closeStatus, statusDescription, false, cancellationToken); } public override Task CloseAsync(WebSocketCloseStatus closeStatus, string? statusDescription, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - + // this validation should be synchronous WebSocketValidate.ValidateCloseStatus(closeStatus, statusDescription); - var fastState = FastState; - if (fastState == WebSocketState.None || fastState == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, fastState, "Connecting, Open, CloseSent, Aborted")); - } - WebSocketState state; - lock (_lockObject) - { - state = State; - if (state == WebSocketState.Closed) - { - return Task.CompletedTask; - } - if (state == WebSocketState.None || state == WebSocketState.Closed) - { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, state, "Connecting, Open, CloseSent, Aborted")); - } + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); - } // lock - return CloseAsyncCore(closeStatus, statusDescription, state != WebSocketState.Aborted, cancellationToken, fastState); + return CloseAsyncCore(closeStatus, statusDescription, true, cancellationToken); } public override void Abort() { - WebSocketState state; lock (_lockObject) { - if (_disposed) + if (_disposed || _aborted) { return; } - state = State; - } // lock - AbortCore(this, state); + var fastState = FastState; + if (fastState == WebSocketState.Closed || fastState == WebSocketState.Aborted) + { + return; + } + + FastState = WebSocketState.Aborted; + _aborted = true; + + // We can call this cross-thread from inside the lock, because there are no callbacks which would lock the same lock + // This will reject/resolve some promises + BrowserInterop.WebSocketAbort(_innerWebSocket!); + } } public override void Dispose() @@ -274,18 +235,40 @@ public override void Dispose() } _disposed = true; state = State; + + if (state < WebSocketState.Closed && state != WebSocketState.None) + { + _shouldAbort = true; + FastState = WebSocketState.Aborted; + } + else if (state != WebSocketState.Aborted) + { + FastState = WebSocketState.Closed; + } + } // lock - if (state < WebSocketState.Closed && state != WebSocketState.None) + // if this is finalizer thread, we need to postpone the abort -> dispose + _innerWebSocket?.SynchronizationContext.Post(static _state => { - AbortCore(this, state); - } - if (state != WebSocketState.Aborted) - { - FastState = WebSocketState.Closed; - } - _innerWebSocket?.Dispose(); - responseStatusHandle?.Dispose(); + var self = (BrowserWebSocket)_state!; + var state = self.State; + lock (self._lockObject) + { + if (self._shouldAbort && !self._aborted) + { + self._aborted = true; + self._shouldAbort = false; + + // We can call this inside the lock, because there are no callbacks which would lock the same lock + // This will reject/resolve some promises + BrowserInterop.WebSocketAbort(self._innerWebSocket!); + } + self._innerWebSocket?.Dispose(); + self.responseStatusHandle?.Dispose(); + } + + }, this); } private void ThrowIfDisposed() @@ -296,19 +279,6 @@ private void ThrowIfDisposed() } // lock } - private static void AbortCore(BrowserWebSocket self, WebSocketState currentState) - { - lock (self._lockObject) - { - self.FastState = WebSocketState.Aborted; - self._aborted = true; - if (self._disposed || currentState == WebSocketState.Closed || currentState == WebSocketState.Aborted) - { - return; - } - } - BrowserInterop.WebSocketAbort(self._innerWebSocket!); - } private void CreateCore(Uri uri, List? requestedSubProtocols) { @@ -330,6 +300,8 @@ private void CreateCore(Uri uri, List? requestedSubProtocols) private async Task ConnectAsyncCore(CancellationToken cancellationToken) { + Task openTask; + lock (_lockObject) { if (_aborted) @@ -338,12 +310,12 @@ private async Task ConnectAsyncCore(CancellationToken cancellationToken) throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure); } ThrowIfDisposed(); + + openTask = BrowserInterop.WebSocketOpen(_innerWebSocket!); } // lock try { - var openTask = BrowserInterop.WebSocketOpen(_innerWebSocket!); - await CancellationHelper(openTask!, cancellationToken, WebSocketState.Connecting).ConfigureAwait(false); lock (_lockObject) @@ -375,57 +347,81 @@ private async Task ConnectAsyncCore(CancellationToken cancellationToken) } } - private async Task SendAsyncCore(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken, WebSocketState previousState) + private async Task SendAsyncCore(ArraySegment buffer, WebSocketMessageType messageType, bool endOfMessage, CancellationToken cancellationToken) { + WebSocketState previousState = WebSocketState.None; + Task? sendTask; + MemoryHandle? pinBuffer = null; + try { - Task? sendTask; - if (buffer.Count == 0) + lock (_lockObject) { - sendTask = BrowserInterop.WebSocketSend(_innerWebSocket!, IntPtr.Zero, 0, (int)messageType, endOfMessage); - if (sendTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + + previousState = FastState; + if (previousState != WebSocketState.Open && previousState != WebSocketState.CloseReceived) { - await CancellationHelper(sendTask, cancellationToken, previousState).ConfigureAwait(false); + throw new InvalidOperationException(SR.net_WebSockets_NotConnected); } - } - else - { - Memory bufferMemory = buffer.AsMemory(); - MemoryHandle pinBuffer = bufferMemory.Pin(); - try + + if (buffer.Count == 0) { - sendTask = BrowserInterop.UnsafeSend(_innerWebSocket!, pinBuffer, bufferMemory.Length, messageType, endOfMessage); - if (sendTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts - { - await CancellationHelper(sendTask, cancellationToken, previousState, pinBuffer).ConfigureAwait(false); - } + sendTask = BrowserInterop.WebSocketSend(_innerWebSocket!, IntPtr.Zero, 0, (int)messageType, endOfMessage); } - finally + else { - // must be after await! - pinBuffer.Dispose(); + Memory bufferMemory = buffer.AsMemory(); + pinBuffer = bufferMemory.Pin(); + sendTask = BrowserInterop.UnsafeSend(_innerWebSocket!, pinBuffer.Value, bufferMemory.Length, messageType, endOfMessage); } } + + if (sendTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts. Null means synchronously resolved. + { + await CancellationHelper(sendTask, cancellationToken, previousState, pinBuffer).ConfigureAwait(false); + } } catch (JSException ex) { if (ex.Message.StartsWith("InvalidState:")) { - throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, FastState, "Open"), ex); + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, previousState, "Open"), ex); } throw new WebSocketException(WebSocketError.NativeError, ex); } + finally + { + // must be after await! + pinBuffer?.Dispose(); + } } - private async Task ReceiveAsyncCore(ArraySegment buffer, CancellationToken cancellationToken, WebSocketState previousState) + private async Task ReceiveAsyncCore(ArraySegment buffer, CancellationToken cancellationToken) { - Memory bufferMemory = buffer.AsMemory(); - MemoryHandle pinBuffer = bufferMemory.Pin(); + WebSocketState previousState = WebSocketState.None; + Task? receiveTask; + MemoryHandle? pinBuffer = null; try { - var receiveTask = BrowserInterop.ReceiveUnsafe(_innerWebSocket!, pinBuffer, bufferMemory.Length); + lock (_lockObject) + { + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); - if (receiveTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + previousState = FastState; + if (previousState != WebSocketState.Open && previousState != WebSocketState.CloseSent) + { + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, previousState, "Open, CloseSent")); + } + + Memory bufferMemory = buffer.AsMemory(); + pinBuffer = bufferMemory.Pin(); + receiveTask = BrowserInterop.ReceiveUnsafe(_innerWebSocket!, pinBuffer.Value, bufferMemory.Length); + } + + if (receiveTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts. Null means synchronously resolved. { await CancellationHelper(receiveTask, cancellationToken, previousState, pinBuffer).ConfigureAwait(false); } @@ -443,7 +439,7 @@ private async Task ReceiveAsyncCore(ArraySegment b finally { // must be after await! - pinBuffer.Dispose(); + pinBuffer?.Dispose(); } } @@ -464,12 +460,12 @@ private WebSocketReceiveResult ConvertResponse() { _closeReceived = true; FastState = _closeSent ? WebSocketState.Closed : WebSocketState.CloseReceived; + ForceReadCloseStatusLocked(); } } // lock if (messageType == WebSocketMessageType.Close) { - ForceReadCloseStatus(); switch (_closeStatus ?? WebSocketCloseStatus.NormalClosure) { case WebSocketCloseStatus.NormalClosure: @@ -491,28 +487,45 @@ private WebSocketReceiveResult ConvertResponse() return new WebSocketReceiveResult(count, messageType, isEnd); } - private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, bool waitForCloseReceived, CancellationToken cancellationToken, WebSocketState previousState) + private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? statusDescription, bool fullClose, CancellationToken cancellationToken) { + Task? closeTask; + WebSocketState previousState; lock (_lockObject) { + cancellationToken.ThrowIfCancellationRequested(); + + previousState = FastState; + if (_aborted) + { + return; + } if (!_closeReceived) { _closeStatus = closeStatus; _closeStatusDescription = statusDescription; } + if (previousState == WebSocketState.None || previousState == WebSocketState.Closed) + { + throw new WebSocketException(WebSocketError.InvalidState, SR.Format(SR.net_WebSockets_InvalidState, previousState, "Connecting, Open, CloseSent, Aborted")); + } + _closeSent = true; + + closeTask = BrowserInterop.WebSocketClose(_innerWebSocket!, (int)closeStatus, statusDescription, fullClose); } - var closeTask = BrowserInterop.WebSocketClose(_innerWebSocket!, (int)closeStatus, statusDescription, waitForCloseReceived); - if (closeTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts + + if (closeTask != null) // this is optimization for single-threaded build, see resolvedPromise() in web-socket.ts. Null means synchronously resolved. { await CancellationHelper(closeTask, cancellationToken, previousState).ConfigureAwait(false); } - if (waitForCloseReceived) + + if (fullClose) { lock (_lockObject) { _closeReceived = true; - ForceReadCloseStatus(); + ForceReadCloseStatusLocked(); _ = State; } } @@ -532,6 +545,13 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati } try { + if (promise.IsCompleted) + { + // don't have to register for cancelation + await promise.ConfigureAwait(false); + return; + } + using (var receiveRegistration = cancellationToken.Register(static s => { CancelablePromise.CancelPromise((Task)s!); @@ -548,7 +568,7 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati var state = State; if (state == WebSocketState.Aborted) { - ForceReadCloseStatus(); + ForceReadCloseStatusLocked(); throw new OperationCanceledException(nameof(WebSocketState.Aborted), ex); } @@ -566,7 +586,7 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati } if (previousState == WebSocketState.Connecting) { - ForceReadCloseStatus(); + ForceReadCloseStatusLocked(); throw new WebSocketException(WebSocketError.Faulted, SR.net_webstatus_ConnectFailure, ex); } throw new WebSocketException(WebSocketError.NativeError, ex); @@ -578,47 +598,42 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati } } - private void ForceReadCloseStatus() + // needs to be called with locked _lockObject + private void ForceReadCloseStatusLocked() { - lock (_lockObject) + if (!_disposed && _closeStatus == null) { - if (!_disposed && _closeStatus == null) - { - GetCloseStatus(); - GetCloseStatusDescription(); - } + GetCloseStatusLocked(); + GetCloseStatusDescriptionLocked(); } } - private WebSocketCloseStatus? GetCloseStatus() + // needs to be called with locked _lockObject + private WebSocketCloseStatus? GetCloseStatusLocked() { ThrowIfDisposed(); var closeStatus = BrowserInterop.GetCloseStatus(_innerWebSocket); - lock (_lockObject) + if (closeStatus != null && _closeStatus == null) { - if (closeStatus != null && _closeStatus == null) - { - _closeStatus = closeStatus; - } + _closeStatus = closeStatus; } return _closeStatus; } - private string? GetCloseStatusDescription() + // needs to be called with locked _lockObject + private string? GetCloseStatusDescriptionLocked() { ThrowIfDisposed(); var closeStatusDescription = BrowserInterop.GetCloseStatusDescription(_innerWebSocket); - lock (_lockObject) + if (closeStatusDescription != null && _closeStatusDescription == null) { - if (closeStatusDescription != null && _closeStatusDescription == null) - { - _closeStatusDescription = closeStatusDescription; - } + _closeStatusDescription = closeStatusDescription; } return _closeStatusDescription; } - private static WebSocketState GetReadyState(JSObject innerWebSocket) + // needs to be called with locked _lockObject + private static WebSocketState GetReadyStateLocked(JSObject innerWebSocket) { var readyState = BrowserInterop.GetReadyState(innerWebSocket); // https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/readyState diff --git a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs index c319b8f6c686cc..81f80a1e647bdb 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs @@ -38,7 +38,7 @@ public CloseTest(ITestOutputHelper output) : base(output) { } [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServersAndBoolean))] public async Task CloseAsync_ServerInitiatedClose_Success(Uri server, bool useCloseOutputAsync) { - const string closeWebSocketMetaCommand = ".shutdown"; + const string shutdownWebSocketMetaCommand = ".shutdown"; using (ClientWebSocket cws = await GetConnectedWebSocket(server, TimeOutMilliseconds, _output)) { @@ -46,7 +46,7 @@ public async Task CloseAsync_ServerInitiatedClose_Success(Uri server, bool useCl _output.WriteLine("SendAsync starting."); await cws.SendAsync( - WebSocketData.GetBufferFromText(closeWebSocketMetaCommand), + WebSocketData.GetBufferFromText(shutdownWebSocketMetaCommand), WebSocketMessageType.Text, true, cts.Token); @@ -59,13 +59,13 @@ await cws.SendAsync( // Verify received server-initiated close message. Assert.Equal(WebSocketCloseStatus.NormalClosure, recvResult.CloseStatus); - Assert.Equal(closeWebSocketMetaCommand, recvResult.CloseStatusDescription); + Assert.Equal(shutdownWebSocketMetaCommand, recvResult.CloseStatusDescription); Assert.Equal(WebSocketMessageType.Close, recvResult.MessageType); // Verify current websocket state as CloseReceived which indicates only partial close. Assert.Equal(WebSocketState.CloseReceived, cws.State); Assert.Equal(WebSocketCloseStatus.NormalClosure, cws.CloseStatus); - Assert.Equal(closeWebSocketMetaCommand, cws.CloseStatusDescription); + Assert.Equal(shutdownWebSocketMetaCommand, cws.CloseStatusDescription); // Send back close message to acknowledge server-initiated close. _output.WriteLine("Close starting."); @@ -79,7 +79,7 @@ await cws.SendAsync( // Verify that there is no follow-up echo close message back from the server by // making sure the close code and message are the same as from the first server close message. Assert.Equal(WebSocketCloseStatus.NormalClosure, cws.CloseStatus); - Assert.Equal(closeWebSocketMetaCommand, cws.CloseStatusDescription); + Assert.Equal(shutdownWebSocketMetaCommand, cws.CloseStatusDescription); } } diff --git a/src/mono/browser/runtime/web-socket.ts b/src/mono/browser/runtime/web-socket.ts index 34e32490c0350a..bbcac93a1bd790 100644 --- a/src/mono/browser/runtime/web-socket.ts +++ b/src/mono/browser/runtime/web-socket.ts @@ -24,9 +24,10 @@ const wasm_ws_pending_open_promise_used = Symbol.for("wasm wasm_ws_pending_open_ const wasm_ws_pending_close_promises = Symbol.for("wasm ws_pending_close_promises"); const wasm_ws_pending_send_promises = Symbol.for("wasm ws_pending_send_promises"); const wasm_ws_is_aborted = Symbol.for("wasm ws_is_aborted"); +const wasm_ws_close_sent = Symbol.for("wasm wasm_ws_close_sent"); +const wasm_ws_close_received = Symbol.for("wasm wasm_ws_close_received"); const wasm_ws_receive_status_ptr = Symbol.for("wasm ws_receive_status_ptr"); -let mono_wasm_web_socket_close_warning = false; const ws_send_buffer_blocking_threshold = 65536; const emptyBuffer = new Uint8Array(); @@ -74,6 +75,7 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece if (ws[wasm_ws_is_aborted]) return; if (loaderHelpers.is_exited()) return; + ws[wasm_ws_close_received] = true; ws["close_status"] = ev.code; ws["close_status_description"] = ev.reason; @@ -126,6 +128,10 @@ export function ws_wasm_open(ws: WebSocketExtension): Promise | null { mono_assert(!!ws, "ERR17: expected ws instance"); + if (ws[wasm_ws_is_aborted] || ws[wasm_ws_close_sent]) { + return rejectedPromise("InvalidState: The WebSocket is not connected."); + } + const buffer_view = new Uint8Array(localHeapViewU8().buffer, buffer_ptr, buffer_length); const whole_buffer = _mono_wasm_web_socket_send_buffering(ws, buffer_view, message_type, end_of_message); @@ -139,6 +145,15 @@ export function ws_wasm_send(ws: WebSocketExtension, buffer_ptr: VoidPtr, buffer export function ws_wasm_receive(ws: WebSocketExtension, buffer_ptr: VoidPtr, buffer_length: number): Promise | null { mono_assert(!!ws, "ERR18: expected ws instance"); + // we can't quickly return if wasm_ws_close_received==true, because there could be pending messages + if (ws[wasm_ws_is_aborted]) { + const receive_status_ptr = ws[wasm_ws_receive_status_ptr]; + setI32(receive_status_ptr, 0); // count + setI32(receive_status_ptr + 4, 2); // type:close + setI32(receive_status_ptr + 8, 1);// end_of_message: true + return resolvedPromise(); + } + const receive_event_queue = ws[wasm_ws_pending_receive_event_queue]; const receive_promise_queue = ws[wasm_ws_pending_receive_promise_queue]; @@ -171,10 +186,10 @@ export function ws_wasm_receive(ws: WebSocketExtension, buffer_ptr: VoidPtr, buf export function ws_wasm_close(ws: WebSocketExtension, code: number, reason: string | null, wait_for_close_received: boolean): Promise | null { mono_assert(!!ws, "ERR19: expected ws instance"); - if (ws.readyState == WebSocket.CLOSED) { + if (ws[wasm_ws_is_aborted] || ws[wasm_ws_close_sent] || ws.readyState == WebSocket.CLOSED) { return resolvedPromise(); } - + ws[wasm_ws_close_sent] = true; if (wait_for_close_received) { const { promise, promise_control } = createPromiseController(); ws[wasm_ws_pending_close_promises].push(promise_control); @@ -187,10 +202,6 @@ export function ws_wasm_close(ws: WebSocketExtension, code: number, reason: stri return promise; } else { - if (!mono_wasm_web_socket_close_warning) { - mono_wasm_web_socket_close_warning = true; - mono_log_warn("WARNING: Web browsers do not support closing the output side of a WebSocket. CloseOutputAsync has closed the socket and discarded any incoming messages."); - } if (typeof reason === "string") { ws.close(code, reason); } else { @@ -203,6 +214,10 @@ export function ws_wasm_close(ws: WebSocketExtension, code: number, reason: stri export function ws_wasm_abort(ws: WebSocketExtension): void { mono_assert(!!ws, "ERR18: expected ws instance"); + if (ws[wasm_ws_is_aborted] || ws[wasm_ws_close_sent]) { + return; + } + ws[wasm_ws_is_aborted] = true; reject_promises(ws, new Error("OperationCanceledException")); @@ -413,6 +428,8 @@ type WebSocketExtension = WebSocket & { [wasm_ws_pending_send_promises]: PromiseController[] [wasm_ws_pending_close_promises]: PromiseController[] [wasm_ws_is_aborted]: boolean + [wasm_ws_close_received]: boolean + [wasm_ws_close_sent]: boolean [wasm_ws_receive_status_ptr]: VoidPtr [wasm_ws_pending_send_buffer_offset]: number [wasm_ws_pending_send_buffer_type]: number @@ -446,4 +463,9 @@ function resolvedPromise(): Promise | null { // in practice the `resolve()` callback would arrive before the `reject()` of the cancelation. return wrap_as_cancelable(resolved); } -} \ No newline at end of file +} + +function rejectedPromise(message: string): Promise | null { + const resolved = Promise.reject(new Error(message)); + return wrap_as_cancelable(resolved); +} From 3a665a257fdbec19d6f5a1f6651f82918f4362c8 Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Mon, 15 Jan 2024 17:42:23 +0100 Subject: [PATCH 3/9] Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marek Fišera --- .../System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs index 3382103b404f78..b119439dafe90c 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs @@ -29,7 +29,8 @@ internal static partial class BrowserInterop public static string? GetCloseStatusDescription(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) return null; + if (webSocket == null || webSocket.IsDisposed) + return null; string? description = webSocket.GetPropertyAsString("close_status_description"); return description; } From c381d9cb14c3bcaff9d1df099e24284a4c924680 Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Mon, 15 Jan 2024 17:42:32 +0100 Subject: [PATCH 4/9] Update src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marek Fišera --- .../System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs index b119439dafe90c..1b6e37d0bb9b79 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs @@ -18,7 +18,8 @@ internal static partial class BrowserInterop public static WebSocketCloseStatus? GetCloseStatus(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) return null; + if (webSocket == null || webSocket.IsDisposed) + return null; if (!webSocket.HasProperty("close_status")) { return null; From 6e02d4715ff43b84731e9f92406981a027f31580 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 15 Jan 2024 17:52:39 +0100 Subject: [PATCH 5/9] fix --- .../BrowserWebSockets/BrowserInterop.cs | 24 ++++++++++++++++--- .../BrowserWebSockets/BrowserWebSocket.cs | 15 ++++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs index 1b6e37d0bb9b79..acfcc684264c21 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs @@ -11,7 +11,11 @@ internal static partial class BrowserInterop { public static string? GetProtocol(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) return null; + if (webSocket == null || webSocket.IsDisposed) + { + return null; + } + string? protocol = webSocket.GetPropertyAsString("protocol"); return protocol; } @@ -19,11 +23,14 @@ internal static partial class BrowserInterop public static WebSocketCloseStatus? GetCloseStatus(JSObject? webSocket) { if (webSocket == null || webSocket.IsDisposed) + { return null; + } if (!webSocket.HasProperty("close_status")) { return null; } + int status = webSocket.GetPropertyAsInt32("close_status"); return (WebSocketCloseStatus)status; } @@ -31,16 +38,27 @@ internal static partial class BrowserInterop public static string? GetCloseStatusDescription(JSObject? webSocket) { if (webSocket == null || webSocket.IsDisposed) + { return null; + } + string? description = webSocket.GetPropertyAsString("close_status_description"); return description; } public static int GetReadyState(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) return -1; + if (webSocket == null || webSocket.IsDisposed) + { + return -1; + } + int? readyState = webSocket.GetPropertyAsInt32("readyState"); - if (!readyState.HasValue) return -1; + if (!readyState.HasValue) + { + return -1; + } + return readyState.Value; } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 8d9e9ebd873506..c0639dcbc76e1f 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -248,8 +248,7 @@ public override void Dispose() } // lock - // if this is finalizer thread, we need to postpone the abort -> dispose - _innerWebSocket?.SynchronizationContext.Post(static _state => + static void Cleanup(object? _state) { var self = (BrowserWebSocket)_state!; var state = self.State; @@ -264,11 +263,17 @@ public override void Dispose() // This will reject/resolve some promises BrowserInterop.WebSocketAbort(self._innerWebSocket!); } - self._innerWebSocket?.Dispose(); - self.responseStatusHandle?.Dispose(); } + self._innerWebSocket?.Dispose(); + self.responseStatusHandle?.Dispose(); + } - }, this); +#if FEATURE_WASM_THREADS + // if this is finalizer thread, we need to postpone the abort -> dispose + _innerWebSocket?.SynchronizationContext.Post(Cleanup, this); +#else + Cleanup(); +#endif } private void ThrowIfDisposed() From ab5c5ffd8acd0297178e56f92b685a11fc8b6b09 Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Mon, 15 Jan 2024 18:11:41 +0100 Subject: [PATCH 6/9] fix --- .../System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index c0639dcbc76e1f..1aaed3c42d92c2 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -272,7 +272,7 @@ static void Cleanup(object? _state) // if this is finalizer thread, we need to postpone the abort -> dispose _innerWebSocket?.SynchronizationContext.Post(Cleanup, this); #else - Cleanup(); + Cleanup(this); #endif } From cd8fc46bd7da397e472b09320ce12c7596b7bd8e Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Mon, 15 Jan 2024 18:38:43 +0100 Subject: [PATCH 7/9] whitespace --- .../System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs index acfcc684264c21..53f43c3c8592f0 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserInterop.cs @@ -11,7 +11,7 @@ internal static partial class BrowserInterop { public static string? GetProtocol(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) + if (webSocket == null || webSocket.IsDisposed) { return null; } @@ -48,7 +48,7 @@ internal static partial class BrowserInterop public static int GetReadyState(JSObject? webSocket) { - if (webSocket == null || webSocket.IsDisposed) + if (webSocket == null || webSocket.IsDisposed) { return -1; } From d18155a0f85743b932bf5cb5e7236f56f1992286 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 15 Jan 2024 21:06:23 +0100 Subject: [PATCH 8/9] fix --- .../Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index 1aaed3c42d92c2..fe041f34080ac0 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -347,6 +347,10 @@ private async Task ConnectAsyncCore(CancellationToken cancellationToken) } catch (Exception) { + lock (_lockObject) + { + FastState = WebSocketState.Closed; + } Dispose(); throw; } From b9659a5d4b27f2f7bd37d00891113cfbe1676358 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Tue, 16 Jan 2024 10:58:08 +0100 Subject: [PATCH 9/9] fix --- .../BrowserWebSockets/BrowserWebSocket.cs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs index fe041f34080ac0..c635a3d48aad81 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs @@ -542,18 +542,14 @@ private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus, string? stat private async Task CancellationHelper(Task promise, CancellationToken cancellationToken, WebSocketState previousState, IDisposable? disposable = null) { - if (cancellationToken.IsCancellationRequested) - { - disposable?.Dispose(); - cancellationToken.ThrowIfCancellationRequested(); - } - if (promise.IsCompletedSuccessfully) - { - disposable?.Dispose(); - return; - } try { + cancellationToken.ThrowIfCancellationRequested(); + if (promise.IsCompletedSuccessfully) + { + disposable?.Dispose(); + return; + } if (promise.IsCompleted) { // don't have to register for cancelation @@ -570,7 +566,7 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati return; } } - catch (JSException ex) + catch (Exception ex) { lock (_lockObject) { @@ -580,7 +576,15 @@ private async Task CancellationHelper(Task promise, CancellationToken cancellati ForceReadCloseStatusLocked(); throw new OperationCanceledException(nameof(WebSocketState.Aborted), ex); } - + if (ex is OperationCanceledException) + { + if(state != WebSocketState.Closed) + { + FastState = WebSocketState.Aborted; + } + _cancelled = true; + throw; + } if (state != WebSocketState.Closed && cancellationToken.IsCancellationRequested) { FastState = WebSocketState.Aborted;