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-113117: Support posix_spawn in subprocess.Popen with close_fds=True by kulikjak · Pull Request #113118 · python/cpython

Conversation

@kulikjak

@kulikjak kulikjak commented Dec 14, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  • Issue: Add support for posix_spawn call in subprocess.Popen with close_fds=True #113117

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bdraco reacted with hooray emoji bdraco reacted with rocket emoji
All reactions
  • 🎉 1 reaction
  • 🚀 1 reaction
kulikjak added 2 commits December 14, 2023 15:52
@kulikjak
Add support for os.POSIX_SPAWN_CLOSEFROM and posix_spawn_file_actions…
7d48962 
…_addclosefrom_np
@kulikjak
Add documentation
81207b8
@kulikjak
kulikjak requested review from corona10, erlend-aasland and gpshead as code owners December 14, 2023 15:08
@bedevere-app bedevere-app Bot added the awaiting review label Dec 14, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Dec 14, 2023
Add support for posix_spawn call in subprocess.Popen with close_fds=True #113117
Closed
@kulikjak kulikjak changed the title gh-113117: Posix spawn close fds support gh-113117: Support posix_spawn in subprocess.Popen with close_fds=True Dec 14, 2023
@kulikjak kulikjak mentioned this pull request Dec 14, 2023
subprocess.Popen can pass incomplete environment when posix_spawn is used #113119
Closed
@kulikjak
Add regenerated configure
79d094e
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka December 14, 2023 20:37
serhiy-storchaka
serhiy-storchaka approved these changes Dec 15, 2023
View reviewed changes

@serhiy-storchaka serhiy-storchaka 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. Please add a NEWS and What's New entries.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gpshead reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Modules/posixmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Dec 15, 2023
gpshead
gpshead approved these changes Dec 15, 2023
View reviewed changes
@gpshead gpshead self-assigned this Dec 15, 2023
@ronaldoussoren ronaldoussoren mentioned this pull request Dec 16, 2023
Support the macOS-only POSIX_SPAWN_CLOEXEC_DEFAULT flag and posix_spawn_file_actions_addinherit_np() in os.posix_spawn #109154
Open
kulikjak added 2 commits December 16, 2023 10:49
@kulikjak
use PyModule_AddIntMacro
6f17115
@kulikjak
Add NEWS fragment and whatsnew/3.13 entry
4a99eef
gpshead
gpshead approved these changes Dec 17, 2023
View reviewed changes
Comment thread Modules/posixmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 17, 2023
@bedevere-bot

This comment was marked as outdated.

Sign in to view
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 17, 2023
gpshead added 2 commits December 16, 2023 23:16
@gpshead
Merge branch 'main' into posix_spawn-close_fds-support
55790a1
@gpshead
Have vfork specific tests disable posix_spawn.
cb496e3
@gpshead gpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 17, 2023
@bedevere-bot

bedevere-bot commented Dec 17, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @gpshead for commit cb496e3 🤖

If you want to schedule another build, you need to add the 🔨 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 Dec 17, 2023
gpshead added 2 commits December 17, 2023 12:32
@gpshead
Expand upon the What's New text.
dfa43af 
Gives a more useful description and mentions that it can be turned off but that we'd like to hear from anyone doing so.
@gpshead
fix syntax typos in what's new
4c96d42 
(the joys of using the github web editor)
@gpshead
Reword What's New, add os and subprocess headers.
654f630 
This describes the module changes in module sections and makes the
Optimizations section brief, referring back up to the subprocess
module.
@gpshead
gpshead enabled auto-merge (squash) December 17, 2023 21:26
@gpshead gpshead added type-feature A feature request or enhancement 3.13 only security fixes labels Dec 17, 2023
@gpshead
gpshead merged commit 2b93f52 into python:main Dec 17, 2023
@bedevere-app bedevere-app Bot removed the awaiting merge label Dec 17, 2023
ryan-duve pushed a commit to ryan-duve/cpython that referenced this pull request Dec 26, 2023
@kulikjak @gpshead @ryan-duve
pythongh-113117: Support posix_spawn in subprocess.Popen with close_f…
d08a42d 
…ds=True (python#113118)

Add support for `os.POSIX_SPAWN_CLOSEFROM` and
`posix_spawn_file_actions_addclosefrom_np` and have the `subprocess` module use
them when available.  This means `posix_spawn` can now be used in the default
`close_fds=True` situation on many platforms.

Co-authored-by: Gregory P. Smith [Google LLC] <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@kulikjak @gpshead @aisk
pythongh-113117: Support posix_spawn in subprocess.Popen with close_f…
61befbe 
…ds=True (python#113118)

Add support for `os.POSIX_SPAWN_CLOSEFROM` and
`posix_spawn_file_actions_addclosefrom_np` and have the `subprocess` module use
them when available.  This means `posix_spawn` can now be used in the default
`close_fds=True` situation on many platforms.

Co-authored-by: Gregory P. Smith [Google LLC] <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@kulikjak @gpshead @Glyphack
pythongh-113117: Support posix_spawn in subprocess.Popen with close_f…
8899d1a 
…ds=True (python#113118)

Add support for `os.POSIX_SPAWN_CLOSEFROM` and
`posix_spawn_file_actions_addclosefrom_np` and have the `subprocess` module use
them when available.  This means `posix_spawn` can now be used in the default
`close_fds=True` situation on many platforms.

Co-authored-by: Gregory P. Smith [Google LLC] <[email protected]>
@vstinner

vstinner commented Oct 18, 2024

Copy link
Copy Markdown
Member

I just discover this optimization by reading What's New in Python 3.13. Congrats @kulikjak, it's a great step forward! I didn't know that Linux supported this feature!
kulikjak reacted with hooray emoji gpshead and erlend-aasland reacted with heart emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

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

kulikjak commented Oct 20, 2024

Copy link
Copy Markdown
Contributor Author

^^ Thanks! Though I am not sure how much Linux will benefit, considering it has vfork.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vfazio vfazio mentioned this pull request Jan 22, 2025
Doc: mention a minimal version of QEMU user emulation necessary for 3.13+? #129204
Closed
@lazka lazka mentioned this pull request Sep 21, 2025
[backport] Cygwin: lock cygheap during fork msys2/msys2-runtime#312
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner

Uh oh!

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

Assignees

@gpshead gpshead

Labels

3.13 only security fixes 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.

5 participants

@kulikjak @bedevere-bot @vstinner @gpshead @serhiy-storchaka
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.