Stack overflow exception in Websocket.Client library

[spudwebb]

I get a stack overflow crash in our program by doing this:

1. create a ZWaveJS.NET driver and connect it to an external ZWave JS Server
2. stop the ZWave JS server and let the ZWaveJS.NET driver goes into reconnection mode.
3. wait about 10 minutes to trigger hundreds of reconnection attempts
4. call driver.Destroy()

=> a StackOverflowException occurs in ClientWebSocket.Dispose()

It is 100% reproducible in our program, but I wasn't able to reproduce it with a simple console program, so I must be missing something there.

Anyway, I have traced down the problem to be related to this issue in the Websocket.Client library: Marfusios/websocket-client#124
While debugging in VS, if you "break all" while waiting in step 3 from above, you can see the hundred of recursive calls in the task used for reconnection.
A fix for this problem has been implemented in recent version of the Websocket.Client library, but unfortunately this version has dropped support for netstandard2.0 , so we cannot directly update the library from nuget unless netstandard2.0 is dropped in ZWaveJS.NET as well, which is out of question for us because our program still use .NET Framework.

So I'm not sure how to best fix this issue?
For now, I pulled the source code for Websocket.Client version 4.6.1 (the version currently used by ZwaveJS.NET) and I manually merged the fix for the recursive call problem: Marfusios/websocket-client@df2e007
I successfully tested that it fixes my stack overflow problem.

[marcus-j-davies]

Thanks @spudwebb

Looks like I'll need to include the source in to the binary to address it (and to cash in on the benefits).
and see where I can keep support for netstandard2.0 - I'll take the opportunity to bump the schema and (hopefully) provide support for Long Range.

I'll ping you once tested

[marcus-j-davies]

@spudwebb

Seems trying to bend the latest socket client to net standard 2.0 is more challenging than I thought.
can you point to the source for 4.6.1?

[spudwebb]

@marcus-j-davies I think 4.6.1 was this commit: Marfusios/websocket-client@638cf7f

I had to open and build the project with VS 2019 though, using VS 2022 gave me build errors.

[marcus-j-davies]

@spudwebb

I have managed to compile the latest socket client source code with netstandard2.0 support:

• Embeds the latest socket client source code
• Polyfilled where not supported in netstandard2.0
• A few changes in source code of the socket library

If I attach the nupkg are you able to try your luck with it?

I am not sure if surprises are awaiting, but I don't have the means to test this currently

[spudwebb]

yes, I think I can test the nupkg

[marcus-j-davies]

OK, see this branch: https://github.com/zwave-js/ZWaveJS.NET/tree/bump-schema-%2B-Embed-Socket-Client-%2B-LR
And the nupkg that's there also : https://github.com/zwave-js/ZWaveJS.NET/blob/bump-schema-%2B-Embed-Socket-Client-%2B-LR/Visual%20Studio%20Projects/ZWaveJS.NET/ZWaveJS.NET.4.0.0.nupkg

Depending on how successful this is, I will continue to bump the schema

it now directly references some libs used by the socket client, as well as conditionally references some other libs to provide support for netstandard2.0 / net48

[spudwebb]

@marcus-j-davies, it doesn't work as is, the received messages from the websocket is always a bunch of 0.

I changed the code in WebsocketClient.Listen() to use an array as the buffer instead of Memory:

        private async Task Listen(WebSocket client, CancellationToken token)
        {
            Exception? causedException = null;
            try
            {
                // define buffer here and reuse, to avoid more allocation
                const int chunkSize = 1024 * 4;
                var buffer = new byte[chunkSize];
                

                do
                {
                    //  ValueWebSocketReceiveResult result;
                    WebSocketReceiveResult result;
                    var ms = (RecyclableMemoryStream)_memoryStreamManager.GetStream();

                    ArraySegment<byte> bufferSegment = new ArraySegment<byte>(buffer);
                    

                    while (true)
                    {
                        result = await client.ReceiveAsync(bufferSegment, token);
                        ms.Write(buffer, 0, result.Count);

                        if (result.EndOfMessage)
                            break;
                    }

with this change it seems to work, but honestly I'm not super confortable using this version in production as it was not designed to work on netstandard2.0 / net48 in the first place and you had to change a few things to adapt it. I would rather embed version 4.6.1 of the Websocket.Client library in ZWaveJS.NET and only add the one fix that caused the stack overflow.
I can try to do that in a new branch if you want.

[marcus-j-davies]

Hi @spudwebb

I think I agree - feels a bit desperate, leave it with me, as I will work on adding LR support.

[marcus-j-davies]

Fixed in v4