Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Multiple noop packets sent on upgrade #174

Closed
plievone opened this Issue May 31, 2013 · 8 comments

Comments

Projects
None yet
4 participants

Hi, it seems that a client can occasionally get dozens on noop packets from engine.io server during websocket upgrade, perhaps due to checkIntervalTimer in lib/socket.js triggering ten times per second, and not checking if it has succeeded in sending a noop packet already? Or is it intended to just keep sending until upgrade packet is received. The issue is seen mostly behind slow connections, of course.

Contributor

rauchg commented May 31, 2013

master or latest tag?

Contributor

rauchg commented May 31, 2013

@plievone after sending one packet we should clear the timer. GOOOD catch

@rauchg rauchg closed this May 31, 2013

plievone commented Jun 1, 2013

I guess you closed this by accident? :)

Now, perhaps sending multiple noop packets is intentional -- in my brief testing now the client failed to upgrade with only one packet, as it came before polling cycle started pausing and then the situation stalled until eventually ping timeout was emitted and the socket was closed. If it really is so, there might be a problem with the process here... Hopefully it is my setup (open socket, client side close it in 300 ms, then open it again in 1000 ms, then see what happens).

plievone commented Jun 3, 2013

Hi @guille, thanks for this, but unfortunately now the client can miss the noop packet (it comes via polling transport, whereas probe pong comes via websocket, the ordering can change?) and stall in upgrade? Perhaps best for now would be to have a longer, 500 ms interval just in case, and trigger it also on the leading edge separately.

Contributor

rauchg commented Jun 3, 2013

Good point lets patch it

Guillermo Rauch
Sent on my phone

On Monday, June 3, 2013 at 3:21 AM, plievone wrote:

Hi @guille (https://github.com/guille), thanks for this, but unfortunately now the client can miss the noop packet (it comes via polling transport, whereas probe pong comes via websocket, the ordering can change?) and stall in upgrade? Perhaps best for now would be to have a longer, 500 ms interval just in case, and trigger it also on the leading edge separately.


Reply to this email directly or view it on GitHub (LearnBoost#174 (comment)).

@rauchg rauchg reopened this Jun 15, 2013

@rauchg rauchg closed this in d352ccd Jun 15, 2013

@rauchg rauchg reopened this Jun 15, 2013

Contributor

rauchg commented Jun 15, 2013

Open for discussion on how to improve

Contributor

defunctzombie commented Jan 10, 2015

@plievone Is this still an open issue?

@Adrien-P Adrien-P referenced this issue in googollee/go-engine.io Dec 7, 2015

Open

Stall in upgrade (race condition) #33

Contributor

darrachequesne commented Dec 7, 2016

Closed due to inactivity, please reopen if needed.

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