From 6bf7e17a1c90797dd1a780fd0805fbbe8b6a8d90 Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Tue, 9 Jun 2026 19:49:39 +0300 Subject: [PATCH 1/4] gh-151179: Fix pidfd leak in asyncio _PidfdChildWatcher --- Lib/asyncio/unix_events.py | 27 ++++++------- Lib/test/test_asyncio/test_unix_events.py | 39 +++++++++++++++++++ ...-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst | 3 ++ 3 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index 915ff845249151c..9e697e74d28ce34 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -876,19 +876,20 @@ def _do_wait(self, pid, pidfd, callback, args): loop = events.get_running_loop() loop._remove_reader(pidfd) try: - _, status = os.waitpid(pid, 0) - except ChildProcessError: - # The child process is already reaped - # (may happen if waitpid() is called elsewhere). - returncode = 255 - logger.warning( - "child process pid %d exit status already read: " - " will report returncode 255", - pid) - else: - returncode = waitstatus_to_exitcode(status) - - os.close(pidfd) + try: + _, status = os.waitpid(pid, 0) + except ChildProcessError: + # The child process is already reaped + # (may happen if waitpid() is called elsewhere). + returncode = 255 + logger.warning( + "child process pid %d exit status already read: " + " will report returncode 255", + pid) + else: + returncode = waitstatus_to_exitcode(status) + finally: + os.close(pidfd) callback(pid, returncode, *args) class _ThreadedChildWatcher: diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index d2b3de3b9a4cb61..b2b032b163174b9 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -1333,5 +1333,44 @@ async def child_main(): self.assertEqual(result.value, 0) + +@unittest.skipUnless( + unix_events.can_use_pidfd(), + "operating system does not support pidfd", +) +class PidfdChildWatcherTests(test_utils.TestCase): + + def setUp(self): + super().setUp() + self.loop = asyncio.new_event_loop() + self.set_event_loop(self.loop) + + def test_pidfd_closed_when_waitpid_raises(self): + # _do_wait() must close the pidfd even when waitpid() + # fails with something other than ChildProcessError, otherwise the + # pidfd is leaked + watcher = unix_events._PidfdChildWatcher() + pid = os.posix_spawn(sys.executable, [sys.executable, '-c', ''], + os.environ) + pidfd = os.pidfd_open(pid) + try: + async def coro(): + with mock.patch.object(os, 'waitpid', + side_effect=OSError('unexpected')): + with self.assertRaises(OSError): + watcher._do_wait(pid, pidfd, lambda *a: None, ()) + + self.loop.run_until_complete(coro()) + + with self.assertRaises(OSError): + os.fstat(pidfd) + finally: + try: + os.close(pidfd) + except OSError: + pass + os.waitpid(pid, 0) + + if __name__ == '__main__': unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst b/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst new file mode 100644 index 000000000000000..a92210d5483ffbe --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst @@ -0,0 +1,3 @@ +Fix a pidfd leak in :class:`_PidfdChildWatcher` on Linux: the watcher no +longer leaks the process file descriptor when ``waitpid()`` fails with an +error other than :exc:`ChildProcessError`. From de437354729cf6dcddd6621a84a6d8c8c4947deb Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Tue, 9 Jun 2026 21:41:16 +0300 Subject: [PATCH 2/4] removed class: from news --- .../next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst b/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst index a92210d5483ffbe..904edf3a8c70908 100644 --- a/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst +++ b/Misc/NEWS.d/next/Library/2026-06-09-19-43-24.gh-issue-151179.hl_6Z0.rst @@ -1,3 +1,3 @@ -Fix a pidfd leak in :class:`_PidfdChildWatcher` on Linux: the watcher no +Fix a pidfd leak in ``_PidfdChildWatcher`` on Linux: the watcher no longer leaks the process file descriptor when ``waitpid()`` fails with an error other than :exc:`ChildProcessError`. From d765491e3a8436fd1dfa2bb66d3ae9490c9da186 Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Wed, 10 Jun 2026 09:19:22 +0300 Subject: [PATCH 3/4] Use try/finally instead of a nested handler --- Lib/asyncio/unix_events.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index 9e697e74d28ce34..b537d9bd674c0cd 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -876,18 +876,17 @@ def _do_wait(self, pid, pidfd, callback, args): loop = events.get_running_loop() loop._remove_reader(pidfd) try: - try: - _, status = os.waitpid(pid, 0) - except ChildProcessError: - # The child process is already reaped - # (may happen if waitpid() is called elsewhere). - returncode = 255 - logger.warning( - "child process pid %d exit status already read: " - " will report returncode 255", - pid) - else: - returncode = waitstatus_to_exitcode(status) + _, status = os.waitpid(pid, 0) + except ChildProcessError: + # The child process is already reaped + # (may happen if waitpid() is called elsewhere). + returncode = 255 + logger.warning( + "child process pid %d exit status already read: " + " will report returncode 255", + pid) + else: + returncode = waitstatus_to_exitcode(status) finally: os.close(pidfd) callback(pid, returncode, *args) From 0eab9be2a711451d04667efed4e476332f447ad4 Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Wed, 10 Jun 2026 20:35:00 +0300 Subject: [PATCH 4/4] Rewrite test to use the real watcher path --- Lib/test/test_asyncio/test_unix_events.py | 39 ++++++++++++----------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py index b2b032b163174b9..118cb866997c51c 100644 --- a/Lib/test/test_asyncio/test_unix_events.py +++ b/Lib/test/test_asyncio/test_unix_events.py @@ -1349,27 +1349,28 @@ def test_pidfd_closed_when_waitpid_raises(self): # _do_wait() must close the pidfd even when waitpid() # fails with something other than ChildProcessError, otherwise the # pidfd is leaked - watcher = unix_events._PidfdChildWatcher() - pid = os.posix_spawn(sys.executable, [sys.executable, '-c', ''], - os.environ) - pidfd = os.pidfd_open(pid) - try: - async def coro(): - with mock.patch.object(os, 'waitpid', - side_effect=OSError('unexpected')): - with self.assertRaises(OSError): - watcher._do_wait(pid, pidfd, lambda *a: None, ()) + self.loop.set_exception_handler(lambda loop, context: None) - self.loop.run_until_complete(coro()) + async def coro(): + before = os_helper.fd_count() + proc = await asyncio.create_subprocess_exec( + sys.executable, '-c', 'import sys; sys.stdin.read()', + stdin=asyncio.subprocess.PIPE + ) - with self.assertRaises(OSError): - os.fstat(pidfd) - finally: - try: - os.close(pidfd) - except OSError: - pass - os.waitpid(pid, 0) + with mock.patch.object(os, 'waitpid', + side_effect=OSError('unexpected')) as m: + proc.stdin.close() + while not m.called: + await asyncio.sleep(0) + + os.waitpid(proc.pid, 0) + proc._transport._process_exited(0) + await proc.wait() + + self.assertEqual(os_helper.fd_count(), before) + + self.loop.run_until_complete(coro()) if __name__ == '__main__':