Skip to content

Reliability Improvements on Transport Tier#148

Closed
roamm wants to merge 8 commits intosocketio:masterfrom
roamm:socket_tier_improvement
Closed

Reliability Improvements on Transport Tier#148
roamm wants to merge 8 commits intosocketio:masterfrom
roamm:socket_tier_improvement

Conversation

@roamm
Copy link
Contributor

@roamm roamm commented Feb 20, 2013

Firstly I'd like to say I don't run into any obvious bugs without these changes.

Because Socket has some implicit assumptions like:

  1. never call this.transport.close() more than once;
  2. never rely on 'close' event from the underlying transport for a forced close;
  3. the polling transport can be in an 'opening' state even Socket has closed;
  4. callback of transport.close() is not reentrant;
  5. 'close' event (not triggered in current implementation) from the old transport will break the logic while upgrading;

These assumptions make the code kinda fragile so I took some time trying to fix it and hope this would help.

@rauchg
Copy link
Contributor

rauchg commented Mar 19, 2013

@roamm can we make more focused pull requests? Like one for idempotent close, that could be a PR by itself. Will make reviewing easier

@rauchg
Copy link
Contributor

rauchg commented Mar 19, 2013

Thanks!

@plievone
Copy link

@guille, are there important failing test cases here? roamm@aa0209f

darrachequesne pushed a commit that referenced this pull request May 8, 2020
Don't check against the window to detect node.js, wrap code in a try cat...
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.

3 participants