Skip to content

bpo-32777: subprocess: Fix usage of _Py_set_inheritable() in child_exec() #5560

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

Merged
merged 4 commits into from
Feb 6, 2018

Conversation

izbyshev
Copy link
Contributor

@izbyshev izbyshev commented Feb 6, 2018

_Py_set_inheritable() raises a Python-level exception on error and
thus is not async-signal-safe, but child_exec() must use only
async-signal-safe functions because it's executed between fork() and
exec().

Fix this by introducing a non-raising version of _Py_set_inheritable()
and using it throughout child_exec().

https://bugs.python.org/issue32777

…ec()

_Py_set_inheritable() raises a Python-level exception on error and
thus is not async-signal-safe, but child_exec() must use only
async-signal-safe functions because it's executed between fork() and
exec().

Fix this by introducing a non-raising version of _Py_set_inheritable()
and using it throughout child_exec().
ioctl() is not async-signal-safe, so avoid using that "fast"-path when
we are asked not to raise an exception (currently the only indicator the
API has).
It better describes the actual purpose of the API.
@gpshead gpshead merged commit c1e46e9 into python:master Feb 6, 2018
@miss-islington
Copy link
Contributor

Thanks @izbyshev for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 6, 2018
…GH-5560)

Fix a rare but potential pre-exec child process deadlock in subprocess on POSIX systems when marking file descriptors inheritable on exec in the child process.  This bug appears to have been introduced in 3.4 with the inheritable file descriptors support.

This also changes Python/fileutils.c `set_inheritable` to use the "slow" two `fcntl` syscall path instead of the "fast" single `ioctl` syscall path when asked to be async signal safe (by way of being asked not to raise exceptions).  `ioctl` is not a POSIX async-signal-safe approved function.

ref: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
(cherry picked from commit c1e46e9)

Co-authored-by: Alexey Izbyshev <izbyshev@users.noreply.github.com>
@bedevere-bot
Copy link

GH-5562 is a backport of this pull request to the 3.7 branch.

@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels Feb 6, 2018
@miss-islington
Copy link
Contributor

Thanks @izbyshev for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 6, 2018
…GH-5560)

Fix a rare but potential pre-exec child process deadlock in subprocess on POSIX systems when marking file descriptors inheritable on exec in the child process.  This bug appears to have been introduced in 3.4 with the inheritable file descriptors support.

This also changes Python/fileutils.c `set_inheritable` to use the "slow" two `fcntl` syscall path instead of the "fast" single `ioctl` syscall path when asked to be async signal safe (by way of being asked not to raise exceptions).  `ioctl` is not a POSIX async-signal-safe approved function.

ref: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
(cherry picked from commit c1e46e9)

Co-authored-by: Alexey Izbyshev <izbyshev@users.noreply.github.com>
@bedevere-bot
Copy link

GH-5563 is a backport of this pull request to the 3.6 branch.

gpshead pushed a commit that referenced this pull request Feb 6, 2018
… (GH-5562)

Fix a rare but potential pre-exec child process deadlock in subprocess on POSIX systems when marking file descriptors inheritable on exec in the child process.  This bug appears to have been introduced in 3.4 with the inheritable file descriptors support.

This also changes Python/fileutils.c `set_inheritable` to use the "slow" two `fcntl` syscall path instead of the "fast" single `ioctl` syscall path when asked to be async signal safe (by way of being asked not to raise exceptions).  `ioctl` is not a POSIX async-signal-safe approved function.

ref: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
(cherry picked from commit c1e46e9)

Co-authored-by: Alexey Izbyshev <izbyshev@users.noreply.github.com>
gpshead pushed a commit that referenced this pull request Feb 6, 2018
… (GH-5563)

Fix a rare but potential pre-exec child process deadlock in subprocess on POSIX systems when marking file descriptors inheritable on exec in the child process.  This bug appears to have been introduced in 3.4 with the inheritable file descriptors support.

This also changes Python/fileutils.c `set_inheritable` to use the "slow" two `fcntl` syscall path instead of the "fast" single `ioctl` syscall path when asked to be async signal safe (by way of being asked not to raise exceptions).  `ioctl` is not a POSIX async-signal-safe approved function.

ref: http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
(cherry picked from commit c1e46e9)

Co-authored-by: Alexey Izbyshev <izbyshev@users.noreply.github.com>
@izbyshev izbyshev deleted the bpo-32777 branch February 6, 2018 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants