gh-142302: Fix mkstemp() documentation: clarify file descriptor inheritance behavior#142338
gh-142302: Fix mkstemp() documentation: clarify file descriptor inheritance behavior#142338vstinner merged 13 commits intopython:mainfrom
Conversation
The documentation incorrectly stated that the file descriptor is not inherited by child processes. In reality, the close-on-exec flag (when available) only prevents inheritance across exec() calls, not fork(). The fd will still be inherited by forked child processes.
|
Please sign the CLA and address the CI issues. In addition, please wait for the maintainer to confirm that the documentation should be changed like that. |
Adjust indentation to match the rest of the file formatting.
|
Thank you for your review
|
|
Hopefully this is appropriate to comment here - I don't see that O_CLOEXEC is included as a flag in the tempfile.mkstemp: Following things backward from where the file descriptor is created here: It looks like the flags are created here: Which is just a simple switch to these guys: I have only tested this on "fork" but theoretically the file descriptor will be inherited across the "exec" calls as well since the O_CLOEXEC doesn't appear to be present in any of the flags during the file descriptor creation, and the tempfile.mkstemp function signature does not present the user any options to add O_CLOEXEC or any other filesystem-level flags (presumably by design, since the user could use their own calls to os.open if they needed that fine-grained flag control). |
…w , O_CLOEXEC is not included in the flags used by mkstemp(). The fd will be inherited by all child processes. Thanks to @QuiteAFoxtrot for the investigation
|
@QuiteAFoxtrot You're absolutely right. I've confirmed that _text_openflags = _os.O_RDWR | _os.O_CREAT | _os.O_EXCL
_bin_openflags = _text_openflagsThis means that file descriptor will be inherited by the child processes created via both |
|
@vstinner 's comments on the original issue #142302 show that O_CLOEXEC is part of the default behavior of the call to |
vstinner
left a comment
There was a problem hiding this comment.
This documentation change is wrong, I suggest to close it.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Respectfully, I'd like to point out that the original docs were also wrong - the file descriptor is inherited (and more importantly, usable) with a fork()-no-exec(), shouldn't the docs still be fixed? Apologies if that's not what you meant by suggesting to close. |
|
If you consider that the documentation is unclear, you can complete/clarify https://docs.python.org/dev/library/os.html#fd-inheritance. And you might add a link to this section from mkstemp() doc ("The file descriptor is not inherited by child processes"). |
The original documentation stated 'The file descriptor is not inherited by child processes' without clarifying that this only applies to exec() calls due to O_CLOEXEC (set via PEP 446). The fd is still inherited and usable after fork() before exec(). Update documentation to specifically mention exec() calls for accuracy
|
Thank you @vstinner and @QuiteAFoxtrot for the clarification regarding PEP 446 and O_CLOEXEC behaviour.
|
|
@vstinner I hope you're doing well. Just wanted to check if you had a chance to review the latest changes. No rush - I know you're busy. Happy to make any additional adjustments needed. |
Doc/library/os.rst
Outdated
| they remain accessible after :func:`os.fork` until an exec call occurs. In other | ||
| words, a forked child process can still use the file descriptor, but it will be | ||
| closed if that child process calls exec to run a new program. Inheritable file | ||
| descriptors are inherited across both fork and exec calls. |
There was a problem hiding this comment.
This paragraph is quite verbose. I expected something shorter, such as:
On UNIX, non-inheritable file descriptors are closed in child processes at the
execution of a new program, other file descriptors are inherited. Note that
non-inheritable file descriptors are still *inherited* by child process on :func:`os.fork`.
|
@vstinner Perfect! I've simplified both the changes as you suggested :-
Thank you for the guidance on keeping the docs concise. |
vstinner
left a comment
There was a problem hiding this comment.
LGTM, thanks for the simpler documentation.
|
Thank you sir for the review and the guidance on keeping the documentation concise. Much appreciated! |
|
Thanks @raixyzaditya for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
… inheritance behavior (pythonGH-142338) The documentation incorrectly stated that the file descriptor is not inherited by child processes. In reality, the close-on-exec flag (when available) only prevents inheritance across exec() calls, not fork(). (cherry picked from commit e79c9b7) Co-authored-by: ADITYA RAI <adi.hack1234@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
… inheritance behavior (pythonGH-142338) The documentation incorrectly stated that the file descriptor is not inherited by child processes. In reality, the close-on-exec flag (when available) only prevents inheritance across exec() calls, not fork(). (cherry picked from commit e79c9b7) Co-authored-by: ADITYA RAI <adi.hack1234@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
|
GH-143486 is a backport of this pull request to the 3.14 branch. |
|
GH-143487 is a backport of this pull request to the 3.13 branch. |
…r inheritance behavior (GH-142338) (#143486) gh-142302: Fix mkstemp() documentation: clarify file descriptor inheritance behavior (GH-142338) The documentation incorrectly stated that the file descriptor is not inherited by child processes. In reality, the close-on-exec flag (when available) only prevents inheritance across exec() calls, not fork(). (cherry picked from commit e79c9b7) Co-authored-by: ADITYA RAI <adi.hack1234@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
…r inheritance behavior (GH-142338) (#143487) gh-142302: Fix mkstemp() documentation: clarify file descriptor inheritance behavior (GH-142338) The documentation incorrectly stated that the file descriptor is not inherited by child processes. In reality, the close-on-exec flag (when available) only prevents inheritance across exec() calls, not fork(). (cherry picked from commit e79c9b7) Co-authored-by: ADITYA RAI <adi.hack1234@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
|
Merged, thanks. |
Description
Fixes the misleading documentation for
tempfile.mkstemp()regarding file descriptor inheritance.The current documentation states "The file descriptor is not inherited by child processes," which is incomplete and misleading.
What was wrong
The file descriptor created by
mkstemp()is inherited by child processes created viafork(). The close-on-exec flag (O_CLOEXEC), when available, only prevents inheritance acrossexec()calls, notfork()calls.Changes made
Updated the documentation to accurately describe the inheritance behavior:
O_CLOEXEConly affectsexec()callsTesting
Verified the documentation builds correctly:
cd Doc make htmlRelated Issue
Fixes #142302
📚 Documentation preview 📚: https://cpython-previews--142338.org.readthedocs.build/