From 866acfce4767c039d7c8bd9e91f4e498c162ea8f Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 30 Apr 2024 11:35:56 +0100 Subject: [PATCH] websockets: fix ping_timeout * Closes #3258 * Fixes an issue with the calculation of ping timeout interval that could cause connections to be erroneously timed out and closed from the server end. --- tornado/test/websocket_test.py | 8 ++++++-- tornado/websocket.py | 34 ++++++++++++---------------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/tornado/test/websocket_test.py b/tornado/test/websocket_test.py index 4d39f37046..f279d0706e 100644 --- a/tornado/test/websocket_test.py +++ b/tornado/test/websocket_test.py @@ -806,7 +806,11 @@ class PingHandler(TestWebSocketHandler): def on_pong(self, data): self.write_message("got pong") - return Application([("/", PingHandler)], websocket_ping_interval=0.01) + return Application( + [("/", PingHandler)], + websocket_ping_interval=0.01, + websocket_ping_timeout=1, + ) @gen_test def test_server_ping(self): @@ -827,7 +831,7 @@ def on_ping(self, data): @gen_test def test_client_ping(self): - ws = yield self.ws_connect("/", ping_interval=0.01) + ws = yield self.ws_connect("/", ping_interval=0.01, ping_timeout=1) for i in range(3): response = yield ws.read_message() self.assertEqual(response, "got ping") diff --git a/tornado/websocket.py b/tornado/websocket.py index fbfd700887..5386fae4dd 100644 --- a/tornado/websocket.py +++ b/tornado/websocket.py @@ -835,7 +835,6 @@ def __init__( self._wire_bytes_in = 0 self._wire_bytes_out = 0 self.ping_callback = None # type: Optional[PeriodicCallback] - self.last_ping = 0.0 self.last_pong = 0.0 self.close_code = None # type: Optional[int] self.close_reason = None # type: Optional[str] @@ -1303,38 +1302,29 @@ def start_pinging(self) -> None: """Start sending periodic pings to keep the connection alive""" assert self.ping_interval is not None if self.ping_interval > 0: - self.last_ping = self.last_pong = IOLoop.current().time() self.ping_callback = PeriodicCallback( self.periodic_ping, self.ping_interval * 1000 ) self.ping_callback.start() - def periodic_ping(self) -> None: - """Send a ping to keep the websocket alive + async def periodic_ping(self) -> None: + """Send a ping and wait for a pong if ping_timeout is configured. Called periodically if the websocket_ping_interval is set and non-zero. """ - if self.is_closing() and self.ping_callback is not None: - self.ping_callback.stop() - return - - # Check for timeout on pong. Make sure that we really have - # sent a recent ping in case the machine with both server and - # client has been suspended since the last ping. now = IOLoop.current().time() - since_last_pong = now - self.last_pong - since_last_ping = now - self.last_ping - assert self.ping_interval is not None - assert self.ping_timeout is not None - if ( - since_last_ping < 2 * self.ping_interval - and since_last_pong > self.ping_timeout - ): - self.close() - return + # send a ping self.write_ping(b"") - self.last_ping = now + + if self.ping_timeout and self.ping_timeout > 0: + # wait for the pong + await asyncio.sleep(self.ping_timeout) + + # close the connection if the pong is not received within the + # configured timeout + if self.last_pong - now > self.ping_timeout: + self.close() def set_nodelay(self, x: bool) -> None: self.stream.set_nodelay(x)