From d0896a7076e6fe7f17ebfc22eb10f341ab0590c3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 12 Sep 2018 17:36:03 -0700 Subject: [PATCH 1/6] Fail os.fork() if in a subinterpreter. --- Modules/posixmodule.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 2fddd9587f76b9..f174121d3eb5ab 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -5790,6 +5790,10 @@ os_fork1_impl(PyObject *module) { pid_t pid; + if (_PyInterpreterState_Get() != PyInterpreterState_Main()) { + PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters"); + return NULL; + } PyOS_BeforeFork(); pid = fork1(); if (pid == 0) { @@ -5821,6 +5825,10 @@ os_fork_impl(PyObject *module) { pid_t pid; + if (_PyInterpreterState_Get() != PyInterpreterState_Main()) { + PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters"); + return NULL; + } PyOS_BeforeFork(); pid = fork(); if (pid == 0) { @@ -6416,6 +6424,10 @@ os_forkpty_impl(PyObject *module) int master_fd = -1; pid_t pid; + if (_PyInterpreterState_Get() != PyInterpreterState_Main()) { + PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters"); + return NULL; + } PyOS_BeforeFork(); pid = forkpty(&master_fd, NULL, NULL, NULL); if (pid == 0) { From 1fe097c94fa40a0beab5ed13cb851f914027df37 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 13 Sep 2018 11:06:52 -0700 Subject: [PATCH 2/6] Clear out subinterpreters when re-initializing threads. --- Include/internal/pystate.h | 1 + Modules/posixmodule.c | 2 ++ Python/pystate.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/Include/internal/pystate.h b/Include/internal/pystate.h index ef83af59323e1e..c93dda28954ae8 100644 --- a/Include/internal/pystate.h +++ b/Include/internal/pystate.h @@ -218,6 +218,7 @@ PyAPI_FUNC(_PyInitError) _PyRuntime_Initialize(void); /* Other */ PyAPI_FUNC(_PyInitError) _PyInterpreterState_Enable(_PyRuntimeState *); +PyAPI_FUNC(void) _PyInterpreterState_DeleteExceptMain(void); #ifdef __cplusplus } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f174121d3eb5ab..032de508e76ccd 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -25,6 +25,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" +#include "internal/pystate.h" #include "pythread.h" #include "structmember.h" #ifndef MS_WINDOWS @@ -420,6 +421,7 @@ void PyOS_AfterFork_Child(void) { _PyGILState_Reinit(); + _PyInterpreterState_DeleteExceptMain(); PyEval_ReInitThreads(); _PyImport_ReInitLock(); _PySignal_AfterFork(); diff --git a/Python/pystate.c b/Python/pystate.c index 7d63f4febb932b..7b3d3d4c903259 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -268,6 +268,44 @@ PyInterpreterState_Delete(PyInterpreterState *interp) } +/* + * Delete all interpreter states except the main interpreter. If there + * is a current interpreter state, it *must* be the main interpreter. + */ +void +_PyInterpreterState_DeleteExceptMain() +{ + PyThreadState *tstate = PyThreadState_Swap(NULL); + if (tstate != NULL && tstate->interp != _PyRuntime.interpreters.main) { + Py_FatalError("PyInterpreterState_DeleteExceptMain: not main interpreter"); + } + + HEAD_LOCK(); + PyInterpreterState *interp = _PyRuntime.interpreters.head; + _PyRuntime.interpreters.head = NULL; + for (; interp != NULL; interp = interp->next) { + if (interp == _PyRuntime.interpreters.main) { + _PyRuntime.interpreters.main->next = NULL; + _PyRuntime.interpreters.head = interp; + continue; + } + + PyInterpreterState_Clear(interp); // XXX must activate? + zapthreads(interp); + if (interp->id_mutex != NULL) { + PyThread_free_lock(interp->id_mutex); + } + PyMem_RawFree(interp); + } + HEAD_UNLOCK(); + + if (_PyRuntime.interpreters.head == NULL) { + Py_FatalError("PyInterpreterState_DeleteExceptMain: missing main"); + } + PyThreadState_Swap(tstate); +} + + PyInterpreterState * _PyInterpreterState_Get(void) { From 8b8296ef905b76752a87ca2b6655f282b9c190dd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 13 Sep 2018 12:21:19 -0700 Subject: [PATCH 3/6] Add a NEWS entry. --- .../Core and Builtins/2018-09-13-12-21-08.bpo-34651.v-bUeV.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-09-13-12-21-08.bpo-34651.v-bUeV.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-13-12-21-08.bpo-34651.v-bUeV.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-13-12-21-08.bpo-34651.v-bUeV.rst new file mode 100644 index 00000000000000..a3f0132fc1d69d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-13-12-21-08.bpo-34651.v-bUeV.rst @@ -0,0 +1,3 @@ +Only allow the main interpreter to fork. The avoids the possibility of +affecting the main interprerter, which is critical to operation of the +runtime. From d41f2cb8931f3d561d42f21bbd3a8a71051cb8cd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Sep 2018 09:11:26 -0700 Subject: [PATCH 4/6] Fix a test. --- Lib/test/test__xxsubinterpreters.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/Lib/test/test__xxsubinterpreters.py b/Lib/test/test__xxsubinterpreters.py index ac76cc198ad70d..26032d6c85f2a5 100644 --- a/Lib/test/test__xxsubinterpreters.py +++ b/Lib/test/test__xxsubinterpreters.py @@ -824,23 +824,12 @@ def test_fork(self): expected = 'spam spam spam spam spam' script = dedent(f""" - # (inspired by Lib/test/test_fork.py) import os - pid = os.fork() - if pid == 0: # child + try: + os.fork() + except RuntimeError: with open('{file.name}', 'w') as out: out.write('{expected}') - # Kill the unittest runner in the child process. - os._exit(1) - else: - SHORT_SLEEP = 0.1 - import time - for _ in range(10): - spid, status = os.waitpid(pid, os.WNOHANG) - if spid == pid: - break - time.sleep(SHORT_SLEEP) - assert(spid == pid) """) interpreters.run_string(self.id, script) From 5e507c3fc4304f212a28197100f72b6fcc45bdc6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Sep 2018 11:26:05 -0700 Subject: [PATCH 5/6] Fix Windows. --- Modules/posixmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 032de508e76ccd..0ac0042f124dd8 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -25,7 +25,6 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" -#include "internal/pystate.h" #include "pythread.h" #include "structmember.h" #ifndef MS_WINDOWS @@ -33,6 +32,7 @@ #else #include "winreparse.h" #endif +#include "internal/pystate.h" /* On android API level 21, 'AT_EACCESS' is not declared although * HAVE_FACCESSAT is defined. */ From 4cbce6dffc03fce1d6f5f4c158ee5e0c27bb58ac Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Sep 2018 12:20:34 -0700 Subject: [PATCH 6/6] Disallow fork() in subinterpreters. --- Modules/_posixsubprocess.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index dd69b9eb1ff6c9..9661e38540eda4 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -576,6 +576,11 @@ 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"); + return NULL; + } + if (close_fds && errpipe_write < 3) { /* precondition */ PyErr_SetString(PyExc_ValueError, "errpipe_write must be >= 3"); return NULL;