Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocketModule.OnClientDisconnectedAsync called twice when closing and reopening websocket #528

Closed
azixMcAze opened this issue Jul 21, 2021 · 1 comment · Fixed by #534
Labels
area:websockets Issue with WebSocket protocol bug pinned Pinned issues are not closed automatically when stale. v3.x

Comments

@azixMcAze
Copy link

azixMcAze commented Jul 21, 2021

Context

Hello,
I am doing some tests with websockets. I close an existing websocket and I open a new one just after:

if(ws)
   ws.close();
ws = new WebSocket(`ws://${host}/ws`, []);

I have an minimal WebsocketModule:

    class TestWebSocketModule : WebSocketModule
    {
        public TestWebSocketModule(string urlPath) : base(urlPath, false) { }

        protected override Task OnMessageReceivedAsync(IWebSocketContext context, byte[] buffer, IWebSocketReceiveResult result)
        {
            string messageStr = Encoding.GetString(buffer);
            Debug.Log($"[{context.Id}] OnMessageReceivedAsync\n{messageStr}");
            return Task.CompletedTask;
        }

        protected override Task OnClientConnectedAsync(IWebSocketContext context)
        {
            Debug.Log($"[{context.Id}] OnClientConnectedAsync");
            return Task.CompletedTask;
        }

        protected override Task OnClientDisconnectedAsync(IWebSocketContext context)
        {
            Debug.Log($"[{context.Id}] OnClientDisconnectedAsync");
            return Task.CompletedTask;
        }
    }

Observed Behaviour

The second time I re-open socket, OnClientDisconnectedAsync is called twice for the existing websocket that I juste closed.

I get these log messages:

[189a7lnBhUe+ZWx0n6Dmtw] OnClientConnectedAsync (initial open)
[ce14wIQRbkyfBgmzkNkQTA] OnClientConnectedAsync (1st repoen)
[189a7lnBhUe+ZWx0n6Dmtw] OnClientDisconnectedAsync
[ce14wIQRbkyfBgmzkNkQTA] OnClientDisconnectedAsync (2nd reopen)
[STMccCfqYkq/83YctmrMbA] OnClientConnectedAsync
[ce14wIQRbkyfBgmzkNkQTA] OnClientDisconnectedAsync

After tracing the code, It seems that OnClientDisconnectedAsync is called the first time from PurgeDisconnectedContexts() in OnRequestAsync() and the second time from the finallyblock in OnRequestAsync().

Enabling the watchdog or not does not change the behaviour

Expected Behavior

I expect that OnClientDisconnectedAsync is only called once per context

Versions

I observed the problem with EmbedIO v3.4.3 from nuget and build from source.
I tested with Firefox 89 and Chrome 91 on mac.
EmbedIO is called from the Unity Editor.

@rdeago rdeago added area:websockets Issue with WebSocket protocol bug pinned Pinned issues are not closed automatically when stale. v3.x labels Jul 21, 2021
@rdeago
Copy link
Collaborator

rdeago commented Jul 21, 2021

Hello @azixMcAze, so nice to have you back here, despite the unhappy circumstances.

Thanks for the detailed report. Unfortunately I won't be able to investigate this issue in the next couple weeks. Maybe @geoperez might want to chime in; otherwise I'm starting to work on it on the second week of August. I have pinned the issue so it doesn't get marked as stale in the meantime.

radioegor146 added a commit to radioegor146/embedio that referenced this issue Oct 29, 2021
rdeago pushed a commit that referenced this issue Jan 19, 2022
rdeago added a commit to rdeago/embedio that referenced this issue Feb 3, 2022
rdeago added a commit to rdeago/embedio that referenced this issue Jun 9, 2022
@rdeago rdeago mentioned this issue Jun 9, 2022
rdeago added a commit that referenced this issue Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:websockets Issue with WebSocket protocol bug pinned Pinned issues are not closed automatically when stale. v3.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants