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

Many connections left in readyState 2 #64

Closed
nicokaiser opened this issue Apr 18, 2012 · 13 comments
Closed

Many connections left in readyState 2 #64

nicokaiser opened this issue Apr 18, 2012 · 13 comments

Comments

@nicokaiser
Copy link
Contributor

After running for some hours, there are quite some connections with readyState 2 (WebSocket.CLOSING). They are never closed, even after some minutes (socket timeouts).

This may be the result of a client never reacting to close packets.

I agree that socket.end is nicer than socket.destroy, but there must be some kind of timer that destroys a connection that is CLOSING for too long. This gets even worse when the connection is ended because of handshake failures – then, the connection is invisible for the application (not emitted as connection event) and dangling around forever.

@crickeys
Copy link

Is this problem in socket.io as well?

@nicokaiser
Copy link
Contributor Author

As @einaros already mentioned, especially in the handshake sequence, the error handling in ws may have some issues.

This is what I observed – some connections being stuck in readyState 2, so I added the (quite radical but effective) solution with socket.destroy. This is not very nice (connections should be ended properly) but still better than stuck connection objects.

@nicokaiser
Copy link
Contributor Author

@einaros
Copy link
Contributor

einaros commented May 15, 2012

Thanks for bumping this again. I'll deal with it today.

@einaros
Copy link
Contributor

einaros commented May 19, 2012

I've made quite a few changes now - could you try the current master and see if that improves things on your end?

@crickeys
Copy link

Are there any chances that the fixes you put into the WS library can make there way into the websocket parsers you wrote for socket.io??

@einaros
Copy link
Contributor

einaros commented May 20, 2012

I would prefer that socket.io soon made the switch to use websocket.io. Websocket.io uses the ws parsers internally.

@nicokaiser
Copy link
Contributor Author

@einaros I'll update one of the servers tomorrow, looks good!
I'll then watch if the destroy's I added in WebSocketServer (before a WebSocket object is instantiated) are really needed...

@einaros
Copy link
Contributor

einaros commented May 20, 2012

@nicokaiser I included a similar fix in WebSocketServer, and pushed v0.4.15. Hopefully this will improve things for you this following week.

@nicokaiser
Copy link
Contributor Author

@einaros ok, great! I installed ws 0.4.15 (default settings, this time also with default node.js without any nocrankshaft noopt switches), let's see how it behaves.

@einaros
Copy link
Contributor

einaros commented May 21, 2012

Eagerly awaiting the results :) Hopefully things will be at least a little bit better.

@einaros
Copy link
Contributor

einaros commented Jun 14, 2012

Where do we stand on this now? Does the latest connection shutdown fixes deal with this?

@nicokaiser
Copy link
Contributor Author

Did not have problems for some weeks now. So considering this as fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants