Fixed issue where the pinger would attempt to ping while the socket is closing

[psahgal]

This was causing a crash in my application. The call to reset the pinger object was occurring while the app was trying to disconnect the socket, which was causing pings to sometimes occur while the socket was closed. This was also an issue when the socket was opening. I fixed the issue by only resetting the pinger when the socket opens.

[robhogan]

Thanks for the PR! I'm not sure this is the right approach though - the reset happens onmessage for a reason - to make sure we don't send a keepalive message unless there has been a period of inactivity. This change appears to make the ping happen at regular intervals regardless of other messages being sent.

I'm curious about this crash. Did you get a stack trace or any more info? The best solution might just be to guard against attempting to send messages while we're in (dis)connecting state, if that's the problem.

[psahgal]

Sorry for the late response. Here's the stack trace:

I did some debugging and I noticed this behavior:

1. When disconnecting, the code adds a message to the queue, to tell the other end it wants to disconnect.
2. When that message is sent, and while we're waiting for it to process on the other end, we reset the pinger.
3. Resetting the pinger causes it to ping after a given interval, and that ping could occur after the socket has already been disconnected. (I think this scenario is what leads to the stack trace.)

You are correct: I changed things so that the pings are at regular intervals. I guess I didn't fully understand the keep-alive logic here. Perhaps a better solution would be to stop pinging altogether when sending a disconnect message over the socket? I agree that stopping other messages while attempting to disconnect would be a good idea, too.

[robhogan]

Thanks for the clarification. It does seem like there might be a very small window between the raw socket closing and the ping timer being cancelled (it is cancelled in the _disconnected() callback).

My inclination is simply to wrap the send() call within _doPing() in a try/catch/ignore, since we really don't care about the ping failing in any circumstances I can think of.

I wonder whether I'm missing something here though - the window is so small it just seems unlikely you'd hit this bug repeatedly, unless there's another explanation (like the _disconnected() callback not being called at all for some reason).

[sebirdman]

I've seen this error a few times on my app as well.