Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
gh-83069: Use efficient event-drivensubprocess.Popen.wait() on Linux / macOS / BSD#144047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
+301 −5
Merged
Changes fromall commits
Commits
Show all changes
68 commits Select commitHold shift + click to select a range
288e4db Use pidfd_open() and save the FD as an attribute
giampaolo6e494ca Create _busy_wait() and move code in there
giampaoloaaf193c Move existing code in new _blocking_wait() method
giampaolo7cbb0ad Use _wait_pidfd()
giampaolod9760c3 Use pidfd_open() not at class level but at method level
giampaolo5fd8eec Add kqueue() implementation for macOS and BSD
giampaolo100b111 Add docstrings
giampaolo74ac2f4 Be conservative and check for specific error codes
giampaolofc0cfd6 Document possible failures
giampaolof282459 Add missing import
giampaolo6125976 Write test for pidfd_open() failing
giampaolo41dc6c0 Write test case for pidfd_open() / kqueue failing. Assert fallback is…
giampaolo73ba380 Write test case for kqueue failing. Assert fallback is used.
giampaolo3c8e603 Move tests in their own class
giampaolo932ae58 Add test for terminated PID race
giampaolobb5080a Add test_kqueue_race()
giampaolof067073 Add test_kqueue_control_error
giampaolo4d96c2f Add docstring
giampaolofe05acc Guard against possible slow test
giampaoloadb444e Timeout: use math.ceil to avoid truncation
giampaolo4ec17c1 Remove unused exception var
giampaoloe807ba9 Timeout: use math.ceil to avoid truncation
giampaolo1ddc52b Replace _can_use_kqueue() -> _CAN_USE_KQUEUE
giampaolo645ef6c Shorten code
giampaolo6ee771b Use waitpid() + WNOHANG even if process exited to avoid rare race
giampaolo61c6b99 Revert prev change
giampaolo527646d Use ceil(timeout) to avoid truncation
giampaolo0a8a1b2 Remove check for timeout < 0 + rm test case which didn't make sense.
giampaolo73b97dc Don't use _blocking_wait() as it has a while loop
giampaolo2d3c3f7 Add docstring
giampaolo4eac42f Rm _busy_wait()
giampaolo3c156a9 Add comment
giampaolodac7d3b Add assert
giampaolo5c7ec2f Update comments
giampaolo5c29144 Add test for timeout=0
giampaolo4359b07 Update comment about PID reuse race
giampaolo81275c8 Update comment
giampaolo43b500f Handle rare case where poll() says we're done, but waitpid() doesn't
giampaolob64e42b Update Doc/library/subprocess.rst
giampaoloa101406 Add news entry
giampaolo452f8c4 Add entry in Doc/whatsnew/3.15.rst
giampaolob0c9890 Fix typo
giampaolo27b7c9f Re-wording
giampaoloe1da996 Raise on timeout < 0 and re-add test case
giampaolo5c78acc Check if can really use can_use_pidfd() in unit tests
giampaolodf0538a Check if can really use kqueue() in unit tests
giampaolo6d8e36c Pre-emptively check whether to use the fast way methods
giampaolo6ba7465 Add test_fast_path_avoid_busy_loop
giampaoloe3c7977 Update comments
giampaolo97cc3be Fix missing import on Windows
giampaoloc4342e3 Merge branch 'main' into subprocess-fast-wait
giampaolo5d78d24 Try to fix doc build error
giampaoloa916da3 Try to fix doc build error 2
giampaolo86200bd Try to fix doc build error 3
giampaolo85c38bc Try to fix doc build error 4
giampaolo3c92c1d Try to fix doc build error 5
giampaolo33c8b1f Minor rewordings
giampaolo525c047 Small refact
giampaolo2b931b8 Address review comments (bare assert, don't use ceil())
giampaolod1c6e91 Address review comments (use test class VARIABLES)
giampaolo3e9d303 Address review comments (add periods in doc)
giampaoloa44b2d7 Merge branch 'main' into subprocess-fast-wait
giampaolob4f6020 Remove weird unicode char from doc added by accident
giampaolo4e17856 Don't import select on Windows
giampaolo6f6600d Make kq.control() invocation clearer by passing kwargs
giampaolo37288c6 Revert previous commit
giampaolo300692e Remove bare assert statements
giampaolo7bee1e9 Merge branch 'main' into subprocess-fast-wait
giampaoloFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
21 changes: 18 additions & 3 deletionsDoc/library/subprocess.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1423,6 +1423,8 @@ def test_wait(self): | ||
| deftest_wait_timeout(self): | ||
| p=subprocess.Popen([sys.executable, | ||
| "-c","import time; time.sleep(0.3)"]) | ||
| withself.assertRaises(subprocess.TimeoutExpired)asc: | ||
| p.wait(timeout=0) | ||
| withself.assertRaises(subprocess.TimeoutExpired)asc: | ||
| p.wait(timeout=0.0001) | ||
| self.assertIn("0.0001",str(c.exception))# For coverage of __str__. | ||
| @@ -4094,5 +4096,122 @@ def test_broken_pipe_cleanup(self): | ||
| self.assertTrue(proc.stdin.closed) | ||
| classFastWaitTestCase(BaseTestCase): | ||
| """Tests for efficient (pidfd_open() + poll() / kqueue()) process | ||
| waiting in subprocess.Popen.wait(). | ||
| """ | ||
| CAN_USE_PIDFD_OPEN=subprocess._CAN_USE_PIDFD_OPEN | ||
| CAN_USE_KQUEUE=subprocess._CAN_USE_KQUEUE | ||
giampaolo marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| COMMAND= [sys.executable,"-c","import time; time.sleep(0.3)"] | ||
| WAIT_TIMEOUT=0.0001# 0.1 ms | ||
| defassert_fast_waitpid_error(self,patch_point): | ||
| # Emulate a case where pidfd_open() or kqueue() fails. | ||
| # Busy-poll wait should be used as fallback. | ||
| exc=OSError(errno.EMFILE,os.strerror(errno.EMFILE)) | ||
| withmock.patch(patch_point,side_effect=exc)asm: | ||
| p=subprocess.Popen(self.COMMAND) | ||
| withself.assertRaises(subprocess.TimeoutExpired): | ||
| p.wait(self.WAIT_TIMEOUT) | ||
| self.assertEqual(p.wait(timeout=support.SHORT_TIMEOUT),0) | ||
| self.assertTrue(m.called) | ||
| @unittest.skipIf(notCAN_USE_PIDFD_OPEN,reason="needs pidfd_open()") | ||
| deftest_wait_pidfd_open_error(self): | ||
| self.assert_fast_waitpid_error("os.pidfd_open") | ||
| @unittest.skipIf(notCAN_USE_KQUEUE,reason="needs kqueue() for proc") | ||
| deftest_wait_kqueue_error(self): | ||
| self.assert_fast_waitpid_error("select.kqueue") | ||
| @unittest.skipIf(notCAN_USE_KQUEUE,reason="needs kqueue() for proc") | ||
| deftest_kqueue_control_error(self): | ||
| # Emulate a case where kqueue.control() fails. Busy-poll wait | ||
| # should be used as fallback. | ||
| p=subprocess.Popen(self.COMMAND) | ||
| kq_mock=mock.Mock() | ||
| kq_mock.control.side_effect=OSError( | ||
| errno.EPERM,os.strerror(errno.EPERM) | ||
| ) | ||
| kq_mock.close=mock.Mock() | ||
| withmock.patch("select.kqueue",return_value=kq_mock)asm: | ||
| withself.assertRaises(subprocess.TimeoutExpired): | ||
| p.wait(self.WAIT_TIMEOUT) | ||
| self.assertEqual(p.wait(timeout=support.SHORT_TIMEOUT),0) | ||
| self.assertTrue(m.called) | ||
| defassert_wait_race_condition(self,patch_target,real_func): | ||
| # Call pidfd_open() / kqueue(), then terminate the process. | ||
| # Make sure that the wait call (poll() / kqueue.control()) | ||
| # still works for a terminated PID. | ||
| p=subprocess.Popen(self.COMMAND) | ||
| defwrapper(*args,**kwargs): | ||
| ret=real_func(*args,**kwargs) | ||
| try: | ||
| os.kill(p.pid,signal.SIGTERM) | ||
| os.waitpid(p.pid,0) | ||
| exceptOSError: | ||
| pass | ||
| returnret | ||
| withmock.patch(patch_target,side_effect=wrapper)asm: | ||
| status=p.wait(timeout=support.SHORT_TIMEOUT) | ||
| self.assertTrue(m.called) | ||
| self.assertEqual(status,0) | ||
| @unittest.skipIf(notCAN_USE_PIDFD_OPEN,reason="needs pidfd_open()") | ||
| deftest_pidfd_open_race(self): | ||
| self.assert_wait_race_condition("os.pidfd_open",os.pidfd_open) | ||
| @unittest.skipIf(notCAN_USE_KQUEUE,reason="needs kqueue() for proc") | ||
| deftest_kqueue_race(self): | ||
| self.assert_wait_race_condition("select.kqueue",select.kqueue) | ||
| defassert_notification_without_immediate_reap(self,patch_target): | ||
| # Verify fallback to busy polling when poll() / kqueue() | ||
| # succeeds, but waitpid(pid, WNOHANG) returns (0, 0). | ||
| defwaitpid_wrapper(pid,flags): | ||
| nonlocalncalls | ||
| ncalls+=1 | ||
| ifncalls==1: | ||
| return (0,0) | ||
| returnreal_waitpid(pid,flags) | ||
| ncalls=0 | ||
| real_waitpid=os.waitpid | ||
| withmock.patch.object(subprocess.Popen,patch_target,return_value=True)asm1: | ||
| withmock.patch("os.waitpid",side_effect=waitpid_wrapper)asm2: | ||
| p=subprocess.Popen(self.COMMAND) | ||
| withself.assertRaises(subprocess.TimeoutExpired): | ||
| p.wait(self.WAIT_TIMEOUT) | ||
| self.assertEqual(p.wait(timeout=support.SHORT_TIMEOUT),0) | ||
| self.assertTrue(m1.called) | ||
| self.assertTrue(m2.called) | ||
| @unittest.skipIf(notCAN_USE_PIDFD_OPEN,reason="needs pidfd_open()") | ||
| deftest_pidfd_open_notification_without_immediate_reap(self): | ||
| self.assert_notification_without_immediate_reap("_wait_pidfd") | ||
| @unittest.skipIf(notCAN_USE_KQUEUE,reason="needs kqueue() for proc") | ||
| deftest_kqueue_notification_without_immediate_reap(self): | ||
| self.assert_notification_without_immediate_reap("_wait_kqueue") | ||
| @unittest.skipUnless( | ||
| CAN_USE_PIDFD_OPENorCAN_USE_KQUEUE, | ||
| "fast wait mechanism not available" | ||
| ) | ||
| deftest_fast_path_avoid_busy_loop(self): | ||
| # assert that the busy loop is not called as long as the fast | ||
| # wait is available | ||
| withmock.patch('time.sleep')asm: | ||
| p=subprocess.Popen(self.COMMAND) | ||
| withself.assertRaises(subprocess.TimeoutExpired): | ||
| p.wait(self.WAIT_TIMEOUT) | ||
| self.assertEqual(p.wait(timeout=support.LONG_TIMEOUT),0) | ||
| self.assertFalse(m.called) | ||
| if__name__=="__main__": | ||
| unittest.main() | ||
7 changes: 7 additions & 0 deletionsMisc/NEWS.d/next/Library/2026-01-19-16-45-16.gh-issue-83069.0TaeH9.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| :meth:`subprocess.Popen.wait`: when ``timeout`` is not ``None``, an efficient | ||
| event-driven mechanism now waits for process termination, if available. Linux | ||
| >= 5.3 uses:func:`os.pidfd_open` +:func:`select.poll`. macOS and other BSD | ||
| variants use:func:`select.kqueue` + ``KQ_FILTER_PROC`` + ``KQ_NOTE_EXIT``. | ||
| Windows keeps using ``WaitForSingleObject`` (unchanged). If none of these | ||
| mechanisms are available, the function falls back to the traditional busy loop | ||
| (non-blocking call and short sleeps). Patch by Giampaolo Rodola. |
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.