WebSocketSharp unit test failure due to StackOverflow

[yallie]

Describe the bug
WebSocketSharp channel unit tests occasionally fail due to stack overflows (quite rare actually).

To Reproduce
Run WebSocketSharp unit tests until they fail. Not guaranteed!

Expected behavior
The unit tests should not produce stack overflow.

Additional notes

========== Starting test run ==========
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.5.1.1+5fe7eebc65 (64-bit .NET 6.0.36)
[xUnit.net 00:00:00.08]   Starting:    CoreRemoting.Tests
Stack overflow.
   at WebSocketSharp.WebSocketFrame.readExtendedPayloadLengthAsync(System.IO.Stream, WebSocketSharp.WebSocketFrame, System.Action`1<WebSocketSharp.WebSocketFrame>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.Ext+<>c__DisplayClass58_0.<ReadBytesAsync>b__0(System.IAsyncResult)
   at System.Threading.Tasks.TaskToApm+TaskAsyncResult..ctor(System.Threading.Tasks.Task, System.Object, System.AsyncCallback)
   at System.Net.Sockets.NetworkStream.BeginRead(Byte[], Int32, Int32, System.AsyncCallback, System.Object)
   at WebSocketSharp.Ext.ReadBytesAsync(System.IO.Stream, Int32, System.Action`1<Byte[]>, System.Action`1<System.Exception>)
   at WebSocketSharp.WebSocket+<>c__DisplayClass170_0.<startReceiving>b__1(WebSocketSharp.WebSocketFrame)
...
(dosens of pages of the same call stack frames omitted)

PS. Disabled WebSocketSharp tests for now (this channel is going to be superseded by new HttpListener-based websocket channel) — see commit f8abb35.

[sancheolz]

System.Net.Sockets.NetworkStream.BeginRead can sometimes call a callback in the same thread in which it was called. I came across something similar in Socket.BeginRead when switching from .Net Framework to .net. It seems that in .net, the probability of calling a callback in the same thread is much higher due to some optimizations. It was decided to call a task in the callback that performs processing if the callback call was synchronous.

[yallie]

System.Net.Sockets.NetworkStream.BeginRead can sometimes call a callback in the same thread in which it was called.

Hmm, interesting... But I don't see why it should overflow unless the callback starts stream.BeginRead again from the same stream.Position. The same stream.Position means that read bytes count = 0, which indicates the end of the stream, so the process should stop. Looks like bug in application code. Am I missing something?

[sancheolz]

I forgot to clarify, stack overflow occurred if there was data in the buffer at the time of each callback call and if the BeginRead call occurred in a callback that was passed to BeginRead. Apparently, some condition for exiting recursion is not met in the websockets. I also think that this is a bug in the websocket. Whether it will be fixed is unknown.

It looks like there are now two potential stable channel implementations left - quick and httplistener-based websocket.

[anders9ustafsson]

@yallie I can confirm that I have experienced the StackOverflowException problem in websockets-core in my .NET 8 application . Occasionally it also happened when I ran some of the unit tests locally. I have similar problems with my own source code as well, still struggling to find a solution to that.

@sancheolz Did you manage to workaround the recursion issue in your code? In that case, I would be very interested to get some tips :-)

[yallie]

I can confirm that I have experienced the StackOverflowException problem in websockets-core in my .NET 8 application

@anders9ustafsson, please try updating to the latest version of CoreRemoting.
Websocket channels are now based on HttpListener implementation and should work on Windows and Linux.
Legacy WebSocketSharp-based implementation is renamed and extracted in a separate project.

[yallie]

two potential stable channel implementations left - quick and httplistener-based websocket.

@sancheolz, I'm currently using Fleck library in one of my projects.

A few years ago I was evaluating Fleck and websocket-sharp and found that Chromium browsers
fail to connect to websocket-sharp-based servers, but connect to Fleck-based servers just fine.
Not sure if it's still the case though, but back then our team decided to use Fleck for this reason.

Also, there is WebSocket4Net, based on SuperSocket library, but I haven't tried it.

[sancheolz]

@anders9ustafsson you need to check property IAsyncResult.CompletedSynchronously. IAsyncResult is passed to callback wich passed to BeginRead. If CompletedSynchronously returns true then you neen run receive logic in Task.Run, else run it as is.

@yallie thanks for the information. I really want to try out a new implementation of the channel on websocket when I find some time. I think I'll focus on it.

[yallie]

I really want to try out a new implementation of the channel on websocket when I find some time. I think I'll focus on it.

@sancheolz Please keep in mind the channel is still beta.

I'm not sure I've done everything right — it just passes the unit tests, nothing more.
I didn't stress test it and I didn't run any benchmarks on it apart from the unit tests.
Would be grateful for the code review when you have some spare time.

[anders9ustafsson]

Thanks for the info @yallie , I think it is a great step in the right direction to make use of the .NET internal support for WebSockets. I did try it with my application, but for some reason failed to establish contact between the client and the server. Not sure why, I will investigaste further.

And thanks @sancheolz for the info. I have tried to make use of CompletedSynchronously, but have still not been able to avoid recursion, so I am rewriting the code using ReceiveAsync instead.

[yallie]

I did try it with my application, but for some reason failed to establish contact between the client and the server.

I knew it shouldn't be that easy! :-/

— What platform are you using, Windows, Linux?
— Did you run both client and server on the same machine or on different machines?
— Firewall maybe?

My best guess is a problem with HTTP.sys driver permissions. Localhost connections don't require any configuration, that's why the unit tests run without any issues. But accepting connections from other machines requires administrator privileges and setting up namespace reservations with netsh: Configuring HTTP and HTTPS...

[anders9ustafsson]

@yallie I am running on Windows 11, client and server on same machine. I have tested with and without message encryption in case that would play a role, but no difference. I am using .NET binary formatter serialization, but that shouldn't matter either, right?

[yallie]

@yallie I am running on Windows 11, client and server on same machine.

That's weird...

I have tested with and without message encryption in case that would play a role, but no difference. I am using .NET binary formatter serialization, but that shouldn't matter either, right?

No, it shouldn't matter.
The unit tests are executed with different serializers and encryption settings.

Have you tried running CoreRemoting unit tests on your machine?
They succeed on my Windows 11 laptop and on github-actions Ubuntu Linux images.

But do they work for your setup? If they do, please try to spot the difference
between the unit tests and your application.

[anders9ustafsson]

Unit tests succeed. I will investigate more thoroughly to identify the root cause of the failing communication in my application, I'll get back to you as soon as I have more information.

[anders9ustafsson]

@yallie Here is what I have found so far. My server was invoked with HostName set to "localhost", whereas the client was invoked with ServerHostName equal to the name of my computer. This resulted in the server not being able to recognize that the client was trying to connect. Changing the client's ServerHostName to "localhost" made the server to recognize the connection attempt.

Ideally though, it should not matter if the host name differs in the server and client invocation, as long as the same machine is referenced.

I am still having problems with the handshake; the server has issued the handshake but the client is not able to recognize it. I'll continue my investigation.

BTW, looking at the await:ed code in the websocket classes, the use of ConfigureAwait(false) seems to be inconsistent. Is this deliberate?

[yallie]

Ideally though, it should not matter if the host name differs in the server and client invocation, as long as the same machine is referenced.

Yes, I think it shouldn't matter, too.

BTW, looking at the await:ed code in the websocket classes, the use of ConfigureAwait(false) seems to be inconsistent. Is this deliberate?

I tend to use ConfigureAwait(false) every time I remember about using it.
Especially when I wait for the result synchronously afterwards, i.e. ConfigureAwait(false).Result.
It discards the captured synchronization context and helps avoiding deadlocks in a single-threaded app.
It also improves performance a bit because restoring the captured context has its cost.
I wish that ConfigureAwait(false) were the default setting.

And as I said, I'm not sure I've done everything right — the code just passes the unit tests, and that's it.
Would be grateful for the code review from you guys.