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-121381 Remove subprocess._USE_VFORK escape hatch by cmaloney · Pull Request #121383 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Jul 4, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This flag was added as an escape hatch in bpo-47245 and backported to Python 3.10. The flag broke at some point between its addition and now. As there is currently no publicly known environments that require this, remove it rather than work on fixing it.

discussion: https://discuss.python.org/t/subprocess-use-vfork-escape-hatch-broken-fix-or-remove/56915/2

📚 Documentation preview 📚: https://cpython-previews--121383.org.readthedocs.build/

  • Issue: subprocess._USE_VFORK doesn't change usage of vfork #121381

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@cmaloney
Remove subprocess._USE_VFORK escape hatch
d19dd0c 
This flag was added as an escape hatch in pythongh-91401 and backported to
Python 3.10. The flag broke at some point between its addition and now.
As there is currently no publicly known environments that require this,
remove it rather than work on fixing it.

This leaves the flag in the subprocess module to not break code which
may have used / checked the flag itself.

discussion: https://discuss.python.org/t/subprocess-use-vfork-escape-hatch-broken-fix-or-remove/56915/2
@cmaloney
cmaloney requested review from gpshead and markshannon as code owners July 4, 2024 21:46
@bedevere-app bedevere-app Bot added the awaiting review label Jul 4, 2024
@cmaloney cmaloney changed the title Remove subprocess._USE_VFORK escape hatch gh-121381 Remove subprocess._USE_VFORK escape hatch Jul 4, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jul 4, 2024
subprocess._USE_VFORK doesn't change usage of vfork #121381
Closed
@cmaloney
cmaloney force-pushed the cmaloney/rm_use_vfork branch from 8dba0d8 to d19dd0c Compare July 4, 2024 21:51
cmaloney added 3 commits July 4, 2024 14:57
@cmaloney
Fix News docs issues
3226642
@cmaloney
docs: Remove repeated _USE_VFORK in rendering
904c87a
@cmaloney
Tweak docs to make case match
cd086a3
vstinner
vstinner reviewed Jul 5, 2024
View reviewed changes
Comment thread Doc/library/subprocess.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@cmaloney
Update to remove _USE_VFORK entirely
9a804f9
vstinner
vstinner reviewed Jul 5, 2024
View reviewed changes

@vstinner vstinner 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.

@gpshead: What do you think? Should we get rid of this feature, or fix it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/subprocess.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
cmaloney added 2 commits July 5, 2024 12:41
@cmaloney
Simplify further per review
8c4cf55
@cmaloney
tweak title underline length
f8540c9
vstinner
vstinner approved these changes Jul 8, 2024
View reviewed changes

@vstinner vstinner 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.

LGTM.

The glibc posix_spawn() can use vfork() automatically and there is no option to control it. I don't think that Python users have to decide if vfork() is used or not. At least, I don't know any good reason to disable vfork() in subprocess.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 8, 2024
@vstinner vstinner added the topic-subprocess Subprocess issues. label Jul 8, 2024
hauntsaninja
hauntsaninja approved these changes Jul 31, 2024
View reviewed changes
@hauntsaninja
hauntsaninja merged commit a9344cd into python:main Jul 31, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 31, 2024
@hauntsaninja

hauntsaninja commented Jul 31, 2024

Copy link
Copy Markdown
Contributor

Thank you!
cmaloney reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cmaloney
cmaloney deleted the cmaloney/rm_use_vfork branch July 31, 2024 02:06
@cmaloney cmaloney mentioned this pull request Jul 31, 2024
GH-120754: Add a strace helper and test set of syscalls for open().read() #121143
Merged
5 tasks done
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@cmaloney @blhsing
pythongh-121381 Remove subprocess._USE_VFORK escape hatch (python#121383
1c0079d 
)

This flag was added as an escape hatch in pythongh-91401 and backported to
Python 3.10. The flag broke at some point between its addition and now.
As there is currently no publicly known environments that require this,
remove it rather than work on fixing it.

This leaves the flag in the subprocess module to not break code which
may have used / checked the flag itself.

discussion: https://discuss.python.org/t/subprocess-use-vfork-escape-hatch-broken-fix-or-remove/56915/2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@hauntsaninja hauntsaninja hauntsaninja approved these changes
@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

topic-subprocess Subprocess issues.

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

@cmaloney @hauntsaninja @vstinner
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.