classification
Title: subprocess: child_exec() uses _Py_set_inheritable() which is not async-signal-safe
Type: behavior Stage: commit review
Components: Extension Modules, Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: gregory.p.smith, izbyshev, vstinner
Priority: normal Keywords: patch

Created on 2018-02-06 01:34 by izbyshev, last changed 2018-02-06 07:25 by izbyshev. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5560 merged izbyshev, 2018-02-06 01:44
PR 5562 merged miss-islington, 2018-02-06 06:09
PR 5563 merged miss-islington, 2018-02-06 06:10
Messages (6)
msg311699 - (view) Author: Alexey Izbyshev (izbyshev) * (Python triager) Date: 2018-02-06 01:34
_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().

Since a non-raising version is already implemented in Python/fileutils.c for internal use (set_inheritable), I suggest to simply expose it via another public function (similar to _Py_open_noraise(), etc.).
msg311706 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-06 06:05
out of curiosity, did you actually diagnose a process deadlocked due to this or was it noted via code inspection?

this issue has been around since 3.4 and the file descriptor inheritance changes from the looks of things.
msg311707 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-06 06:09
New changeset c1e46e94de38a92f98736af9a42d89c3975a9919 by Gregory P. Smith (Alexey Izbyshev) in branch 'master':
bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560)
https://github.com/python/cpython/commit/c1e46e94de38a92f98736af9a42d89c3975a9919
msg311708 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-06 06:31
New changeset 2bb0bfafb0beb819b7c60d9004382ce6867847c0 by Gregory P. Smith (Miss Islington (bot)) in branch '3.7':
bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) (GH-5562)
https://github.com/python/cpython/commit/2bb0bfafb0beb819b7c60d9004382ce6867847c0
msg311709 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-06 06:51
New changeset b90c68586e1f2c45c736dd38880f182be267e2ef by Gregory P. Smith (Miss Islington (bot)) in branch '3.6':
bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) (GH-5563)
https://github.com/python/cpython/commit/b90c68586e1f2c45c736dd38880f182be267e2ef
msg311712 - (view) Author: Alexey Izbyshev (izbyshev) * (Python triager) Date: 2018-02-06 07:25
> out of curiosity, did you actually diagnose a process deadlocked due to this or was it noted via code inspection?

The latter. I noted it while working on another issue. While it was easy to trigger a malloc() in child_exec() by e.g. specifying an invalid fd in pass_fds for Popen, I haven't tried to arrange a deadlock.
History
Date User Action Args
2018-02-06 07:25:34izbyshevsetmessages: + msg311712
2018-02-06 06:52:35gregory.p.smithsetstatus: open -> closed
resolution: fixed
components: + Extension Modules
stage: patch review -> commit review
2018-02-06 06:51:12gregory.p.smithsetmessages: + msg311709
2018-02-06 06:31:24gregory.p.smithsetmessages: + msg311708
2018-02-06 06:10:43miss-islingtonsetpull_requests: + pull_request5386
2018-02-06 06:09:48miss-islingtonsetpull_requests: + pull_request5385
2018-02-06 06:09:36gregory.p.smithsetmessages: + msg311707
2018-02-06 06:05:20gregory.p.smithsetmessages: + msg311706
2018-02-06 04:47:02gregory.p.smithsetassignee: gregory.p.smith

nosy: + gregory.p.smith
2018-02-06 01:44:45izbyshevsetkeywords: + patch
stage: patch review
pull_requests: + pull_request5382
2018-02-06 01:34:03izbyshevcreate