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

sanity check that conn exists on connection #637

Merged
merged 2 commits into from
Oct 5, 2016
Merged

Conversation

jskz
Copy link
Contributor

@jskz jskz commented Oct 4, 2016

Low-impact bug fix that prevents an edge case where null is passed (and consequently aborts the server, which is the current behaviour) on a new connection if all handles are occupied.

I am able to reliably trigger this by rapidly refreshing on a page in development.

This prevents an edge case where null is passed (and aborts the server) on a new connection if all handles are occupied.  I am able to reliably trigger this by rapidly refreshing on a page in development.
Copy link
Member

@SpaceK33z SpaceK33z left a comment

Choose a reason for hiding this comment

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

If conn does not exist anymore, maybe it's better to do a return; just before it? So that the code under conn.on("close, ...) also doesn't get executed.

The linter is failing, you have a trailing space somewhere.

@jskz
Copy link
Contributor Author

jskz commented Oct 5, 2016

The check and return now happen immediately, and the spacing issue is resolved.

@SpaceK33z SpaceK33z merged commit 3eb2a92 into webpack:master Oct 5, 2016
@SpaceK33z
Copy link
Member

Thanks!

@jskz jskz deleted the patch-1 branch October 6, 2016 02:53
SpaceK33z pushed a commit that referenced this pull request Oct 6, 2016
This prevents an edge case where null is passed (and aborts the server) on a new connection if all handles are occupied.  I am able to reliably trigger this by rapidly refreshing on a page in development.
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.

None yet

2 participants