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

clearTimeout should be called from the same scope as setTimeout #1568

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

mast
Copy link
Contributor

@mast mast commented Dec 28, 2022

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

In nw.js environment, when socket.io-client is imported as ES module (supposed to run in node.js context of nw.js) I observe following unexpected behavior.
Client successfully establishes connection via WebSocket transport, messages can be exchanged well, ping/pong mechanism is working well, but after 20 seconds connection is getting dropped and re-established again. It happens every 20 seconds exactly.

When I change timeout option of manager to some other value, I see that connection is getting dropped after exactly this value. That made me think that monitoring timer is not getting stopped when connection is successfully established.

I found that nw.js has several JS contexts running and problem disappears when I enable mixed context mode of nw.js (https://docs.nwjs.io/en/latest/For%20Users/Advanced/JavaScript%20Contexts%20in%20NW.js/#mixed-context-mode)

After looking into source code, I found that socket.io-client is starting timer using manager.setTimeoutFn function which is set by engine.io-client. But timer is getting stopped by regular clearTimeout function from current scope.
When I change it to manager.clearTimeoutFn problem disappears.
It seems somehow timer is starting from one scope and is getting stopped by different scope, that's why it's not actually stopped.

When I import socket.io-client as html script (running in browser context), I don't see behaviour and it works as expected.

New behaviour

Using manager.clearTimeoutFn to stop monitoring timer.

Other information (e.g. related issues)

https://docs.nwjs.io/en/latest/For%20Users/Advanced/JavaScript%20Contexts%20in%20NW.js/

Details of change:
 - engine.io-client sets clearTimeoutFn and setTimeoutFn
   function depending on settings passed to manager
 - socker.io-client is using manager.setTimeoutFn to
   start connection monitoring timer, but is using regular
   clearTimeout function to stop it when connection is established
 - In some setups it is causing timer fail to stop and it will break
   connection every _timeout_ milliseconds (which is 20000 by default)
@darrachequesne darrachequesne merged commit f2892ab into socketio:main Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants