From 7ab314941e3d02b681207b4bb655c9697481507f Mon Sep 17 00:00:00 2001 From: jb2170 Date: Mon, 2 Dec 2024 19:44:39 +0000 Subject: [PATCH 1/7] Fix gh-127529: Correct asyncio.selector_events.BaseSelectorEventLoop._accept_connection's behaviour for handling ConnectionAbortedError --- Lib/asyncio/selector_events.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index f94bf10b4225e7..542210d5c17958 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -180,9 +180,13 @@ def _accept_connection( logger.debug("%r got a new connection from %r: %r", server, addr, conn) conn.setblocking(False) - except (BlockingIOError, InterruptedError, ConnectionAbortedError): - # Early exit because the socket accept buffer is empty. - return None + except ConnectionAbortedError: + # Discard connections that were aborted before accept(). + continue + except (BlockingIOError, InterruptedError): + # Early exit because of a signal or + # the socket accept buffer is empty. + return except OSError as exc: # There's nowhere to send the error, so just log it. if exc.errno in (errno.EMFILE, errno.ENFILE, From 00f8fa3041cf733b9c4b38869807112a895be1af Mon Sep 17 00:00:00 2001 From: jb2170 Date: Mon, 2 Dec 2024 19:45:30 +0000 Subject: [PATCH 2/7] Add NEWS article --- .../Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst diff --git a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst new file mode 100644 index 00000000000000..dc9f931355be5a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst @@ -0,0 +1,4 @@ +Correct behavior of +:meth:`asyncio.selector_events.BaseSelectorEventLoop._accept_connection` +in handling :exc:`ConnectionAbortedError` in a loop. This improves +performance on OpenBSD From 0f05a32bbf4c75fb74487d7d3a2e7ae11a3615b8 Mon Sep 17 00:00:00 2001 From: jb2170 Date: Mon, 2 Dec 2024 21:05:53 +0000 Subject: [PATCH 3/7] Remove :func:/:meth: from news article; doc checks don't like it for a private method --- .../next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst index dc9f931355be5a..f4c7af640a03d2 100644 --- a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst +++ b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst @@ -1,4 +1,4 @@ Correct behavior of -:meth:`asyncio.selector_events.BaseSelectorEventLoop._accept_connection` +asyncio.selector_events.BaseSelectorEventLoop._accept_connection in handling :exc:`ConnectionAbortedError` in a loop. This improves performance on OpenBSD From 317923d7bd8a4740e379922c2c5f44d70154dd34 Mon Sep 17 00:00:00 2001 From: jb2170 Date: Sun, 29 Dec 2024 21:13:46 +0000 Subject: [PATCH 4/7] Add BaseSelectorEventLoopTests.test_accept_connection_skip_connectionabortederror to test graceful handling of ConnectionAbortedError --- Lib/test/test_asyncio/test_selector_events.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py index aaeda33dd0c677..a65e55191128f0 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -364,6 +364,31 @@ def test_accept_connection_multiple(self): self.loop.run_until_complete(asyncio.sleep(0)) self.assertEqual(sock.accept.call_count, backlog) + def test_accept_connection_skip_connectionabortederror(self): + sock = mock.Mock() + + def mock_sock_accept(): + # mock accept(2) returning -ECONNABORTED every-other + # time that it's called. This applies most to OpenBSD + # whose sockets generate this errno more reproducibly than + # Linux and other OSs. + if sock.accept.call_count % 2 == 0: + raise ConnectionAbortedError + return (mock.Mock(), mock.Mock()) + + sock.accept.side_effect = mock_sock_accept + backlog = 100 + # test that _accept_connection's loop calls sock.accept + # all 100 times, continuing past ConnectionAbortedError + # instead of unnecessarily returning early + mock_obj = mock.patch.object + with mock_obj(self.loop, '_accept_connection2') as accept2_mock: + self.loop._accept_connection( + mock.Mock(), sock, backlog=backlog) + # as in test_accept_connection_multiple avoid task pending + # warnings by using asyncio.sleep(0) + self.loop.run_until_complete(asyncio.sleep(0)) + self.assertEqual(sock.accept.call_count, backlog) class SelectorTransportTests(test_utils.TestCase): From 94afe3efa3f57af4e4b76293b86d8e33f4fd083f Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 3 Jan 2025 15:35:09 +0530 Subject: [PATCH 5/7] Update Lib/test/test_asyncio/test_selector_events.py --- Lib/test/test_asyncio/test_selector_events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py index a65e55191128f0..d1723367428650 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -371,7 +371,7 @@ def mock_sock_accept(): # mock accept(2) returning -ECONNABORTED every-other # time that it's called. This applies most to OpenBSD # whose sockets generate this errno more reproducibly than - # Linux and other OSs. + # Linux and other OS. if sock.accept.call_count % 2 == 0: raise ConnectionAbortedError return (mock.Mock(), mock.Mock()) From 8182a2a6e8e6bb2934b3a1b52ade32097dc3f81e Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 3 Jan 2025 15:35:37 +0530 Subject: [PATCH 6/7] Update Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst --- .../next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst index f4c7af640a03d2..615b270bdb1b60 100644 --- a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst +++ b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst @@ -1,4 +1,4 @@ Correct behavior of -asyncio.selector_events.BaseSelectorEventLoop._accept_connection +:func:`!asyncio.selector_events.BaseSelectorEventLoop._accept_connection` in handling :exc:`ConnectionAbortedError` in a loop. This improves performance on OpenBSD From 651cdd955464c2b782f3725fcfd4036b14a1d01b Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 3 Jan 2025 15:35:51 +0530 Subject: [PATCH 7/7] Update Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst --- .../next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst index 615b270bdb1b60..26f2fd5923ab7b 100644 --- a/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst +++ b/Misc/NEWS.d/next/Library/2024-12-02-19-13-19.gh-issue-127529.Pj1Xtf.rst @@ -1,4 +1,4 @@ Correct behavior of :func:`!asyncio.selector_events.BaseSelectorEventLoop._accept_connection` in handling :exc:`ConnectionAbortedError` in a loop. This improves -performance on OpenBSD +performance on OpenBSD.