GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-94182: run the PidfdChildWatcher on the running loop by graingert · Pull Request #94184 · python/cpython Conversation
|
|
|
||
|
|
||
| class GenericWatcherTests: | ||
| class GenericWatcherTests(test_utils.TestCase): |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
this test wasn't enabled?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I split this fix out, so this PR is easier to review: #95009
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
test_create_subprocess_fails_with_inactive_watcher was only setting the is_active() method on the context manager not the context manager result
|
|
||
| async def execute(): | ||
| watcher = mock.create_authspec(asyncio.AbstractChildWatcher) | ||
| watcher.is_active.return_value = False |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
this was patching the wrong attribute - asyncio calls asyncio.get_child_watcher().__enter__().is_active()
Sorry, something went wrong.
There was an error while loading. Please reload this page.
otherwise it invalidates test_create_subprocess_with_pidfd
|
@asvetlov can I get a review on this please? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We'll probably merge this ASAP.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
|
|
||
| @functools.cache | ||
| def _has_pidfd_support(): |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This same function is also introduced by @kumaraditya303's PR GH-98024 (which uses PidfdChildWatcher when supported). I propose to refactor things so that everything uses Kumar's version. We can do that in Kumar's PR once this PR is merged.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| def _do_wait(self, pid): | ||
| pidfd, callback, args = self._callbacks.pop(pid) | ||
| self._loop._remove_reader(pidfd) | ||
| def _do_wait(self, pid, pidfd, callback, args): |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Curious why we can't pass the loop as an argument as well? (Though maybe that creates an unhealthy self-reference to the loop.)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
(Waiting for @1st1 review and passing tests.) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Not sure what's wrong with the docs build. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
I've fixed it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Reverted the "fix" because apparently it was caused by an unrelated commit in main. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I can take care of getting CI green and hitting the merge button here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Please do. You might need to rebase this PR to the latest main. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The macOS build hit some GitHub Actions limit. I'm going to merge without waiting for it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* main: pythongh-68686: Retire eptag ptag scripts (python#98064) pythongh-97922: Run the GC only on eval breaker (python#97920) GitHub Workflows security hardening (python#96492) Add `@ezio-melotti` as codeowner for `.github/`. (python#98079) pythongh-97913 Docs: Add walrus operator to the index (python#97921) [doc] Fix broken links to C extensions accelerating stdlib modules (python#96914) pythongh-97822: Fix http.server documentation reference to test() function (python#98027) pythongh-91052: Add PyDict_Unwatch for unwatching a dictionary (python#98055) pythonGH-98023: Change default child watcher to PidfdChildWatcher on supported systems (python#98024) pythonGH-94182: Run the PidfdChildWatcher on the running loop (python#94184)
* main: (5519 commits) Minor edits to the Descriptor HowTo Guide (pythonGH-24901) Fix link to Lifecycle of a Pull Request in CONTRIBUTING (python#98102) pythonGH-94597: deprecate `SafeChildWatcher`, `FastChildWatcher` and `MultiLoopChildWatcher` child watchers (python#98089) Auto-cancel old builds when new commit pushed to branch (python#98009) pythongh-95011: Migrate syslog module to Argument Clinic (pythonGH-95012) pythongh-68686: Retire eptag ptag scripts (python#98064) pythongh-97922: Run the GC only on eval breaker (python#97920) GitHub Workflows security hardening (python#96492) Add `@ezio-melotti` as codeowner for `.github/`. (python#98079) pythongh-97913 Docs: Add walrus operator to the index (python#97921) [doc] Fix broken links to C extensions accelerating stdlib modules (python#96914) pythongh-97822: Fix http.server documentation reference to test() function (python#98027) pythongh-91052: Add PyDict_Unwatch for unwatching a dictionary (python#98055) pythonGH-98023: Change default child watcher to PidfdChildWatcher on supported systems (python#98024) pythonGH-94182: Run the PidfdChildWatcher on the running loop (python#94184) pythongh-92886: make test_ast pass with -O (assertions off) (pythonGH-98058) pythongh-92886: make test_coroutines pass with -O (assertions off) (pythonGH-98060) pythongh-57179: Add note on symlinks for os.walk (python#94799) pythongh-94808: Fix regex on exotic platforms (python#98036) pythongh-90085: Remove vestigial -t and -c timeit options (python#94941) ...
…#94184) There is no reason for this watcher to be attached to any particular loop. This should make it safe to use regardless of the lifetime of the event loop running in the main thread (relative to other loops). Co-authored-by: Yury Selivanov <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]>
This is a backport from cpython 3.12 https://docs.python.org/3/library/asyncio-policy.html > PidfdChildWatcher is a “Goldilocks” child watcher implementation. It doesn’t require signals or threads, doesn’t interfere with any processes launched outside the event loop, and scales linearly with the number of subprocesses launched by the event loop. The main disadvantage is that pidfds are specific to Linux, and only work on recent (5.3+) kernels. python/cpython#98024 There are some additional fixes in cpython 3.12 in python/cpython#94184 when there is no event loop running in the main thread but this is not a problem we have
This is a backport from cpython 3.12 https://docs.python.org/3/library/asyncio-policy.html > PidfdChildWatcher is a “Goldilocks” child watcher implementation. It doesn’t require signals or threads, doesn’t interfere with any processes launched outside the event loop, and scales linearly with the number of subprocesses launched by the event loop. The main disadvantage is that pidfds are specific to Linux, and only work on recent (5.3+) kernels. python/cpython#98024 There are some additional fixes in cpython 3.12 in python/cpython#94184 when there is no event loop running in the main thread but this is not a problem we have
This is a backport from cpython 3.12 https://docs.python.org/3/library/asyncio-policy.html > PidfdChildWatcher is a “Goldilocks” child watcher implementation. It doesn’t require signals or threads, doesn’t interfere with any processes launched outside the event loop, and scales linearly with the number of subprocesses launched by the event loop. The main disadvantage is that pidfds are specific to Linux, and only work on recent (5.3+) kernels. python/cpython#98024 There are some additional fixes in cpython 3.12 in python/cpython#94184 when there is no event loop running in the main thread but this is not a problem we have
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.