From c1878fa42ceb9b4952366872d9e5527bd65448ec Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Fri, 7 Mar 2025 10:16:56 +0100 Subject: [PATCH 01/17] Check for PONG before closing connection for missing PONG Currently, if receive is never called, PONG messages aren't handled and aiohttp closes the connection for missing PONG after half of the heartbeat time has passed. This commit changes the code that closes the connection for missing PONG so that instead of immediately closing the connection, it checks whether a PONG has been sent. If a PONG has been sent, the connection doesn't get closed. --- aiohttp/web_ws.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 78c130179f5..dc9115e10c0 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -185,12 +185,38 @@ def _ping_task_done(self, task: "asyncio.Task[None]") -> None: self._ping_task = None def _pong_not_received(self) -> None: + """Callback for when no PONG was received after self._pong_heartbeat seconds""" if self._req is not None and self._req.transport is not None: - self._handle_ping_pong_exception( - asyncio.TimeoutError( - f"No PONG received after {self._pong_heartbeat} seconds" - ) + loop = self._loop + if loop is not None: + loop.create_task(self._pong_not_received_coro()) + else: + self._handle_ping_pong_exception( + asyncio.TimeoutError( + f"No PONG received after {self._pong_heartbeat} seconds" + ) + ) + + async def _pong_not_received_coro(self): + """Coroutine to check for pending PONG when no PONG was received after self._pong_heartbeat seconds""" + reader = self._reader + assert reader is not None + try: + async with async_timeout.timeout(1): + msg = await reader.read() + self._reset_heartbeat() + if msg.type is not WSMsgType.PONG: + ws_logger.warning(f"Received {msg} while waiting for PONG. It seems like you haven't called `receive` within {self._pong_heartbeat} seconds.") + return + except asyncio.TimeoutError: # We still did not receive a PONG + pass + except: # If any exception happens we also did not receive a PONG + pass + self._handle_ping_pong_exception( + asyncio.TimeoutError( + f"No PONG received after {self._pong_heartbeat} seconds" ) + ) def _handle_ping_pong_exception(self, exc: BaseException) -> None: """Handle exceptions raised during ping/pong processing.""" From 77d624d3353dbd5960a0d6548d515b0d0c7bc62f Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 09:58:50 +0100 Subject: [PATCH 02/17] Handle exceptions in pong_not_received_coro() Handle exceptions in pong_not_received_coro() analogous to receive() --- aiohttp/web_ws.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index dc9115e10c0..4a7976d1cb7 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -210,8 +210,11 @@ async def _pong_not_received_coro(self): return except asyncio.TimeoutError: # We still did not receive a PONG pass - except: # If any exception happens we also did not receive a PONG - pass + except Exception as exc: + self._exception = exc + self._set_closing(WSCloseCode.ABNORMAL_CLOSURE) + await self.close() + return self._handle_ping_pong_exception( asyncio.TimeoutError( f"No PONG received after {self._pong_heartbeat} seconds" From a89f86fbe0507c031d0a1fd83d2928e4f47d2c0e Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 10:30:50 +0100 Subject: [PATCH 03/17] Added myself to contributors --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 3004ee5cd18..c759e39eb1b 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -256,6 +256,7 @@ Matvey Tingaev Meet Mangukiya Meshya Michael Ihnatenko +Michael Stegmaier Michał Górny Mikhail Burshteyn Mikhail Kashkin From 365fdf7d36c6d738b5d8aaf9aa64c0b77e087f51 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 09:31:15 +0000 Subject: [PATCH 04/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/web_ws.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 4a7976d1cb7..146ab8b4d4c 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -192,10 +192,10 @@ def _pong_not_received(self) -> None: loop.create_task(self._pong_not_received_coro()) else: self._handle_ping_pong_exception( - asyncio.TimeoutError( - f"No PONG received after {self._pong_heartbeat} seconds" - ) + asyncio.TimeoutError( + f"No PONG received after {self._pong_heartbeat} seconds" ) + ) async def _pong_not_received_coro(self): """Coroutine to check for pending PONG when no PONG was received after self._pong_heartbeat seconds""" @@ -206,9 +206,11 @@ async def _pong_not_received_coro(self): msg = await reader.read() self._reset_heartbeat() if msg.type is not WSMsgType.PONG: - ws_logger.warning(f"Received {msg} while waiting for PONG. It seems like you haven't called `receive` within {self._pong_heartbeat} seconds.") + ws_logger.warning( + f"Received {msg} while waiting for PONG. It seems like you haven't called `receive` within {self._pong_heartbeat} seconds." + ) return - except asyncio.TimeoutError: # We still did not receive a PONG + except asyncio.TimeoutError: # We still did not receive a PONG pass except Exception as exc: self._exception = exc From 605f6002aa27a8845cf8cc9fa290ceec4da122be Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 10:34:13 +0100 Subject: [PATCH 05/17] Added rst file to CHANGES folder --- CHANGES/10544.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/10544.bugfix.rst diff --git a/CHANGES/10544.bugfix.rst b/CHANGES/10544.bugfix.rst new file mode 100644 index 00000000000..0f2429878cb --- /dev/null +++ b/CHANGES/10544.bugfix.rst @@ -0,0 +1 @@ +Check for PONG before closing connection for missing PONG \ No newline at end of file From 48f77afdc49caaf9916b92e4b7b08bfae1c64254 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 09:34:50 +0000 Subject: [PATCH 06/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- CHANGES/10544.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/10544.bugfix.rst b/CHANGES/10544.bugfix.rst index 0f2429878cb..7916da7c14d 100644 --- a/CHANGES/10544.bugfix.rst +++ b/CHANGES/10544.bugfix.rst @@ -1 +1 @@ -Check for PONG before closing connection for missing PONG \ No newline at end of file +Check for PONG before closing connection for missing PONG From 73ecb05965ba2d3ad43ad3a3a65218642e35fbbb Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 10:38:16 +0100 Subject: [PATCH 07/17] Update 10544.bugfix.rst Added missing contributor name --- CHANGES/10544.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/10544.bugfix.rst b/CHANGES/10544.bugfix.rst index 7916da7c14d..2ccf54fc0ab 100644 --- a/CHANGES/10544.bugfix.rst +++ b/CHANGES/10544.bugfix.rst @@ -1 +1 @@ -Check for PONG before closing connection for missing PONG +Check for PONG before closing connection for missing PONG -- by :user:`mstegmaier`. From 7768ff5bcd04ea200cf349b72f380e1cb7181b53 Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 10:43:36 +0100 Subject: [PATCH 08/17] Added missing return type for _pong_not_received_coro() to make mypy happy --- aiohttp/web_ws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 146ab8b4d4c..83f06abe59f 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -197,7 +197,7 @@ def _pong_not_received(self) -> None: ) ) - async def _pong_not_received_coro(self): + async def _pong_not_received_coro(self) -> None: """Coroutine to check for pending PONG when no PONG was received after self._pong_heartbeat seconds""" reader = self._reader assert reader is not None From 8a591d03367a88eaaed847331c8de0aeb4f48db7 Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 10:55:12 +0100 Subject: [PATCH 09/17] Save pong_not_received_task in a variable to make mypy happy --- aiohttp/web_ws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 83f06abe59f..eb61d21a5a9 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -189,7 +189,7 @@ def _pong_not_received(self) -> None: if self._req is not None and self._req.transport is not None: loop = self._loop if loop is not None: - loop.create_task(self._pong_not_received_coro()) + pong_not_received_task = loop.create_task(self._pong_not_received_coro()) else: self._handle_ping_pong_exception( asyncio.TimeoutError( From 5d7c76ed544b16f8fa9860fddd7894acb595e02f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 09:59:02 +0000 Subject: [PATCH 10/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/web_ws.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index eb61d21a5a9..e601a20bb1e 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -189,7 +189,9 @@ def _pong_not_received(self) -> None: if self._req is not None and self._req.transport is not None: loop = self._loop if loop is not None: - pong_not_received_task = loop.create_task(self._pong_not_received_coro()) + pong_not_received_task = loop.create_task( + self._pong_not_received_coro() + ) else: self._handle_ping_pong_exception( asyncio.TimeoutError( From 290fa312534fd87188e858073e4e9f3f7df75d38 Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 13:48:17 +0100 Subject: [PATCH 11/17] Added callback for pong not received task --- aiohttp/web_ws.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index e601a20bb1e..74fb71b1a5b 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -81,6 +81,7 @@ class WebSocketResponse(StreamResponse): _heartbeat_cb: Optional[asyncio.TimerHandle] = None _pong_response_cb: Optional[asyncio.TimerHandle] = None _ping_task: Optional[asyncio.Task[None]] = None + _pong_not_received_task: Optional[asyncio.Task[None]] = None def __init__( self, @@ -192,6 +193,11 @@ def _pong_not_received(self) -> None: pong_not_received_task = loop.create_task( self._pong_not_received_coro() ) + if not pong_not_received_task.done(): + self._pong_not_received_task = pong_not_received_task + pong_not_received_task.add_done_callback(self._pong_not_received_done) + else: + self._pong_not_received_done(pong_not_received_task) else: self._handle_ping_pong_exception( asyncio.TimeoutError( @@ -225,6 +231,12 @@ async def _pong_not_received_coro(self) -> None: ) ) + def _pong_not_received_done(self, task: "asyncio.Task[None]") -> None: + """Callback for wehn the pong not received task completes.""" + if not task.cancelled() and (exc := task.exception()): + self._handle_ping_pong_exception(exc) + self._pong_not_received_task = None + def _handle_ping_pong_exception(self, exc: BaseException) -> None: """Handle exceptions raised during ping/pong processing.""" if self._closed: From 3439d1c63610920eca16883333e4364ae9bbcd5a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 12:50:32 +0000 Subject: [PATCH 12/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/web_ws.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 74fb71b1a5b..65cb7a7eebe 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -195,7 +195,9 @@ def _pong_not_received(self) -> None: ) if not pong_not_received_task.done(): self._pong_not_received_task = pong_not_received_task - pong_not_received_task.add_done_callback(self._pong_not_received_done) + pong_not_received_task.add_done_callback( + self._pong_not_received_done + ) else: self._pong_not_received_done(pong_not_received_task) else: @@ -236,7 +238,7 @@ def _pong_not_received_done(self, task: "asyncio.Task[None]") -> None: if not task.cancelled() and (exc := task.exception()): self._handle_ping_pong_exception(exc) self._pong_not_received_task = None - + def _handle_ping_pong_exception(self, exc: BaseException) -> None: """Handle exceptions raised during ping/pong processing.""" if self._closed: From 85696f31d7d90ea88a05b7242cbda42065f458da Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 13:53:15 +0100 Subject: [PATCH 13/17] Fixed typo wehn -> when --- aiohttp/web_ws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 65cb7a7eebe..53a1f19260e 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -234,7 +234,7 @@ async def _pong_not_received_coro(self) -> None: ) def _pong_not_received_done(self, task: "asyncio.Task[None]") -> None: - """Callback for wehn the pong not received task completes.""" + """Callback for when the pong not received task completes.""" if not task.cancelled() and (exc := task.exception()): self._handle_ping_pong_exception(exc) self._pong_not_received_task = None From 4d9ebf07719bc138f47f596557714ad60cc0a526 Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Thu, 13 Mar 2025 16:35:59 +0100 Subject: [PATCH 14/17] Fixed test --- aiohttp/web_ws.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 53a1f19260e..94881c8cd0d 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -212,7 +212,7 @@ async def _pong_not_received_coro(self) -> None: reader = self._reader assert reader is not None try: - async with async_timeout.timeout(1): + async with async_timeout.timeout(self._pong_heartbeat / 10.0): msg = await reader.read() self._reset_heartbeat() if msg.type is not WSMsgType.PONG: @@ -222,6 +222,8 @@ async def _pong_not_received_coro(self) -> None: return except asyncio.TimeoutError: # We still did not receive a PONG pass + except AssertionError: # In the test, an AssertionError seems to occur before the timeout + pass except Exception as exc: self._exception = exc self._set_closing(WSCloseCode.ABNORMAL_CLOSURE) From 8f3de6f3b87df24fe28649dccf6ca01739e5a059 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 15:36:36 +0000 Subject: [PATCH 15/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/web_ws.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 94881c8cd0d..73a2a60e7a3 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -222,7 +222,9 @@ async def _pong_not_received_coro(self) -> None: return except asyncio.TimeoutError: # We still did not receive a PONG pass - except AssertionError: # In the test, an AssertionError seems to occur before the timeout + except ( + AssertionError + ): # In the test, an AssertionError seems to occur before the timeout pass except Exception as exc: self._exception = exc From 6e9e13f6ba884a42e0b8ca20277d1d7aee883be6 Mon Sep 17 00:00:00 2001 From: Michael Stegmaier Date: Fri, 14 Mar 2025 17:01:29 +0100 Subject: [PATCH 16/17] Avoid concurrent reads from reader If self._waiting is set, we already are in the receive loop reading from the reader and would have read a PONG if one was already there. This avoids concurrent reads on the reader. --- aiohttp/web_ws.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 73a2a60e7a3..88f7582fdef 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -189,7 +189,7 @@ def _pong_not_received(self) -> None: """Callback for when no PONG was received after self._pong_heartbeat seconds""" if self._req is not None and self._req.transport is not None: loop = self._loop - if loop is not None: + if loop is not None and not self._waiting: # If self._waiting is set we already are in the receive loop and would have read the PONG if one was there pong_not_received_task = loop.create_task( self._pong_not_received_coro() ) @@ -222,10 +222,6 @@ async def _pong_not_received_coro(self) -> None: return except asyncio.TimeoutError: # We still did not receive a PONG pass - except ( - AssertionError - ): # In the test, an AssertionError seems to occur before the timeout - pass except Exception as exc: self._exception = exc self._set_closing(WSCloseCode.ABNORMAL_CLOSURE) From 29a439e7675dcdbaf336cbaeb63587aece9aa0fb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 14 Mar 2025 16:03:10 +0000 Subject: [PATCH 17/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- aiohttp/web_ws.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index 88f7582fdef..d3d4401f1e1 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -189,7 +189,9 @@ def _pong_not_received(self) -> None: """Callback for when no PONG was received after self._pong_heartbeat seconds""" if self._req is not None and self._req.transport is not None: loop = self._loop - if loop is not None and not self._waiting: # If self._waiting is set we already are in the receive loop and would have read the PONG if one was there + if ( + loop is not None and not self._waiting + ): # If self._waiting is set we already are in the receive loop and would have read the PONG if one was there pong_not_received_task = loop.create_task( self._pong_not_received_coro() )