Skip to content

Commit

Permalink
websockets: fix ping_timeout
Browse files Browse the repository at this point in the history
* Closes tornadoweb#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.
  • Loading branch information
oliver-sanders committed Apr 30, 2024
1 parent f399f40 commit 380e1b1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 24 deletions.
8 changes: 6 additions & 2 deletions tornado/test/websocket_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")
Expand Down
39 changes: 17 additions & 22 deletions tornado/websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -1230,6 +1229,7 @@ def _handle_message(self, opcode: int, data: bytes) -> "Optional[Future[None]]":
self.close(self.close_code)
elif opcode == 0x9:
# Ping
print(f':ping: {time()}')
try:
self._write_frame(True, 0xA, data)
except StreamClosedError:
Expand Down Expand Up @@ -1303,38 +1303,33 @@ 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()
if self.ping_timeout and self.ping_timeout > self.ping_interval:
raise ValueError(
'the ping_timeout cannot be greater than the ping_interval'
)
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)
Expand Down

0 comments on commit 380e1b1

Please sign in to comment.