Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-91401: Add a failsafe way to disable vfork.#91490
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Just in case there is ever an issue with _posixsubprocess's use ofvfork() due to the complexity of using it properly and potentialdirections that Linux platforms where it defaults to on could take, thisadds a failsafe so that users can disable its use entirely by settinga global flag.No known reason to disable it exists. But it'd be a shame to encounterone and not be able to use CPython without patching and rebuilding it.See the linked issue for some discussion on reasoning.
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.
Why not a simple boolean flag?
Can subprocess disable vfork using disable_vfork_reason if vfork is not available? You can expose a simple private flag in _posixsubprocess saying if vfork is available. Something similar to _testcapi.WITH_PYMALLOC or _thread.TIMEOUT_MAX.
Uh oh!
There was an error while loading.Please reload this page.
I added |
Being consistent with that seems worthwhile, I'll rework this PR. I expect I'm overthinking it with the whole "make it a string" thing and pleading for people to include an explanation of why. |
I chose to add a private attribute since the default value must be safe for all users. But for the corner cases, IMO a private attribute is enough. For vfork, well, it's up to you to add a private or public attribute. |
No double negatives. Also documents _USE_POSIX_SPAWN.
Rather than re-encoding our platform selection here (configure.acand runtime logic in _posixsubprocess already does that).
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.
LGTM. I just have a doubt about the doc format, but you will see when it will be rendered on docs.python.org.
| code. | ||
| .. versionadded:: 3.8 ``_USE_POSIX_SPAWN`` | ||
| .. versionadded:: 3.11 ``_USE_VFORK`` |
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.
I never tried this syntax, I don't know if it's properly rendered. You will see :-)
Debatable whether to include these, but they are now documented:python/cpython#91490
These are now documented:python/cpython#91490
Just in case there is ever an issue with
_posixsubprocess's use ofvfork()due to the complexity of using it properly and potentialdirections that Linux platforms where it defaults to on could take, this
adds a failsafe so that users can disable its use entirely by setting
a global flag.
No known reason to disable it exists. But it'd be a shame to encounter
one and not be able to use CPython without patching and rebuilding it.
See the linked issue for some discussion on reasoning.