async void with unhandled exception in WebSocketTransportDuplexSessionChannel.cs

[cangosta]

Describe the bug I don't have a way of reproducing it, but I found it by looking at the code. In file: wcf/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/WebSocketTransportDuplexSessionChannel.cs you have a a class WebSocketMessageSource that has a method: async void StartNextReceiveAsync()

it has an exception being thrown: if (currentState != AsyncReceiveState.Finished) { throw FxTrace.Exception.AsError(new InvalidOperationException()); }

In the case of this exception, aren't we crashing the process?

[mconnew]

If that exception is thrown, I believe you would terminate the process, but that check should never throw. It acts more like an assert to check for a condition which should never happen. The state starts as Finished, and a CompareExchange happens to move it to Started. The result of CompareExchange returns what the original value was, which is what's being checked here. It's basically saying if you call StartNextReceiveAsync when the state is already started, that's a catastrophic failure for the channel. There are then 2 ways that the receive can finish, one is to actually receive data, the other is to be cancelled, which includes timing out. In both those cases, we transition back to Finished. It's only once data has been received that we should call the method again.

Aborting the entire process is overkill, but this isn't a condition which should ever happen under any circumstances so the reality is it's safe.

There is another method in the same file that doesn't look as safe though. WriteEndOfMessageAsync looks like it can throw an uncaught exception. The conditions under which that can happen are rare. We have the same code on .NET Framework and I've never seen a report of the problem, but it still looks like it should be improved.