Skip to content

Conversation

@laino
Copy link

@laino laino commented Feb 12, 2014

By sending many upgrade event a client could possible create a lot of intervals without the old ones having a chance of being cleared. I also added the intervals to the constructor and made it use setImmediate instead of process.nextTick.

@laino
Copy link
Author

laino commented Feb 12, 2014

Edit: Don't merge this yet. I'll create a new pull request.

@laino laino closed this Feb 12, 2014
@kapouer
Copy link
Contributor

kapouer commented Feb 12, 2014

you don't have to create a new PR if you want to overwritethe commits
You can clone the repository locally, checkout your patch branch, commit --amend or reset HEAD^ then when you are done, commit and push -f. The PR will be updated.

@laino
Copy link
Author

laino commented Feb 12, 2014

Ah right. I'll do that then. Thanks.

@laino laino reopened this Feb 12, 2014
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to initialize them as null ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that will a) make it clear to the programmer what kinds of attributes this class will have and b) improve performance because the VM will not have to change the internal class at runtime.

@laino
Copy link
Author

laino commented Feb 12, 2014

Now I'm done.

rauchg added a commit that referenced this pull request Feb 12, 2014
Fix possible DOS and small changes to timeouts
@rauchg rauchg merged commit 27141f9 into socketio:master Feb 12, 2014
@rauchg
Copy link
Contributor

rauchg commented Feb 12, 2014

This broke 0.8 - reverting to process.nextTick

@laino
Copy link
Author

laino commented Feb 12, 2014

Ah. I didn't know you're trying to be backwards compatible that far.

@rauchg
Copy link
Contributor

rauchg commented Feb 12, 2014

There's a lot of people on 0.8 still

laino pushed a commit to laino/engine.io that referenced this pull request Jun 19, 2015
Fix possible DOS and small changes to timeouts
darrachequesne pushed a commit that referenced this pull request May 8, 2020
README: remove outdated browserbuild reference
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.

4 participants