Description
ReceiveAsync can receive and handle the close frame, after CloseAsync has started the internal receive task.
Then, when the CloseAsync->ReceiveAsyncPrivate enters the receiveMutex it will throw because the WebSocket has been disposed. This results in a "failed" CloseAsync, despite the closing handshake and connection termination being successful.
Here is an outline of the events (there are some slight variants in logging, its a race condition after all):
[i will use the name ReceiveAsync to indicate the user calls and ReceiveAsyncPrivate for the call from within CloseAsync, just for easier comprehension, also most of CloseAsync is technically CloseAsyncPrivate]
ReceiveAsync enters mutex and waits for message
CloseAsync send close message
CloseAsync waits for mutex to test whether a close frame has already been received
ReceiveAsync receives any message and returns it (unblocks mutex)
CloseAsync enters mutex
ReceiveAsync is called again by the receiveLoop (waits for mutex)
CloseAsync determines that the close frame is missing and internally calls ReceiveAsyncPrivate
ReceiveAsyncPrivate queues for the mutex as well
CloseAsync releases mutex
ReceiveAsync enters mutex
ReceiveAsync receives close frame and handles it (since the new state is Closed, this involves Abort() and therefore Dispose)
ReceiveAsync leaves mutex and completes (the state is now Closed, so the receiveLoop stops)
ReceiveAsyncPrivate enters mutex
ReceiveAsyncPrivate calls ThrowIfDisposed, which throws
ReceiveAsyncPrivate wraps the error in a WebSocketException (WRONGLY ConnectionClosedPrematurely). This wasted a lot of time during my initial debugging
ReceiveAsyncPrivate then leaves mutex
CloseAsync rethrows that WebSocketException, giving the wrong impression that CloseAsync failed
There are multiple issues here, that i think should be addressed:
CloseAsync should not throw after a successful closing handshake has taken place. I propose a check in the catch in CloseAsyncPrivate currently used for logging. If the _state is Closed, don't rethrow the error.- The logic for which WebSocketException to throw in ReceiveAsyncPrivate is faulty. It just always throws
ConnectionClosedPrematurely. This should probably be a separate WebSocketClosed version. The first point might seem to address the wrong exception being propagated to the user. However during my many tests i also had an instance where this error was thrown in the receiveLoop ReceiveAsync instead. It is still incorrect here and it would be nice if the user does not receive wrong and misleading error messages. Some kind of WebSocketClosed would clearly indicate that ReceiveAsync threw because another thread (correctly) closed the connection while it was waiting for messages.
- A very minor thing, but the internal documentation for
WaitForServerToCloseConnectionAsync does not make it clear that the catch is the happy path. It would be nice for people like me who a re not super familiar with this websocket implementation to make it explicit. Either by moving the Abort in a finally or through a comment.
Reproduction Steps
Attached is short no dependency script that can reproduce the error consistently. It also produces a log that can be used to understand the exact order of events outlined above that leads to the incorrect error.
This script was used to reproduce the error on an m1 mac, an M4 mac and windows, all with .net10.
.net 8 and before will not produce the log because NetEventSource was only added in .net9. But the error thrown in the one .net8 test i ran looked like it also has the same kind of issue.
Program.cs
I can provide my collected logs if desired/neddded.
Expected behavior
CloseAsync should not throw an exception at all if the connection was closed successfully. Also ReceiveAsync(Private) should not throw a WebSocketException that indicates failure to complete the closing handshake, when the closing handshake was in fact successful.
Actual behavior
ReceiveAsyncPrivate throws a WebSocketException(WebSocketError.ConnectionClosedPrematurely) if another receive handled the close frame. This leads to either CloseAsync wrongly failing or ReceiveAsync throwing. Both with the objectively false ConnectionClosedPrematurely error message.
Regression?
Not since .net8 at least, but without the internal logs i did not want to try older versions.
Known Workarounds
You can wrap the CloseAsync in a try catch and check whether the WebSocket.State is Closed. If yes then it can be ignored. The same can be done for ReceiveAsync in a receiveLoop.
Configuration
Most of my tests were run on:
- M1 Macbook Pro, Tahoe 26.5.2, .net 10.0.8
I could easily reproduce on:
- M4 Macbook Pro, Sequoia 15.7.4, .net 10.0.3
- Windows (don't know details, i just asked a friend), .net 10.0.10
On windows the first test was with .net 8.0.13, without the internal logs i can only confirm that the exception thrown by CloseAsync is the same (and the State is still Closed, so almost certainly the same issue).
Other information
No response
Description
ReceiveAsync can receive and handle the close frame, after
CloseAsynchas started the internal receive task.Then, when the
CloseAsync->ReceiveAsyncPrivateenters the receiveMutex it will throw because the WebSocket has been disposed. This results in a "failed"CloseAsync, despite the closing handshake and connection termination being successful.Here is an outline of the events (there are some slight variants in logging, its a race condition after all):
[i will use the name
ReceiveAsyncto indicate the user calls andReceiveAsyncPrivatefor the call from withinCloseAsync, just for easier comprehension, also most ofCloseAsyncis technicallyCloseAsyncPrivate]ReceiveAsyncenters mutex and waits for messageCloseAsyncsend close messageCloseAsyncwaits for mutex to test whether a close frame has already been receivedReceiveAsyncreceives any message and returns it (unblocks mutex)CloseAsyncenters mutexReceiveAsyncis called again by the receiveLoop (waits for mutex)CloseAsyncdetermines that the close frame is missing and internally callsReceiveAsyncPrivateReceiveAsyncPrivatequeues for the mutex as wellCloseAsyncreleases mutexReceiveAsyncenters mutexReceiveAsyncreceives close frame and handles it (since the new state is Closed, this involves Abort() and therefore Dispose)ReceiveAsyncleaves mutex and completes (the state is now Closed, so the receiveLoop stops)ReceiveAsyncPrivateenters mutexReceiveAsyncPrivatecalls ThrowIfDisposed, which throwsReceiveAsyncPrivatewraps the error in a WebSocketException (WRONGLYConnectionClosedPrematurely). This wasted a lot of time during my initial debuggingReceiveAsyncPrivatethen leaves mutexCloseAsyncrethrows that WebSocketException, giving the wrong impression that CloseAsync failedThere are multiple issues here, that i think should be addressed:
CloseAsyncshould not throw after a successful closing handshake has taken place. I propose a check in the catch inCloseAsyncPrivatecurrently used for logging. If the _state is Closed, don't rethrow the error.ConnectionClosedPrematurely. This should probably be a separateWebSocketClosedversion. The first point might seem to address the wrong exception being propagated to the user. However during my many tests i also had an instance where this error was thrown in the receiveLoopReceiveAsyncinstead. It is still incorrect here and it would be nice if the user does not receive wrong and misleading error messages. Some kind ofWebSocketClosedwould clearly indicate that ReceiveAsync threw because another thread (correctly) closed the connection while it was waiting for messages.WaitForServerToCloseConnectionAsyncdoes not make it clear that the catch is the happy path. It would be nice for people like me who a re not super familiar with this websocket implementation to make it explicit. Either by moving the Abort in a finally or through a comment.Reproduction Steps
Attached is short no dependency script that can reproduce the error consistently. It also produces a log that can be used to understand the exact order of events outlined above that leads to the incorrect error.
This script was used to reproduce the error on an m1 mac, an M4 mac and windows, all with .net10.
.net 8 and before will not produce the log because NetEventSource was only added in .net9. But the error thrown in the one .net8 test i ran looked like it also has the same kind of issue.
Program.cs
I can provide my collected logs if desired/neddded.
Expected behavior
CloseAsync should not throw an exception at all if the connection was closed successfully. Also ReceiveAsync(Private) should not throw a WebSocketException that indicates failure to complete the closing handshake, when the closing handshake was in fact successful.
Actual behavior
ReceiveAsyncPrivate throws a WebSocketException(WebSocketError.ConnectionClosedPrematurely) if another receive handled the close frame. This leads to either CloseAsync wrongly failing or ReceiveAsync throwing. Both with the objectively false ConnectionClosedPrematurely error message.
Regression?
Not since .net8 at least, but without the internal logs i did not want to try older versions.
Known Workarounds
You can wrap the CloseAsync in a try catch and check whether the WebSocket.State is Closed. If yes then it can be ignored. The same can be done for ReceiveAsync in a receiveLoop.
Configuration
Most of my tests were run on:
I could easily reproduce on:
On windows the first test was with .net 8.0.13, without the internal logs i can only confirm that the exception thrown by CloseAsync is the same (and the State is still Closed, so almost certainly the same issue).
Other information
No response