GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-98023: Change default child watcher to `PidfdChildWatcher` on supported systems by kumaraditya303 · Pull Request #98024 · python/cpython

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Oct 7, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: Change default child watcher to PidfdChildWatcher on supported Linux versions #98023

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kumaraditya303
switch to pidfd
67b155c
@kumaraditya303
kumaraditya303 requested review from 1st1, asvetlov and gvanrossum as code owners October 7, 2022 09:50
@bedevere-bot bedevere-bot added the awaiting review label Oct 7, 2022
@kumaraditya303
kumaraditya303 marked this pull request as draft October 7, 2022 09:50
blurb-it Bot and others added 2 commits October 7, 2022 09:52
@blurb-it
📜🤖 Added by blurb_it.
1e497f8
@kumaraditya303
Merge branch 'main' into pidfd
2e26bf5
@kumaraditya303 kumaraditya303 added type-feature A feature request or enhancement topic-asyncio 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Oct 7, 2022
@bedevere-bot

bedevere-bot commented Oct 7, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 2e26bf5 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 7, 2022
@kumaraditya303

kumaraditya303 commented Oct 7, 2022

Copy link
Copy Markdown
Contributor Author

The unicode failure will be fixed by #98036 and is unrelated.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kumaraditya303
kumaraditya303 marked this pull request as ready for review October 7, 2022 18:05
gvanrossum
gvanrossum reviewed Oct 7, 2022
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, but...

I reviewed the child watcher architecture with @1st1 and we want to first do a refactoring on PidfdChildWatcher. Basically the current PidfdChildWatcher attaches itself to the event loop in the main thread, and if it is called from a loop in another thread it won't work. ThreadedChildWatcher doesn't have this problem.

HOWEVER, Yury has a simple refactoring of PidfdChildWatcher in mind that will solve this problem! It will make attach_loop() a no-op, and is_active() will always be True -- just as it is for the ThreadedChildWatcher.

So once Yury's change is merged, this can be merged. (We don't quite have a PR but there will be one soon, I promise.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
This was referenced Oct 7, 2022
deprecate the asyncio child watchers system #94597
Closed
gh-94182: run the PidfdChildWatcher on the running loop #94184
Merged
@gvanrossum

gvanrossum commented Oct 7, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Actually, @grangert already has a complete PR (GH-94184) to make PidfdChildWatcher independent from the loop, so we'll merge that, then this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum
Merge branch 'main' into pidfd
e89f4b6
@gvanrossum

gvanrossum commented Oct 8, 2022

Copy link
Copy Markdown
Member

The docs failure is due to something that was accidentally merged, no point in updating and re-running all the tests. The Azure failure is due to the same thing. Still waiting for GH-94184 to go green and merge.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum
gvanrossum merged commit 8ba9378 into python:main Oct 8, 2022
@bedevere-bot bedevere-bot removed the awaiting review label Oct 8, 2022
@kumaraditya303
kumaraditya303 deleted the pidfd branch October 8, 2022 06:15
carljm added a commit to carljm/cpython that referenced this pull request Oct 8, 2022
@carljm
Merge branch 'main' into typewatch
d67d6fd
* 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)
@kumaraditya303 kumaraditya303 mentioned this pull request Oct 9, 2022
gh-82504: deprecate the MultiLoopChildWatcher #94648
Closed
carljm added a commit to carljm/cpython that referenced this pull request Oct 9, 2022
@carljm
Merge branch 'main' into urllibnonet
132c32e
* 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)
  ...
mpage pushed a commit to mpage/cpython that referenced this pull request Oct 11, 2022
@kumaraditya303 @mpage
pythonGH-98023: Change default child watcher to PidfdChildWatcher on …
776f894
…supported systems (python#98024)
bdraco added a commit to bdraco/home-assistant that referenced this pull request Feb 12, 2023
@bdraco
Use PidfdChildWatcher by default when available
29f4931
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
@bdraco bdraco mentioned this pull request Feb 12, 2023
Use PidfdChildWatcher by default when available home-assistant/core#87951
Merged
2 of 19 tasks
balloob pushed a commit to home-assistant/core that referenced this pull request Feb 12, 2023
@bdraco
Use PidfdChildWatcher by default when available (#87951)
d1e1734
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
AlePerla pushed a commit to AlePerla/homeassistant_core that referenced this pull request Feb 17, 2023
@bdraco @AlePerla
Use PidfdChildWatcher by default when available (home-assistant#87951)
638c431
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum left review comments
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-asyncio type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@kumaraditya303 @bedevere-bot @gvanrossum
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.