Deadlock in WebSocketBase

[fairful]

Description

I've come across a deadlock in System.Net.WebSockets.WebSocketBase. I'm running .Net 9. It's happening when a web socket is being closed from both ends simultaneously. See attached screenshot of Parallel Stacks.

Looking at the code of WebSocketBase, it's pretty obvious what's happening. In one thread, handling the incoming close request, you get this:

• WebSocketBase.Process, line 1362, takes lock on SessionHandle
• WebSocketBase.Process calls StartOnCloseReceived
• WebSocketBase.StartOnCloseReceived, line 1220 takes lock on _thisLock

In the other thread, handling the local close request, you get this:

• WebSocketBase.CloseAsyncCore, line 616, takes lock on _thisLock
• WebSocketBase.CloseAsyncCore calls CloseOutputAsync
• WebSocketBase.CloseOutputAsyncCore calls TakeLocks
• WebSocketBase.TakeLocks, line 886 takes lock on SessionHandle

Classic deadlock - two threads taking the same two locks in different order.

Reproduction Steps

I'm sorry I can't supply a snippet, but closing a socket from both ends simultaneously, using WebSocket.CloseAsync should do it.

Expected behavior

Socket closes without deadlock

Actual behavior

Deadlock!

Regression?

No response

Known Workarounds

Don't close sockets from both ends. This is happening for me in a test, where two connected processes are being shut down simultaneously. They each close all the sockets they have between them - there can be several and each can have been created by either party. A workaround is difficult in this circumstance.

Configuration

.Net 9, Windows 11, x64

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[MihaZupan]

cc: @rokonec (HttpListener)

[CarnaViire]

TIL WebSockets in HttpListener DON'T use the managed implementation, if running on Windows 🙃😅

There is a flag ForceManagedImplementation, but it's HttpListener-compile-time only, so it seems to be impossible to invoke the managed alternative on Windows in any released .NET.

I'd propose to:

• Change the compile flag into an AppContext switch/feature flag
  (in a way that the trimmer can remove the unused alternative in the end-user apps)

• Consider making the managed implementation to be a default in HttpListener
  (technically breaking, but it would be possible to opt-out)

This should be a small and straightforward change, that would also unify Windows and Linux implementation (as it was done long time ago in HttpClient with SocketsHttpHandler).

NOTE: this will affect the whole HttpListener, not only WebSockets.

Is there any kind of a feature gap that prevents us from doing that? /cc @rokonec @karelz

[karelz]

Triage: One report so far. Moving to managed implementation will be best - not chance in .NET 10.
We will investigate a targeted fix only if it hurts a lot, or more customers are impacted. Moving to 11.

[fairful]

I've managed to work round the issue, so no longer any pain here. Concur with moving to managed implementation in 11.