Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hard-coded FD for stdio in subprocess #390

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

fantix
Copy link
Member

@fantix fantix commented Feb 8, 2021

Use FD=0/1/2 for subprocess(stdin=None, stdout=None, stderr=None). This aligns uvloop with the same behavior in asyncio - when stdin, stdout or stderr is None, the subprocess will use FD 0, 1 or 2 now instead of sys.stdin, sys.stdout or sys.stderr.

asyncio eventually calls this to create subprocess, where p2cread, c2pwrite or errwrite are -1 and the current FD 0, 1 and 2 are left untouched to eventually call execv().

Fixes #136

  • Add test
  • Add description
  • Fix commit message

@fantix fantix force-pushed the t136-use-stdio-fd branch from ca69eab to 15ead44 Compare February 9, 2021 01:47
This aligns uvloop with the same behavior in asyncio - when stdin,
stdout or stderr is None, the subprocess will use FD 0, 1 or 2 now
instead of sys.stdin, sys.stdout or sys.stderr.

Fixes MagicStack#136
@fantix fantix force-pushed the t136-use-stdio-fd branch from 15ead44 to 34cdeb6 Compare February 9, 2021 02:37
@fantix fantix marked this pull request as ready for review February 9, 2021 02:57
@fantix fantix requested a review from 1st1 February 9, 2021 02:57
@1st1
Copy link
Member

1st1 commented Feb 9, 2021

This aligns uvloop with the same behavior in asyncio - when stdin, stdout or stderr is None, the subprocess will use FD 0, 1 or 2 now instead of sys.stdin, sys.stdout or sys.stderr.

This is news to me...

@fantix
Copy link
Member Author

fantix commented Feb 9, 2021

Well yeah - looks like it's not strictly FD 0/1/2 but rather a simple fork() followed by an execv(), without modifying FD 0/1/2 by dup2() or ioctl() (as part of set_inheritable()).

@fantix fantix merged commit 8cdb300 into MagicStack:master Feb 9, 2021
@fantix fantix deleted the t136-use-stdio-fd branch February 9, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use sys.stdout in create_subprocess_exec.
2 participants