Skip to content

gh-77043: Make os.dup2(fd, fd, False) always remove inheritability - #102148

Open
izbyshev wants to merge 3 commits into
python:mainfrom
izbyshev:gh-77043-dup2-eq-fd
Open

izbyshev wants to merge 3 commits into
python:mainfrom
izbyshev:gh-77043-dup2-eq-fd

Conversation

@izbyshev

@izbyshev izbyshev commented Feb 22, 2023 •

Copy link
Copy Markdown
Contributor

Currently, os.dup2(fd, fd, False) behaves differently depending on the
platform:

  • On Windows, Linux before 2.6.27, macOS, and FreeBSD it validates the
    fd and makes it non-inheritable.

  • On Solaris it validates the fd, but doesn't change its inheritability.

  • On modern Linux and Illumos it fails with OSError.

Fix this inconsistency by making it always behave as in the first point.

@izbyshev

Copy link
Copy Markdown
Contributor Author

In contrast to PR #5713, instead of adding more conditions to already unnecessarily convoluted logic of os_dup2_impl, I've took the opportunity to refactor it to clearly express what it wants to do (roughly "if not inheritable, try to do special stuff for atomicity; fallback to dup2()).

Comment thread Doc/library/os.rst

If *fd* is equal to *fd2*, the file descriptor is validated, but its
inheritability is not changed unless *inheritable* is ``False``.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Regarding comment from PR #5713, I'm not sure if it should be versionchanged. The current mess is unintentional, so it's not like we had some clear behavior before and then we changed it.

Currently, os.dup2(fd, fd, False) behaves differently depending on the
platform:

* On Windows, Linux before 2.6.27, macOS, and FreeBSD it validates the
  fd and makes it non-inheritable.

* On Solaris it validates the fd, but doesn't change its inheritability.

* On modern Linux and Illumos it fails with OSError.

Fix this inconsistency by making it always behave as in the first point.
@izbyshev
izbyshev force-pushed the gh-77043-dup2-eq-fd branch from 6a7cce6 to 9442ad8 Compare March 5, 2023 09:30
Comment thread Doc/library/os.rst Outdated
@izbyshev
izbyshev force-pushed the gh-77043-dup2-eq-fd branch from 1123368 to 5f2f2e1 Compare April 5, 2023 18:35
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 12, 2026
@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #35338290 | 📁 Comparing 9392115 against main (c6661bd)

  🔍 Preview build  

4 files changed
± library/os.html
± library/shutil.html
± whatsnew/3.16.html
± whatsnew/changelog.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review extension-modules C modules in the Modules dir stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants