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

Ping Loop Event not getting cleared on reconnect #153

Closed
harmon opened this issue Dec 10, 2019 · 5 comments
Closed

Ping Loop Event not getting cleared on reconnect #153

harmon opened this issue Dec 10, 2019 · 5 comments
Assignees
Labels

Comments

@harmon
Copy link
Contributor

harmon commented Dec 10, 2019

I can't quite put my finger on the exact issue, but we are getting an unexpected disconnect after a client.py reconnect because I think the ping_loop_event is no longer getting cleared with a recent code change in v3.11.0:

e38daad

Was this self.ping_loop_event.clear() supposed to be removed?

09:41:12	- INFO     - [123145481080832 socketio] - engineio.client - 	WebSocket connection accepted with {'sid': 'f27467c12f3c4d85a2925bb4e9f1653f', 'upgrades': [], 'pingTimeout': 60000, 'pingInterval': 25000}
09:41:12	- INFO     - [123145481080832 socketio] - socketio.client - 	Engine.IO connection established
09:41:12	- INFO     - [123145465315328 Thread-5] - engineio.client - 	Sending packet PING data None
09:41:12	- INFO     - [123145465315328 Thread-5] - engineio.client - 	PONG response has not been received, aborting
09:41:12	- INFO     - [123145465315328 Thread-5] - engineio.client - 	Exiting ping task
09:41:12	- WARNING  - [123145470570496 Thread-6] - engineio.client - 	WebSocket connection was closed, aborting

I realize I don't have much detail in this ticket, but if I get time this week I can flush it out with more info on how to recreate.

@harmon harmon changed the title Ping Event not getting cleared Ping Loop Event not getting cleared Dec 10, 2019
@harmon
Copy link
Contributor Author

harmon commented Dec 10, 2019

Ok, I guess that's fine, since the default state of a new Event is already "cleared()", something else about that version bump must be causing my issues. Still looking.

@harmon
Copy link
Contributor Author

harmon commented Dec 10, 2019

Ahh, yes! Ok, figured it out: On reconnect, the self.ping_loop_event already exists, and does not get cleared again, so it basically disconnects again immediately.

The change from:

    def _ping_loop(self):
        """This background task sends a PING to the server at the requested
        interval.
        """
        self.pong_received = True
        self.ping_loop_event.clear()

To:

    def _ping_loop(self):
        """This background task sends a PING to the server at the requested
        interval.
        """
        self.pong_received = True

        if self.ping_loop_event is None:
            self.ping_loop_event = self.create_event()

Means that the second time a _ping_loop thread is created, it isn't reset.

Maybe it needs:

    def _ping_loop(self):
        """This background task sends a PING to the server at the requested
        interval.
        """
        self.pong_received = True

        if self.ping_loop_event is None:
            self.ping_loop_event = self.create_event()
+       else:
+           self.ping_loop_event.clear()

@harmon harmon changed the title Ping Loop Event not getting cleared Ping Loop Event not getting cleared on reconnect Dec 10, 2019
@miguelgrinberg miguelgrinberg self-assigned this Dec 10, 2019
@miguelgrinberg
Copy link
Owner

Yes, I messed this up with that change. I just put a fix in master. Would you like to give it a try before I cut a new release?

@harmon
Copy link
Contributor Author

harmon commented Dec 10, 2019

I tested the fix in my own file, so I think it's good.

@harmon
Copy link
Contributor Author

harmon commented Dec 10, 2019

I just merged a requirements lockdown to 3.10.0 to our code, and then you fixed it!

Such appreciation, much service!

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

No branches or pull requests

2 participants