From 1bc8d53210512afa044fbea0b676d1575aed7baa Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 8 Aug 2024 21:59:10 +0100 Subject: [PATCH 01/15] [PR #8657/6c6ecfaf backport][3.10] Fix multipart reading with split boundary (#8658) **This is a backport of PR #8657 as merged into master (6c6ecfaf320b27eb9f86066c4bfb1f3947c3362d).** --------- Co-authored-by: Sam Bull --- CHANGES/8653.bugfix.rst | 1 + aiohttp/multipart.py | 19 ++++++++++--- tests/test_multipart.py | 61 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 CHANGES/8653.bugfix.rst diff --git a/CHANGES/8653.bugfix.rst b/CHANGES/8653.bugfix.rst new file mode 100644 index 00000000000..5c4d66c181f --- /dev/null +++ b/CHANGES/8653.bugfix.rst @@ -0,0 +1 @@ +Fixed multipart reading when stream buffer splits the boundary over several read() calls -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/multipart.py b/aiohttp/multipart.py index 71fc2654a1c..26780e3060c 100644 --- a/aiohttp/multipart.py +++ b/aiohttp/multipart.py @@ -266,6 +266,7 @@ def __init__( ) -> None: self.headers = headers self._boundary = boundary + self._boundary_len = len(boundary) + 2 # Boundary + \r\n self._content = content self._default_charset = default_charset self._at_eof = False @@ -346,15 +347,25 @@ async def _read_chunk_from_stream(self, size: int) -> bytes: # Reads content chunk of body part with unknown length. # The Content-Length header for body part is not necessary. assert ( - size >= len(self._boundary) + 2 + size >= self._boundary_len ), "Chunk size must be greater or equal than boundary length + 2" first_chunk = self._prev_chunk is None if first_chunk: self._prev_chunk = await self._content.read(size) - chunk = await self._content.read(size) - self._content_eof += int(self._content.at_eof()) - assert self._content_eof < 3, "Reading after EOF" + chunk = b"" + # content.read() may return less than size, so we need to loop to ensure + # we have enough data to detect the boundary. + while len(chunk) < self._boundary_len: + chunk += await self._content.read(size) + self._content_eof += int(self._content.at_eof()) + assert self._content_eof < 3, "Reading after EOF" + if self._content_eof: + break + if len(chunk) > size: + self._content.unread_data(chunk[size:]) + chunk = chunk[:size] + assert self._prev_chunk is not None window = self._prev_chunk + chunk sub = b"\r\n" + self._boundary diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 436b70957fa..6fc9fe573ec 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -2,6 +2,7 @@ import io import json import pathlib +import sys import zlib from unittest import mock @@ -754,6 +755,66 @@ async def test_invalid_boundary(self) -> None: with pytest.raises(ValueError): await reader.next() + @pytest.mark.skipif(sys.version_info < (3, 10), reason="Needs anext()") + async def test_read_boundary_across_chunks(self) -> None: + class SplitBoundaryStream: + def __init__(self) -> None: + self.content = [ + b"--foobar\r\n\r\n", + b"Hello,\r\n-", + b"-fo", + b"ob", + b"ar\r\n", + b"\r\nwor", + b"ld!", + b"\r\n--f", + b"oobar--", + ] + + async def read(self, size=None) -> bytes: + chunk = self.content.pop(0) + assert len(chunk) <= size + return chunk + + def at_eof(self) -> bool: + return not self.content + + async def readline(self) -> bytes: + line = b"" + while self.content and b"\n" not in line: + line += self.content.pop(0) + line, *extra = line.split(b"\n", maxsplit=1) + if extra and extra[0]: + self.content.insert(0, extra[0]) + return line + b"\n" + + def unread_data(self, data: bytes) -> None: + if self.content: + self.content[0] = data + self.content[0] + else: + self.content.append(data) + + stream = SplitBoundaryStream() + reader = aiohttp.MultipartReader( + {CONTENT_TYPE: 'multipart/related;boundary="foobar"'}, stream + ) + part = await anext(reader) + result = await part.read_chunk(10) + assert result == b"Hello," + result = await part.read_chunk(10) + assert result == b"" + assert part.at_eof() + + part = await anext(reader) + result = await part.read_chunk(10) + assert result == b"world!" + result = await part.read_chunk(10) + assert result == b"" + assert part.at_eof() + + with pytest.raises(StopAsyncIteration): + await anext(reader) + async def test_release(self) -> None: with Stream( newline.join( From dbaf17479c8a7fdaf85dc093963c99f740e118fe Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 8 Aug 2024 23:58:29 +0100 Subject: [PATCH 02/15] [PR #8657/6c6ecfaf backport][3.11] Fix multipart reading with split boundary (#8659) **This is a backport of PR #8657 as merged into master (6c6ecfaf320b27eb9f86066c4bfb1f3947c3362d).** --------- Co-authored-by: Sam Bull --- CHANGES/8653.bugfix.rst | 1 + aiohttp/multipart.py | 19 ++++++++++--- tests/test_multipart.py | 61 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 CHANGES/8653.bugfix.rst diff --git a/CHANGES/8653.bugfix.rst b/CHANGES/8653.bugfix.rst new file mode 100644 index 00000000000..5c4d66c181f --- /dev/null +++ b/CHANGES/8653.bugfix.rst @@ -0,0 +1 @@ +Fixed multipart reading when stream buffer splits the boundary over several read() calls -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/multipart.py b/aiohttp/multipart.py index 71fc2654a1c..26780e3060c 100644 --- a/aiohttp/multipart.py +++ b/aiohttp/multipart.py @@ -266,6 +266,7 @@ def __init__( ) -> None: self.headers = headers self._boundary = boundary + self._boundary_len = len(boundary) + 2 # Boundary + \r\n self._content = content self._default_charset = default_charset self._at_eof = False @@ -346,15 +347,25 @@ async def _read_chunk_from_stream(self, size: int) -> bytes: # Reads content chunk of body part with unknown length. # The Content-Length header for body part is not necessary. assert ( - size >= len(self._boundary) + 2 + size >= self._boundary_len ), "Chunk size must be greater or equal than boundary length + 2" first_chunk = self._prev_chunk is None if first_chunk: self._prev_chunk = await self._content.read(size) - chunk = await self._content.read(size) - self._content_eof += int(self._content.at_eof()) - assert self._content_eof < 3, "Reading after EOF" + chunk = b"" + # content.read() may return less than size, so we need to loop to ensure + # we have enough data to detect the boundary. + while len(chunk) < self._boundary_len: + chunk += await self._content.read(size) + self._content_eof += int(self._content.at_eof()) + assert self._content_eof < 3, "Reading after EOF" + if self._content_eof: + break + if len(chunk) > size: + self._content.unread_data(chunk[size:]) + chunk = chunk[:size] + assert self._prev_chunk is not None window = self._prev_chunk + chunk sub = b"\r\n" + self._boundary diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 436b70957fa..6fc9fe573ec 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -2,6 +2,7 @@ import io import json import pathlib +import sys import zlib from unittest import mock @@ -754,6 +755,66 @@ async def test_invalid_boundary(self) -> None: with pytest.raises(ValueError): await reader.next() + @pytest.mark.skipif(sys.version_info < (3, 10), reason="Needs anext()") + async def test_read_boundary_across_chunks(self) -> None: + class SplitBoundaryStream: + def __init__(self) -> None: + self.content = [ + b"--foobar\r\n\r\n", + b"Hello,\r\n-", + b"-fo", + b"ob", + b"ar\r\n", + b"\r\nwor", + b"ld!", + b"\r\n--f", + b"oobar--", + ] + + async def read(self, size=None) -> bytes: + chunk = self.content.pop(0) + assert len(chunk) <= size + return chunk + + def at_eof(self) -> bool: + return not self.content + + async def readline(self) -> bytes: + line = b"" + while self.content and b"\n" not in line: + line += self.content.pop(0) + line, *extra = line.split(b"\n", maxsplit=1) + if extra and extra[0]: + self.content.insert(0, extra[0]) + return line + b"\n" + + def unread_data(self, data: bytes) -> None: + if self.content: + self.content[0] = data + self.content[0] + else: + self.content.append(data) + + stream = SplitBoundaryStream() + reader = aiohttp.MultipartReader( + {CONTENT_TYPE: 'multipart/related;boundary="foobar"'}, stream + ) + part = await anext(reader) + result = await part.read_chunk(10) + assert result == b"Hello," + result = await part.read_chunk(10) + assert result == b"" + assert part.at_eof() + + part = await anext(reader) + result = await part.read_chunk(10) + assert result == b"world!" + result = await part.read_chunk(10) + assert result == b"" + assert part.at_eof() + + with pytest.raises(StopAsyncIteration): + await anext(reader) + async def test_release(self) -> None: with Stream( newline.join( From 3a9de0c1457e04bbe81acfefd031ff436c1da98d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 8 Aug 2024 19:17:35 -0500 Subject: [PATCH 03/15] [PR #8660/14d5295 backport][3.10] Improve performance of WebSockets when there is no timeout (#8663) --- CHANGES/8660.misc.rst | 3 +++ aiohttp/client_ws.py | 11 ++++++++++- aiohttp/web_ws.py | 10 +++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 CHANGES/8660.misc.rst diff --git a/CHANGES/8660.misc.rst b/CHANGES/8660.misc.rst new file mode 100644 index 00000000000..8710063329e --- /dev/null +++ b/CHANGES/8660.misc.rst @@ -0,0 +1,3 @@ +Improved performance of :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` when there is no timeout. -- by :user:`bdraco`. + +The timeout context manager is now avoided when there is no timeout as it accounted for up to 50% of the time spent in the :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` methods. diff --git a/aiohttp/client_ws.py b/aiohttp/client_ws.py index 247f62c758e..7fd141248bd 100644 --- a/aiohttp/client_ws.py +++ b/aiohttp/client_ws.py @@ -281,6 +281,8 @@ async def close(self, *, code: int = WSCloseCode.OK, message: bytes = b"") -> bo return False async def receive(self, timeout: Optional[float] = None) -> WSMessage: + receive_timeout = timeout or self._receive_timeout + while True: if self._waiting: raise RuntimeError("Concurrent call to receive() is not allowed") @@ -294,7 +296,14 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: try: self._waiting = True try: - async with async_timeout.timeout(timeout or self._receive_timeout): + if receive_timeout: + # Entering the context manager and creating + # Timeout() object can take almost 50% of the + # run time in this loop so we avoid it if + # there is no read timeout. + async with async_timeout.timeout(receive_timeout): + msg = await self._reader.read() + else: msg = await self._reader.read() self._reset_heartbeat() finally: diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index ba3332715a6..fe8f537dc76 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -484,6 +484,7 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: loop = self._loop assert loop is not None + receive_timeout = timeout or self._receive_timeout while True: if self._waiting: raise RuntimeError("Concurrent call to receive() is not allowed") @@ -499,7 +500,14 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: try: self._waiting = True try: - async with async_timeout.timeout(timeout or self._receive_timeout): + if receive_timeout: + # Entering the context manager and creating + # Timeout() object can take almost 50% of the + # run time in this loop so we avoid it if + # there is no read timeout. + async with async_timeout.timeout(receive_timeout): + msg = await self._reader.read() + else: msg = await self._reader.read() self._reset_heartbeat() finally: From 52f2e856c43d5f689f5ca910e933882d13df1442 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 8 Aug 2024 19:17:37 -0500 Subject: [PATCH 04/15] [PR #8660/14d5295 backport][3.11] Improve performance of WebSockets when there is no timeout (#8664) --- CHANGES/8660.misc.rst | 3 +++ aiohttp/client_ws.py | 11 ++++++++++- aiohttp/web_ws.py | 10 +++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 CHANGES/8660.misc.rst diff --git a/CHANGES/8660.misc.rst b/CHANGES/8660.misc.rst new file mode 100644 index 00000000000..8710063329e --- /dev/null +++ b/CHANGES/8660.misc.rst @@ -0,0 +1,3 @@ +Improved performance of :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` when there is no timeout. -- by :user:`bdraco`. + +The timeout context manager is now avoided when there is no timeout as it accounted for up to 50% of the time spent in the :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` methods. diff --git a/aiohttp/client_ws.py b/aiohttp/client_ws.py index 247f62c758e..7fd141248bd 100644 --- a/aiohttp/client_ws.py +++ b/aiohttp/client_ws.py @@ -281,6 +281,8 @@ async def close(self, *, code: int = WSCloseCode.OK, message: bytes = b"") -> bo return False async def receive(self, timeout: Optional[float] = None) -> WSMessage: + receive_timeout = timeout or self._receive_timeout + while True: if self._waiting: raise RuntimeError("Concurrent call to receive() is not allowed") @@ -294,7 +296,14 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: try: self._waiting = True try: - async with async_timeout.timeout(timeout or self._receive_timeout): + if receive_timeout: + # Entering the context manager and creating + # Timeout() object can take almost 50% of the + # run time in this loop so we avoid it if + # there is no read timeout. + async with async_timeout.timeout(receive_timeout): + msg = await self._reader.read() + else: msg = await self._reader.read() self._reset_heartbeat() finally: diff --git a/aiohttp/web_ws.py b/aiohttp/web_ws.py index ba3332715a6..fe8f537dc76 100644 --- a/aiohttp/web_ws.py +++ b/aiohttp/web_ws.py @@ -484,6 +484,7 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: loop = self._loop assert loop is not None + receive_timeout = timeout or self._receive_timeout while True: if self._waiting: raise RuntimeError("Concurrent call to receive() is not allowed") @@ -499,7 +500,14 @@ async def receive(self, timeout: Optional[float] = None) -> WSMessage: try: self._waiting = True try: - async with async_timeout.timeout(timeout or self._receive_timeout): + if receive_timeout: + # Entering the context manager and creating + # Timeout() object can take almost 50% of the + # run time in this loop so we avoid it if + # there is no read timeout. + async with async_timeout.timeout(receive_timeout): + msg = await self._reader.read() + else: msg = await self._reader.read() self._reset_heartbeat() finally: From b4ad882576666ce2bba6eafe634001d46b850cb2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 8 Aug 2024 19:31:27 -0500 Subject: [PATCH 05/15] [PR #8661/4d604ea backport][3.10] Improve performance of starting request handlers with Python 3.12+ (#8665) --- CHANGES/8661.misc.rst | 1 + aiohttp/web_protocol.py | 9 ++++++--- 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 CHANGES/8661.misc.rst diff --git a/CHANGES/8661.misc.rst b/CHANGES/8661.misc.rst new file mode 100644 index 00000000000..c0a6fdadb37 --- /dev/null +++ b/CHANGES/8661.misc.rst @@ -0,0 +1 @@ +Improved performance of starting request handlers with Python 3.12+ -- by :user:`bdraco`. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 9ba05a08e75..f60759d927b 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -1,5 +1,6 @@ import asyncio import asyncio.streams +import sys import traceback import warnings from collections import deque @@ -533,9 +534,11 @@ async def start(self) -> None: request = self._request_factory(message, payload, self, writer, handler) try: # a new task is used for copy context vars (#3406) - task = self._loop.create_task( - self._handle_request(request, start, request_handler) - ) + coro = self._handle_request(request, start, request_handler) + if sys.version_info >= (3, 12): + task = asyncio.Task(coro, loop=loop, eager_start=True) + else: + task = loop.create_task(coro) try: resp, reset = await task except (asyncio.CancelledError, ConnectionError): From 58e91a17d6e797df297709e7612eff3876ad060b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 8 Aug 2024 19:32:41 -0500 Subject: [PATCH 06/15] [PR #8661/4d604ea backport][3.11] Improve performance of starting request handlers with Python 3.12+ (#8666) --- CHANGES/8661.misc.rst | 1 + aiohttp/web_protocol.py | 9 ++++++--- 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 CHANGES/8661.misc.rst diff --git a/CHANGES/8661.misc.rst b/CHANGES/8661.misc.rst new file mode 100644 index 00000000000..c0a6fdadb37 --- /dev/null +++ b/CHANGES/8661.misc.rst @@ -0,0 +1 @@ +Improved performance of starting request handlers with Python 3.12+ -- by :user:`bdraco`. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 9ba05a08e75..f60759d927b 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -1,5 +1,6 @@ import asyncio import asyncio.streams +import sys import traceback import warnings from collections import deque @@ -533,9 +534,11 @@ async def start(self) -> None: request = self._request_factory(message, payload, self, writer, handler) try: # a new task is used for copy context vars (#3406) - task = self._loop.create_task( - self._handle_request(request, start, request_handler) - ) + coro = self._handle_request(request, start, request_handler) + if sys.version_info >= (3, 12): + task = asyncio.Task(coro, loop=loop, eager_start=True) + else: + task = loop.create_task(coro) try: resp, reset = await task except (asyncio.CancelledError, ConnectionError): From dbcdb16d6ef82dc310dc03841008c54bd4b61d59 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 9 Aug 2024 09:11:19 -0500 Subject: [PATCH 07/15] [PR #8667/406cd2c7 backport][3.10] Improve performance of generating random WebSocket mask (#8668) Co-authored-by: J. Nick Koston --- CHANGES/8667.misc.rst | 1 + aiohttp/http_websocket.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 CHANGES/8667.misc.rst diff --git a/CHANGES/8667.misc.rst b/CHANGES/8667.misc.rst new file mode 100644 index 00000000000..1c43b6e069a --- /dev/null +++ b/CHANGES/8667.misc.rst @@ -0,0 +1 @@ +Improved performance of generating random WebSocket mask -- by :user:`bdraco`. diff --git a/aiohttp/http_websocket.py b/aiohttp/http_websocket.py index 39f2e4a5c15..b513a45ebdc 100644 --- a/aiohttp/http_websocket.py +++ b/aiohttp/http_websocket.py @@ -8,6 +8,7 @@ import sys import zlib from enum import IntEnum +from functools import partial from struct import Struct from typing import ( Any, @@ -103,6 +104,7 @@ class WSMsgType(IntEnum): PACK_LEN2 = Struct("!BBH").pack PACK_LEN3 = Struct("!BBQ").pack PACK_CLOSE_CODE = Struct("!H").pack +PACK_RANDBITS = Struct("!L").pack MSG_SIZE: Final[int] = 2**14 DEFAULT_LIMIT: Final[int] = 2**16 @@ -612,7 +614,7 @@ def __init__( self.protocol = protocol self.transport = transport self.use_mask = use_mask - self.randrange = random.randrange + self.get_random_bits = partial(random.getrandbits, 32) self.compress = compress self.notakeover = notakeover self._closing = False @@ -668,8 +670,7 @@ async def _send_frame( else: header = PACK_LEN3(0x80 | rsv | opcode, 127 | mask_bit, msg_length) if use_mask: - mask_int = self.randrange(0, 0xFFFFFFFF) - mask = mask_int.to_bytes(4, "big") + mask = PACK_RANDBITS(self.get_random_bits()) message = bytearray(message) _websocket_mask(mask, message) self._write(header + mask + message) From 9fd043ea4c06af921196b44c4b3777c27f42e784 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 9 Aug 2024 09:11:29 -0500 Subject: [PATCH 08/15] [PR #8667/406cd2c7 backport][3.11] Improve performance of generating random WebSocket mask (#8669) Co-authored-by: J. Nick Koston --- CHANGES/8667.misc.rst | 1 + aiohttp/http_websocket.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 CHANGES/8667.misc.rst diff --git a/CHANGES/8667.misc.rst b/CHANGES/8667.misc.rst new file mode 100644 index 00000000000..1c43b6e069a --- /dev/null +++ b/CHANGES/8667.misc.rst @@ -0,0 +1 @@ +Improved performance of generating random WebSocket mask -- by :user:`bdraco`. diff --git a/aiohttp/http_websocket.py b/aiohttp/http_websocket.py index 39f2e4a5c15..b513a45ebdc 100644 --- a/aiohttp/http_websocket.py +++ b/aiohttp/http_websocket.py @@ -8,6 +8,7 @@ import sys import zlib from enum import IntEnum +from functools import partial from struct import Struct from typing import ( Any, @@ -103,6 +104,7 @@ class WSMsgType(IntEnum): PACK_LEN2 = Struct("!BBH").pack PACK_LEN3 = Struct("!BBQ").pack PACK_CLOSE_CODE = Struct("!H").pack +PACK_RANDBITS = Struct("!L").pack MSG_SIZE: Final[int] = 2**14 DEFAULT_LIMIT: Final[int] = 2**16 @@ -612,7 +614,7 @@ def __init__( self.protocol = protocol self.transport = transport self.use_mask = use_mask - self.randrange = random.randrange + self.get_random_bits = partial(random.getrandbits, 32) self.compress = compress self.notakeover = notakeover self._closing = False @@ -668,8 +670,7 @@ async def _send_frame( else: header = PACK_LEN3(0x80 | rsv | opcode, 127 | mask_bit, msg_length) if use_mask: - mask_int = self.randrange(0, 0xFFFFFFFF) - mask = mask_int.to_bytes(4, "big") + mask = PACK_RANDBITS(self.get_random_bits()) message = bytearray(message) _websocket_mask(mask, message) self._write(header + mask + message) From f96182adab30d8609342c6d273c5aad1cd92b29f Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 9 Aug 2024 17:17:22 +0000 Subject: [PATCH 09/15] [PR #8662/be23d16f backport][3.10] Improve performance of keepalive rescheduling (#8670) Co-authored-by: J. Nick Koston --- CHANGES/8662.misc.rst | 3 +++ aiohttp/web_protocol.py | 33 +++++++++++++++------------------ 2 files changed, 18 insertions(+), 18 deletions(-) create mode 100644 CHANGES/8662.misc.rst diff --git a/CHANGES/8662.misc.rst b/CHANGES/8662.misc.rst new file mode 100644 index 00000000000..efe30a60cb2 --- /dev/null +++ b/CHANGES/8662.misc.rst @@ -0,0 +1,3 @@ +Improved performance of HTTP keep-alive checks -- by :user:`bdraco`. + +Previously, when processing a request for a keep-alive connection, the keep-alive check would happen every second; the check is now rescheduled if it fires too early instead. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index f60759d927b..635b668ceb0 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -134,8 +134,6 @@ class RequestHandler(BaseProtocol): """ - KEEPALIVE_RESCHEDULE_DELAY = 1 - __slots__ = ( "_request_count", "_keepalive", @@ -143,7 +141,7 @@ class RequestHandler(BaseProtocol): "_request_handler", "_request_factory", "_tcp_keepalive", - "_keepalive_time", + "_next_keepalive_close_time", "_keepalive_handle", "_keepalive_timeout", "_lingering_time", @@ -197,7 +195,7 @@ def __init__( self._tcp_keepalive = tcp_keepalive # placeholder to be replaced on keepalive timeout setup - self._keepalive_time = 0.0 + self._next_keepalive_close_time = 0.0 self._keepalive_handle: Optional[asyncio.Handle] = None self._keepalive_timeout = keepalive_timeout self._lingering_time = float(lingering_time) @@ -429,23 +427,21 @@ def log_exception(self, *args: Any, **kw: Any) -> None: self.logger.exception(*args, **kw) def _process_keepalive(self) -> None: + self._keepalive_handle = None if self._force_close or not self._keepalive: return - next = self._keepalive_time + self._keepalive_timeout + loop = self._loop + now = loop.time() + close_time = self._next_keepalive_close_time + if now <= close_time: + # Keep alive close check fired too early, reschedule + self._keepalive_handle = loop.call_at(close_time, self._process_keepalive) + return # handler in idle state if self._waiter: - if self._loop.time() > next: - self.force_close() - return - - # not all request handlers are done, - # reschedule itself to next second - self._keepalive_handle = self._loop.call_later( - self.KEEPALIVE_RESCHEDULE_DELAY, - self._process_keepalive, - ) + self.force_close() async def _handle_request( self, @@ -596,11 +592,12 @@ async def start(self) -> None: if self._keepalive and not self._close: # start keep-alive timer if keepalive_timeout is not None: - now = self._loop.time() - self._keepalive_time = now + now = loop.time() + close_time = now + keepalive_timeout + self._next_keepalive_close_time = close_time if self._keepalive_handle is None: self._keepalive_handle = loop.call_at( - now + keepalive_timeout, self._process_keepalive + close_time, self._process_keepalive ) else: break From 3f452a0963b0419033e7d554fc583b8e7754995e Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 9 Aug 2024 19:26:59 +0200 Subject: [PATCH 10/15] [PR #8662/be23d16f backport][3.11] Improve performance of keepalive rescheduling (#8671) Co-authored-by: J. Nick Koston --- CHANGES/8662.misc.rst | 3 +++ aiohttp/web_protocol.py | 33 +++++++++++++++------------------ 2 files changed, 18 insertions(+), 18 deletions(-) create mode 100644 CHANGES/8662.misc.rst diff --git a/CHANGES/8662.misc.rst b/CHANGES/8662.misc.rst new file mode 100644 index 00000000000..efe30a60cb2 --- /dev/null +++ b/CHANGES/8662.misc.rst @@ -0,0 +1,3 @@ +Improved performance of HTTP keep-alive checks -- by :user:`bdraco`. + +Previously, when processing a request for a keep-alive connection, the keep-alive check would happen every second; the check is now rescheduled if it fires too early instead. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index f60759d927b..635b668ceb0 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -134,8 +134,6 @@ class RequestHandler(BaseProtocol): """ - KEEPALIVE_RESCHEDULE_DELAY = 1 - __slots__ = ( "_request_count", "_keepalive", @@ -143,7 +141,7 @@ class RequestHandler(BaseProtocol): "_request_handler", "_request_factory", "_tcp_keepalive", - "_keepalive_time", + "_next_keepalive_close_time", "_keepalive_handle", "_keepalive_timeout", "_lingering_time", @@ -197,7 +195,7 @@ def __init__( self._tcp_keepalive = tcp_keepalive # placeholder to be replaced on keepalive timeout setup - self._keepalive_time = 0.0 + self._next_keepalive_close_time = 0.0 self._keepalive_handle: Optional[asyncio.Handle] = None self._keepalive_timeout = keepalive_timeout self._lingering_time = float(lingering_time) @@ -429,23 +427,21 @@ def log_exception(self, *args: Any, **kw: Any) -> None: self.logger.exception(*args, **kw) def _process_keepalive(self) -> None: + self._keepalive_handle = None if self._force_close or not self._keepalive: return - next = self._keepalive_time + self._keepalive_timeout + loop = self._loop + now = loop.time() + close_time = self._next_keepalive_close_time + if now <= close_time: + # Keep alive close check fired too early, reschedule + self._keepalive_handle = loop.call_at(close_time, self._process_keepalive) + return # handler in idle state if self._waiter: - if self._loop.time() > next: - self.force_close() - return - - # not all request handlers are done, - # reschedule itself to next second - self._keepalive_handle = self._loop.call_later( - self.KEEPALIVE_RESCHEDULE_DELAY, - self._process_keepalive, - ) + self.force_close() async def _handle_request( self, @@ -596,11 +592,12 @@ async def start(self) -> None: if self._keepalive and not self._close: # start keep-alive timer if keepalive_timeout is not None: - now = self._loop.time() - self._keepalive_time = now + now = loop.time() + close_time = now + keepalive_timeout + self._next_keepalive_close_time = close_time if self._keepalive_handle is None: self._keepalive_handle = loop.call_at( - now + keepalive_timeout, self._process_keepalive + close_time, self._process_keepalive ) else: break From f3fcba467676ba1e86aacdc6b1f2ed4a7e72455c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 10 Aug 2024 10:04:32 -0500 Subject: [PATCH 11/15] [PR #8672/c3219bf backport][3.10] Fix TCPConnector doing blocking I/O in the event loop to create the SSLContext (#8673) Co-authored-by: Sam Bull Co-authored-by: pre-commit-ci[bot] --- CHANGES/8672.bugfix.rst | 3 ++ aiohttp/connector.py | 104 ++++++++++++++++++++++++---------------- tests/test_connector.py | 78 +++++++++++++++++++++++------- tests/test_proxy.py | 2 +- 4 files changed, 128 insertions(+), 59 deletions(-) create mode 100644 CHANGES/8672.bugfix.rst diff --git a/CHANGES/8672.bugfix.rst b/CHANGES/8672.bugfix.rst new file mode 100644 index 00000000000..a57ed16d5d2 --- /dev/null +++ b/CHANGES/8672.bugfix.rst @@ -0,0 +1,3 @@ +Fixed :py:class:`aiohttp.TCPConnector` doing blocking I/O in the event loop to create the ``SSLContext`` -- by :user:`bdraco`. + +The blocking I/O would only happen once per verify mode. However, it could cause the event loop to block for a long time if the ``SSLContext`` creation is slow, which is more likely during startup when the disk cache is not yet present. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index d4691b10e6e..04115c36a24 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -50,7 +50,14 @@ ) from .client_proto import ResponseHandler from .client_reqrep import ClientRequest, Fingerprint, _merge_ssl_params -from .helpers import ceil_timeout, is_ip_address, noop, sentinel +from .helpers import ( + ceil_timeout, + is_ip_address, + noop, + sentinel, + set_exception, + set_result, +) from .locks import EventResultOrError from .resolver import DefaultResolver @@ -771,6 +778,7 @@ class TCPConnector(BaseConnector): """ allowed_protocol_schema_set = HIGH_LEVEL_SCHEMA_SET | frozenset({"tcp"}) + _made_ssl_context: Dict[bool, "asyncio.Future[SSLContext]"] = {} def __init__( self, @@ -969,29 +977,24 @@ async def _create_connection( return proto @staticmethod - @functools.lru_cache(None) def _make_ssl_context(verified: bool) -> SSLContext: + """Create SSL context. + + This method is not async-friendly and should be called from a thread + because it will load certificates from disk and do other blocking I/O. + """ if verified: return ssl.create_default_context() - else: - sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - sslcontext.options |= ssl.OP_NO_SSLv2 - sslcontext.options |= ssl.OP_NO_SSLv3 - sslcontext.check_hostname = False - sslcontext.verify_mode = ssl.CERT_NONE - try: - sslcontext.options |= ssl.OP_NO_COMPRESSION - except AttributeError as attr_err: - warnings.warn( - "{!s}: The Python interpreter is compiled " - "against OpenSSL < 1.0.0. Ref: " - "https://docs.python.org/3/library/ssl.html" - "#ssl.OP_NO_COMPRESSION".format(attr_err), - ) - sslcontext.set_default_verify_paths() - return sslcontext - - def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: + sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + sslcontext.options |= ssl.OP_NO_SSLv2 + sslcontext.options |= ssl.OP_NO_SSLv3 + sslcontext.check_hostname = False + sslcontext.verify_mode = ssl.CERT_NONE + sslcontext.options |= ssl.OP_NO_COMPRESSION + sslcontext.set_default_verify_paths() + return sslcontext + + async def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: """Logic to get the correct SSL context 0. if req.ssl is false, return None @@ -1005,25 +1008,46 @@ def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: 3. if verify_ssl is False in req, generate a SSL context that won't verify """ - if req.is_ssl(): - if ssl is None: # pragma: no cover - raise RuntimeError("SSL is not supported.") - sslcontext = req.ssl - if isinstance(sslcontext, ssl.SSLContext): - return sslcontext - if sslcontext is not True: - # not verified or fingerprinted - return self._make_ssl_context(False) - sslcontext = self._ssl - if isinstance(sslcontext, ssl.SSLContext): - return sslcontext - if sslcontext is not True: - # not verified or fingerprinted - return self._make_ssl_context(False) - return self._make_ssl_context(True) - else: + if not req.is_ssl(): return None + if ssl is None: # pragma: no cover + raise RuntimeError("SSL is not supported.") + sslcontext = req.ssl + if isinstance(sslcontext, ssl.SSLContext): + return sslcontext + if sslcontext is not True: + # not verified or fingerprinted + return await self._make_or_get_ssl_context(False) + sslcontext = self._ssl + if isinstance(sslcontext, ssl.SSLContext): + return sslcontext + if sslcontext is not True: + # not verified or fingerprinted + return await self._make_or_get_ssl_context(False) + return await self._make_or_get_ssl_context(True) + + async def _make_or_get_ssl_context(self, verified: bool) -> SSLContext: + """Create or get cached SSL context.""" + try: + return await self._made_ssl_context[verified] + except KeyError: + loop = self._loop + future = loop.create_future() + self._made_ssl_context[verified] = future + try: + result = await loop.run_in_executor( + None, self._make_ssl_context, verified + ) + # BaseException is used since we might get CancelledError + except BaseException as ex: + del self._made_ssl_context[verified] + set_exception(future, ex) + raise + else: + set_result(future, result) + return result + def _get_fingerprint(self, req: ClientRequest) -> Optional["Fingerprint"]: ret = req.ssl if isinstance(ret, Fingerprint): @@ -1180,7 +1204,7 @@ async def _start_tls_connection( # `req.is_ssl()` evaluates to `False` which is never gonna happen # in this code path. Of course, it's rather fragile # maintainability-wise but this is to be solved separately. - sslcontext = cast(ssl.SSLContext, self._get_ssl_context(req)) + sslcontext = cast(ssl.SSLContext, await self._get_ssl_context(req)) try: async with ceil_timeout( @@ -1258,7 +1282,7 @@ async def _create_direct_connection( *, client_error: Type[Exception] = ClientConnectorError, ) -> Tuple[asyncio.Transport, ResponseHandler]: - sslcontext = self._get_ssl_context(req) + sslcontext = await self._get_ssl_context(req) fingerprint = self._get_fingerprint(req) host = req.url.raw_host diff --git a/tests/test_connector.py b/tests/test_connector.py index d146fb4ee51..0d6ca18ef53 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -1540,23 +1540,23 @@ async def test_tcp_connector_clear_dns_cache_bad_args(loop) -> None: conn.clear_dns_cache("localhost") -async def test_dont_recreate_ssl_context(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) - ctx = conn._make_ssl_context(True) - assert ctx is conn._make_ssl_context(True) +async def test_dont_recreate_ssl_context() -> None: + conn = aiohttp.TCPConnector() + ctx = await conn._make_or_get_ssl_context(True) + assert ctx is await conn._make_or_get_ssl_context(True) -async def test_dont_recreate_ssl_context2(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) - ctx = conn._make_ssl_context(False) - assert ctx is conn._make_ssl_context(False) +async def test_dont_recreate_ssl_context2() -> None: + conn = aiohttp.TCPConnector() + ctx = await conn._make_or_get_ssl_context(False) + assert ctx is await conn._make_or_get_ssl_context(False) -async def test___get_ssl_context1(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) +async def test___get_ssl_context1() -> None: + conn = aiohttp.TCPConnector() req = mock.Mock() req.is_ssl.return_value = False - assert conn._get_ssl_context(req) is None + assert await conn._get_ssl_context(req) is None async def test___get_ssl_context2(loop) -> None: @@ -1565,7 +1565,7 @@ async def test___get_ssl_context2(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = ctx - assert conn._get_ssl_context(req) is ctx + assert await conn._get_ssl_context(req) is ctx async def test___get_ssl_context3(loop) -> None: @@ -1574,7 +1574,7 @@ async def test___get_ssl_context3(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = True - assert conn._get_ssl_context(req) is ctx + assert await conn._get_ssl_context(req) is ctx async def test___get_ssl_context4(loop) -> None: @@ -1583,7 +1583,9 @@ async def test___get_ssl_context4(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = False - assert conn._get_ssl_context(req) is conn._make_ssl_context(False) + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context( + False + ) async def test___get_ssl_context5(loop) -> None: @@ -1592,15 +1594,55 @@ async def test___get_ssl_context5(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = aiohttp.Fingerprint(hashlib.sha256(b"1").digest()) - assert conn._get_ssl_context(req) is conn._make_ssl_context(False) + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context( + False + ) -async def test___get_ssl_context6(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) +async def test___get_ssl_context6() -> None: + conn = aiohttp.TCPConnector() + req = mock.Mock() + req.is_ssl.return_value = True + req.ssl = True + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context(True) + + +async def test_ssl_context_once() -> None: + """Test the ssl context is created only once and shared between connectors.""" + conn1 = aiohttp.TCPConnector() + conn2 = aiohttp.TCPConnector() + conn3 = aiohttp.TCPConnector() + req = mock.Mock() req.is_ssl.return_value = True req.ssl = True - assert conn._get_ssl_context(req) is conn._make_ssl_context(True) + assert await conn1._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert await conn2._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert await conn3._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert conn1._made_ssl_context is conn2._made_ssl_context is conn3._made_ssl_context + assert True in conn1._made_ssl_context + + +@pytest.mark.parametrize("exception", [OSError, ssl.SSLError, asyncio.CancelledError]) +async def test_ssl_context_creation_raises(exception: BaseException) -> None: + """Test that we try again if SSLContext creation fails the first time.""" + conn = aiohttp.TCPConnector() + conn._made_ssl_context.clear() + + with mock.patch.object( + conn, "_make_ssl_context", side_effect=exception + ), pytest.raises( # type: ignore[call-overload] + exception + ): + await conn._make_or_get_ssl_context(True) + + assert isinstance(await conn._make_or_get_ssl_context(True), ssl.SSLContext) async def test_close_twice(loop) -> None: diff --git a/tests/test_proxy.py b/tests/test_proxy.py index f335e42c254..c5e98deb8a5 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -817,7 +817,7 @@ async def make_conn(): self.loop.start_tls.assert_called_with( mock.ANY, mock.ANY, - connector._make_ssl_context(True), + self.loop.run_until_complete(connector._make_or_get_ssl_context(True)), server_hostname="www.python.org", ssl_handshake_timeout=mock.ANY, ) From adf4dea030ce12a356528b5854e668c4556743e3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 10 Aug 2024 10:10:25 -0500 Subject: [PATCH 12/15] [PR #8672/c3219bf backport][3.11] Fix TCPConnector doing blocking I/O in the event loop to create the SSLContext (#8674) Co-authored-by: Sam Bull Co-authored-by: pre-commit-ci[bot] --- CHANGES/8672.bugfix.rst | 3 ++ aiohttp/connector.py | 104 ++++++++++++++++++++++++---------------- tests/test_connector.py | 78 +++++++++++++++++++++++------- tests/test_proxy.py | 2 +- 4 files changed, 128 insertions(+), 59 deletions(-) create mode 100644 CHANGES/8672.bugfix.rst diff --git a/CHANGES/8672.bugfix.rst b/CHANGES/8672.bugfix.rst new file mode 100644 index 00000000000..a57ed16d5d2 --- /dev/null +++ b/CHANGES/8672.bugfix.rst @@ -0,0 +1,3 @@ +Fixed :py:class:`aiohttp.TCPConnector` doing blocking I/O in the event loop to create the ``SSLContext`` -- by :user:`bdraco`. + +The blocking I/O would only happen once per verify mode. However, it could cause the event loop to block for a long time if the ``SSLContext`` creation is slow, which is more likely during startup when the disk cache is not yet present. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index d4691b10e6e..04115c36a24 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -50,7 +50,14 @@ ) from .client_proto import ResponseHandler from .client_reqrep import ClientRequest, Fingerprint, _merge_ssl_params -from .helpers import ceil_timeout, is_ip_address, noop, sentinel +from .helpers import ( + ceil_timeout, + is_ip_address, + noop, + sentinel, + set_exception, + set_result, +) from .locks import EventResultOrError from .resolver import DefaultResolver @@ -771,6 +778,7 @@ class TCPConnector(BaseConnector): """ allowed_protocol_schema_set = HIGH_LEVEL_SCHEMA_SET | frozenset({"tcp"}) + _made_ssl_context: Dict[bool, "asyncio.Future[SSLContext]"] = {} def __init__( self, @@ -969,29 +977,24 @@ async def _create_connection( return proto @staticmethod - @functools.lru_cache(None) def _make_ssl_context(verified: bool) -> SSLContext: + """Create SSL context. + + This method is not async-friendly and should be called from a thread + because it will load certificates from disk and do other blocking I/O. + """ if verified: return ssl.create_default_context() - else: - sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - sslcontext.options |= ssl.OP_NO_SSLv2 - sslcontext.options |= ssl.OP_NO_SSLv3 - sslcontext.check_hostname = False - sslcontext.verify_mode = ssl.CERT_NONE - try: - sslcontext.options |= ssl.OP_NO_COMPRESSION - except AttributeError as attr_err: - warnings.warn( - "{!s}: The Python interpreter is compiled " - "against OpenSSL < 1.0.0. Ref: " - "https://docs.python.org/3/library/ssl.html" - "#ssl.OP_NO_COMPRESSION".format(attr_err), - ) - sslcontext.set_default_verify_paths() - return sslcontext - - def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: + sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + sslcontext.options |= ssl.OP_NO_SSLv2 + sslcontext.options |= ssl.OP_NO_SSLv3 + sslcontext.check_hostname = False + sslcontext.verify_mode = ssl.CERT_NONE + sslcontext.options |= ssl.OP_NO_COMPRESSION + sslcontext.set_default_verify_paths() + return sslcontext + + async def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: """Logic to get the correct SSL context 0. if req.ssl is false, return None @@ -1005,25 +1008,46 @@ def _get_ssl_context(self, req: ClientRequest) -> Optional[SSLContext]: 3. if verify_ssl is False in req, generate a SSL context that won't verify """ - if req.is_ssl(): - if ssl is None: # pragma: no cover - raise RuntimeError("SSL is not supported.") - sslcontext = req.ssl - if isinstance(sslcontext, ssl.SSLContext): - return sslcontext - if sslcontext is not True: - # not verified or fingerprinted - return self._make_ssl_context(False) - sslcontext = self._ssl - if isinstance(sslcontext, ssl.SSLContext): - return sslcontext - if sslcontext is not True: - # not verified or fingerprinted - return self._make_ssl_context(False) - return self._make_ssl_context(True) - else: + if not req.is_ssl(): return None + if ssl is None: # pragma: no cover + raise RuntimeError("SSL is not supported.") + sslcontext = req.ssl + if isinstance(sslcontext, ssl.SSLContext): + return sslcontext + if sslcontext is not True: + # not verified or fingerprinted + return await self._make_or_get_ssl_context(False) + sslcontext = self._ssl + if isinstance(sslcontext, ssl.SSLContext): + return sslcontext + if sslcontext is not True: + # not verified or fingerprinted + return await self._make_or_get_ssl_context(False) + return await self._make_or_get_ssl_context(True) + + async def _make_or_get_ssl_context(self, verified: bool) -> SSLContext: + """Create or get cached SSL context.""" + try: + return await self._made_ssl_context[verified] + except KeyError: + loop = self._loop + future = loop.create_future() + self._made_ssl_context[verified] = future + try: + result = await loop.run_in_executor( + None, self._make_ssl_context, verified + ) + # BaseException is used since we might get CancelledError + except BaseException as ex: + del self._made_ssl_context[verified] + set_exception(future, ex) + raise + else: + set_result(future, result) + return result + def _get_fingerprint(self, req: ClientRequest) -> Optional["Fingerprint"]: ret = req.ssl if isinstance(ret, Fingerprint): @@ -1180,7 +1204,7 @@ async def _start_tls_connection( # `req.is_ssl()` evaluates to `False` which is never gonna happen # in this code path. Of course, it's rather fragile # maintainability-wise but this is to be solved separately. - sslcontext = cast(ssl.SSLContext, self._get_ssl_context(req)) + sslcontext = cast(ssl.SSLContext, await self._get_ssl_context(req)) try: async with ceil_timeout( @@ -1258,7 +1282,7 @@ async def _create_direct_connection( *, client_error: Type[Exception] = ClientConnectorError, ) -> Tuple[asyncio.Transport, ResponseHandler]: - sslcontext = self._get_ssl_context(req) + sslcontext = await self._get_ssl_context(req) fingerprint = self._get_fingerprint(req) host = req.url.raw_host diff --git a/tests/test_connector.py b/tests/test_connector.py index d146fb4ee51..0d6ca18ef53 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -1540,23 +1540,23 @@ async def test_tcp_connector_clear_dns_cache_bad_args(loop) -> None: conn.clear_dns_cache("localhost") -async def test_dont_recreate_ssl_context(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) - ctx = conn._make_ssl_context(True) - assert ctx is conn._make_ssl_context(True) +async def test_dont_recreate_ssl_context() -> None: + conn = aiohttp.TCPConnector() + ctx = await conn._make_or_get_ssl_context(True) + assert ctx is await conn._make_or_get_ssl_context(True) -async def test_dont_recreate_ssl_context2(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) - ctx = conn._make_ssl_context(False) - assert ctx is conn._make_ssl_context(False) +async def test_dont_recreate_ssl_context2() -> None: + conn = aiohttp.TCPConnector() + ctx = await conn._make_or_get_ssl_context(False) + assert ctx is await conn._make_or_get_ssl_context(False) -async def test___get_ssl_context1(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) +async def test___get_ssl_context1() -> None: + conn = aiohttp.TCPConnector() req = mock.Mock() req.is_ssl.return_value = False - assert conn._get_ssl_context(req) is None + assert await conn._get_ssl_context(req) is None async def test___get_ssl_context2(loop) -> None: @@ -1565,7 +1565,7 @@ async def test___get_ssl_context2(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = ctx - assert conn._get_ssl_context(req) is ctx + assert await conn._get_ssl_context(req) is ctx async def test___get_ssl_context3(loop) -> None: @@ -1574,7 +1574,7 @@ async def test___get_ssl_context3(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = True - assert conn._get_ssl_context(req) is ctx + assert await conn._get_ssl_context(req) is ctx async def test___get_ssl_context4(loop) -> None: @@ -1583,7 +1583,9 @@ async def test___get_ssl_context4(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = False - assert conn._get_ssl_context(req) is conn._make_ssl_context(False) + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context( + False + ) async def test___get_ssl_context5(loop) -> None: @@ -1592,15 +1594,55 @@ async def test___get_ssl_context5(loop) -> None: req = mock.Mock() req.is_ssl.return_value = True req.ssl = aiohttp.Fingerprint(hashlib.sha256(b"1").digest()) - assert conn._get_ssl_context(req) is conn._make_ssl_context(False) + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context( + False + ) -async def test___get_ssl_context6(loop) -> None: - conn = aiohttp.TCPConnector(loop=loop) +async def test___get_ssl_context6() -> None: + conn = aiohttp.TCPConnector() + req = mock.Mock() + req.is_ssl.return_value = True + req.ssl = True + assert await conn._get_ssl_context(req) is await conn._make_or_get_ssl_context(True) + + +async def test_ssl_context_once() -> None: + """Test the ssl context is created only once and shared between connectors.""" + conn1 = aiohttp.TCPConnector() + conn2 = aiohttp.TCPConnector() + conn3 = aiohttp.TCPConnector() + req = mock.Mock() req.is_ssl.return_value = True req.ssl = True - assert conn._get_ssl_context(req) is conn._make_ssl_context(True) + assert await conn1._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert await conn2._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert await conn3._get_ssl_context(req) is await conn1._make_or_get_ssl_context( + True + ) + assert conn1._made_ssl_context is conn2._made_ssl_context is conn3._made_ssl_context + assert True in conn1._made_ssl_context + + +@pytest.mark.parametrize("exception", [OSError, ssl.SSLError, asyncio.CancelledError]) +async def test_ssl_context_creation_raises(exception: BaseException) -> None: + """Test that we try again if SSLContext creation fails the first time.""" + conn = aiohttp.TCPConnector() + conn._made_ssl_context.clear() + + with mock.patch.object( + conn, "_make_ssl_context", side_effect=exception + ), pytest.raises( # type: ignore[call-overload] + exception + ): + await conn._make_or_get_ssl_context(True) + + assert isinstance(await conn._make_or_get_ssl_context(True), ssl.SSLContext) async def test_close_twice(loop) -> None: diff --git a/tests/test_proxy.py b/tests/test_proxy.py index f335e42c254..c5e98deb8a5 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -817,7 +817,7 @@ async def make_conn(): self.loop.start_tls.assert_called_with( mock.ANY, mock.ANY, - connector._make_ssl_context(True), + self.loop.run_until_complete(connector._make_or_get_ssl_context(True)), server_hostname="www.python.org", ssl_handshake_timeout=mock.ANY, ) From 73d17d40b38ed71dd2066315313ffa53025912dd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 10 Aug 2024 11:31:17 -0500 Subject: [PATCH 13/15] [PR #8676/2915102 backport][3.10] Fix type ignore in SSLContext creation connector test (#8677) --- tests/test_connector.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_connector.py b/tests/test_connector.py index 0d6ca18ef53..8dd7a294b30 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -10,7 +10,7 @@ import uuid from collections import deque from contextlib import closing -from typing import Any, List, Optional +from typing import Any, List, Optional, Type from unittest import mock import pytest @@ -1630,16 +1630,14 @@ async def test_ssl_context_once() -> None: @pytest.mark.parametrize("exception", [OSError, ssl.SSLError, asyncio.CancelledError]) -async def test_ssl_context_creation_raises(exception: BaseException) -> None: +async def test_ssl_context_creation_raises(exception: Type[BaseException]) -> None: """Test that we try again if SSLContext creation fails the first time.""" conn = aiohttp.TCPConnector() conn._made_ssl_context.clear() with mock.patch.object( conn, "_make_ssl_context", side_effect=exception - ), pytest.raises( # type: ignore[call-overload] - exception - ): + ), pytest.raises(exception): await conn._make_or_get_ssl_context(True) assert isinstance(await conn._make_or_get_ssl_context(True), ssl.SSLContext) From 8977bae3b78458e494be6a06514ce4c0a8a66f0b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 10 Aug 2024 11:36:52 -0500 Subject: [PATCH 14/15] [PR #8676/2915102 backport][3.11] Fix type ignore in SSLContext creation connector test (#8678) --- tests/test_connector.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_connector.py b/tests/test_connector.py index 0d6ca18ef53..8dd7a294b30 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -10,7 +10,7 @@ import uuid from collections import deque from contextlib import closing -from typing import Any, List, Optional +from typing import Any, List, Optional, Type from unittest import mock import pytest @@ -1630,16 +1630,14 @@ async def test_ssl_context_once() -> None: @pytest.mark.parametrize("exception", [OSError, ssl.SSLError, asyncio.CancelledError]) -async def test_ssl_context_creation_raises(exception: BaseException) -> None: +async def test_ssl_context_creation_raises(exception: Type[BaseException]) -> None: """Test that we try again if SSLContext creation fails the first time.""" conn = aiohttp.TCPConnector() conn._made_ssl_context.clear() with mock.patch.object( conn, "_make_ssl_context", side_effect=exception - ), pytest.raises( # type: ignore[call-overload] - exception - ): + ), pytest.raises(exception): await conn._make_or_get_ssl_context(True) assert isinstance(await conn._make_or_get_ssl_context(True), ssl.SSLContext) From ef20502821a301df5d376d2d93191a13b7f5e895 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 10 Aug 2024 12:08:16 -0500 Subject: [PATCH 15/15] Release 3.10.3 (#8675) --- CHANGES.rst | 68 +++++++++++++++++++++++++++++++++++++++++ CHANGES/8653.bugfix.rst | 1 - CHANGES/8660.misc.rst | 3 -- CHANGES/8661.misc.rst | 1 - CHANGES/8662.misc.rst | 3 -- CHANGES/8667.misc.rst | 1 - CHANGES/8672.bugfix.rst | 3 -- aiohttp/__init__.py | 2 +- 8 files changed, 69 insertions(+), 13 deletions(-) delete mode 100644 CHANGES/8653.bugfix.rst delete mode 100644 CHANGES/8660.misc.rst delete mode 100644 CHANGES/8661.misc.rst delete mode 100644 CHANGES/8662.misc.rst delete mode 100644 CHANGES/8667.misc.rst delete mode 100644 CHANGES/8672.bugfix.rst diff --git a/CHANGES.rst b/CHANGES.rst index 0150c95494c..43ca69235e3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,74 @@ .. towncrier release notes start +3.10.3 (2024-08-10) +======================== + +Bug fixes +--------- + +- Fixed multipart reading when stream buffer splits the boundary over several read() calls -- by :user:`Dreamsorcerer`. + + + *Related issues and pull requests on GitHub:* + :issue:`8653`. + + + +- Fixed :py:class:`aiohttp.TCPConnector` doing blocking I/O in the event loop to create the ``SSLContext`` -- by :user:`bdraco`. + + The blocking I/O would only happen once per verify mode. However, it could cause the event loop to block for a long time if the ``SSLContext`` creation is slow, which is more likely during startup when the disk cache is not yet present. + + + *Related issues and pull requests on GitHub:* + :issue:`8672`. + + + + +Miscellaneous internal changes +------------------------------ + +- Improved performance of :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` when there is no timeout. -- by :user:`bdraco`. + + The timeout context manager is now avoided when there is no timeout as it accounted for up to 50% of the time spent in the :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` methods. + + + *Related issues and pull requests on GitHub:* + :issue:`8660`. + + + +- Improved performance of starting request handlers with Python 3.12+ -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`8661`. + + + +- Improved performance of HTTP keep-alive checks -- by :user:`bdraco`. + + Previously, when processing a request for a keep-alive connection, the keep-alive check would happen every second; the check is now rescheduled if it fires too early instead. + + + *Related issues and pull requests on GitHub:* + :issue:`8662`. + + + +- Improved performance of generating random WebSocket mask -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`8667`. + + + + +---- + + 3.10.2 (2024-08-08) =================== diff --git a/CHANGES/8653.bugfix.rst b/CHANGES/8653.bugfix.rst deleted file mode 100644 index 5c4d66c181f..00000000000 --- a/CHANGES/8653.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fixed multipart reading when stream buffer splits the boundary over several read() calls -- by :user:`Dreamsorcerer`. diff --git a/CHANGES/8660.misc.rst b/CHANGES/8660.misc.rst deleted file mode 100644 index 8710063329e..00000000000 --- a/CHANGES/8660.misc.rst +++ /dev/null @@ -1,3 +0,0 @@ -Improved performance of :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` when there is no timeout. -- by :user:`bdraco`. - -The timeout context manager is now avoided when there is no timeout as it accounted for up to 50% of the time spent in the :py:meth:`~aiohttp.ClientWebSocketResponse.receive` and :py:meth:`~aiohttp.web.WebSocketResponse.receive` methods. diff --git a/CHANGES/8661.misc.rst b/CHANGES/8661.misc.rst deleted file mode 100644 index c0a6fdadb37..00000000000 --- a/CHANGES/8661.misc.rst +++ /dev/null @@ -1 +0,0 @@ -Improved performance of starting request handlers with Python 3.12+ -- by :user:`bdraco`. diff --git a/CHANGES/8662.misc.rst b/CHANGES/8662.misc.rst deleted file mode 100644 index efe30a60cb2..00000000000 --- a/CHANGES/8662.misc.rst +++ /dev/null @@ -1,3 +0,0 @@ -Improved performance of HTTP keep-alive checks -- by :user:`bdraco`. - -Previously, when processing a request for a keep-alive connection, the keep-alive check would happen every second; the check is now rescheduled if it fires too early instead. diff --git a/CHANGES/8667.misc.rst b/CHANGES/8667.misc.rst deleted file mode 100644 index 1c43b6e069a..00000000000 --- a/CHANGES/8667.misc.rst +++ /dev/null @@ -1 +0,0 @@ -Improved performance of generating random WebSocket mask -- by :user:`bdraco`. diff --git a/CHANGES/8672.bugfix.rst b/CHANGES/8672.bugfix.rst deleted file mode 100644 index a57ed16d5d2..00000000000 --- a/CHANGES/8672.bugfix.rst +++ /dev/null @@ -1,3 +0,0 @@ -Fixed :py:class:`aiohttp.TCPConnector` doing blocking I/O in the event loop to create the ``SSLContext`` -- by :user:`bdraco`. - -The blocking I/O would only happen once per verify mode. However, it could cause the event loop to block for a long time if the ``SSLContext`` creation is slow, which is more likely during startup when the disk cache is not yet present. diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index f050229f008..de896a56398 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.10.2" +__version__ = "3.10.3" from typing import TYPE_CHECKING, Tuple