8000 bpo-37951: Lift subprocess's fork() restriction (GH-15544) by tiran · Pull Request #15544 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-37951: Lift subprocess's fork() restriction (GH-15544) #15544

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.

Sign up for GitHub

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 2 commits into from
Aug 27, 2019
Merged

bpo-37951: Lift subprocess's fork() restriction (GH-15544) #15544

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c91ab6c
bpo-37951: Lift subprocess's fork() restriction
tiran Aug 27, 2019
224bde7
bpo-37951: Improve doc update
tiran Aug 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-37951: Lift subprocess's fork() restriction 
Signed-off-by: Christian Heimes <christian@python.org>
  • Loading branch information
@tiran
tiran committed Aug 27, 2019
commit c91ab6c9f25e956f0555588e81e567f6409cbc3a
6 changes: 6 additions & 0 deletions Doc/library/subprocess.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,12 @@ functions.
The *start_new_session* parameter can take the place of a previously
common use of *preexec_fn* to call os.setsid() in the child.

.. versionchanged:: 3.8

The *preexec_fn* parameter is no longer supported in subinterpreters.
The new restriction may affect applications that are deployed in
mod_wsgi, uWSGI, and other environments.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that it's useful to document affected applications.

Copy link
Member Author

Choose a reason for hiding this comment

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

The list of affected applications makes it easier for affected users to find the culprit. In my experience it's mostly embedded WSGI servers and embedded applications that use subinterpreters. mod_wsgi and uWSGI are the most common ones.

Copy link
Member

Choose a reason for hiding this comment

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

When you say "no longer supported", here and in whatsnew, mention that use of it raises a RuntimeError.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I do agree that most users probably are not aware that they're in a subinterpreter, so listing some common examples of such environments is not a bad thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gpshead The subinterpreter and whatsnew entries now mention RuntimeError.


If *close_fds* is true, all file descriptors except :const:`0`, :const:`1` and
:const:`2` will be closed before the child process is executed. Otherwise
when *close_fds* is false, file descriptors obey their inheritable flag
Expand Down
5 changes: 5 additions & 0 deletions Doc/whatsnew/3.8.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,11 @@ Changes in the Python API
non-zero :attr:`~Popen.returncode`.
(Contributed by Joannah Nanjekye and Victor Stinner in :issue:`35537`.)

* The *preexec_fn* argument of * :class:`subprocess.Popen` is no longer
compatible with subinterpreters.
(Contributed by Eric Snow in :issue:`34651`, modified by Christian Heimes
in :issue:`37951`.)

* The :meth:`imap.IMAP4.logout` method no longer ignores silently arbitrary
exceptions.

Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2019-08-27-10-03-48.bpo-37951.MfRQgL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Most features of the subprocess module now work again in subinterpreters.
Only *preexec_fn* is restricted in subinterpreters.
5 changes: 3 additions & 2 deletions Modules/_posixsubprocess.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,9 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
&restore_signals, &call_setsid, &preexec_fn))
return NULL;

if (_PyInterpreterState_Get() != PyInterpreterState_Main()) {
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
if ((preexec_fn != Py_None) &&
(_PyInterpreterState_Get() != PyInterpreterState_Main())) {
PyErr_SetString(PyExc_RuntimeError, "preexec_fn not supported for subinterpreters");
Copy link
Member

Choose a reason for hiding this comment

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

s/for/within/

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that makes sense. ETOOLAZY

return NULL;
}

Expand Down
0