From dc5d78702499dfc7d5d81f4522cbbd5d12b9ac25 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Thu, 30 Dec 2021 14:19:39 -0800 Subject: [PATCH 01/14] Fix deadlock in print. --- Modules/_io/bufferedio.c | 13 ++++++++ Python/pystate.c | 68 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 28995051abd4bb..c1b6e552d36563 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -428,6 +428,18 @@ buffered_clear(buffered *self) return 0; } +static PyObject * +buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) +{ + int ret = _PyThread_at_fork_reinit(&self->lock); + self->owner = 0; + if (ret == 0) { + return Py_True; + } else { + return Py_False; + } +} + /* Because this can call arbitrary code, it shouldn't be called when the refcount is 0 (that is, not directly from tp_dealloc unless the refcount has been temporarily re-incremented). */ @@ -2493,6 +2505,7 @@ static PyMethodDef bufferedwriter_methods[] = { _IO__BUFFERED_SEEK_METHODDEF {"tell", (PyCFunction)buffered_tell, METH_NOARGS}, {"__sizeof__", (PyCFunction)buffered_sizeof, METH_NOARGS}, + {"_at_fork_reinit", (PyCFunction)buffered_at_fork_reinit, METH_NOARGS}, {NULL, NULL} }; diff --git a/Python/pystate.c b/Python/pystate.c index 68fae8d283091d..25e9f4eff1c287 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -171,6 +171,65 @@ _PyRuntimeState_Fini(_PyRuntimeState *runtime) } #ifdef HAVE_FORK +_Py_IDENTIFIER(stdout); +_Py_IDENTIFIER(stderr); + +static int stdio_at_fork_reinit(_Py_Identifier *key) +{ + + int ret = 0; + + PyObject *stdio, *closed, *isatty, *buffer, *result; + + stdio = _PySys_GetObjectId(key); + + _Py_IDENTIFIER(closed); + if (_PyObject_LookupAttrId(stdio, &PyId_closed, &closed) < 0) { + PyErr_Clear(); + goto end; + } + + if (Py_IsTrue(closed)) { + goto end; + } + + _Py_IDENTIFIER(isatty); + if (_PyObject_GetAttrId(stdio, &PyId_isatty) == NULL) { + PyErr_Clear(); + goto end; + } + + isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); + if (Py_IsFalse(isatty)) { + goto end; + } + + _Py_IDENTIFIER(buffer); + if (_PyObject_LookupAttrId(stdio, &PyId_buffer, &buffer) < 0) { + /* stdout.buffer and stderr.buffer are not part of the + * TextIOBase API and may not exist in some implementations. + * If not present, no need to reinitialize their locks. */ + goto end; + } + + _Py_IDENTIFIER(_at_fork_reinit); + if (_PyObject_GetAttrId(buffer, &PyId__at_fork_reinit) == NULL) { + PyErr_Clear(); + goto end; + } + + result = _PyObject_CallMethodIdNoArgs(buffer, &PyId__at_fork_reinit); + + if (result == Py_True) { + goto end; + } + + /* error */ + ret = -1; +end: + return ret; +} + /* This function is called from PyOS_AfterFork_Child to ensure that newly created child processes do not share locks with the parent. */ PyStatus @@ -202,6 +261,15 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) return _PyStatus_ERR("Failed to reinitialize runtime locks"); } + + int reinit_stdout = stdio_at_fork_reinit(&PyId_stdout); + int reinit_stderr = stdio_at_fork_reinit(&PyId_stderr); + + if (reinit_stdout < 0 || reinit_stderr < 0) { + return _PyStatus_ERR("Failed to reinitialize stdout and stderr"); + } + + return _PyStatus_OK(); } #endif From 46d4ce68808e81c87279611a446c0584d9576f60 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Thu, 30 Dec 2021 15:51:19 -0800 Subject: [PATCH 02/14] Add Misc/NEWS.d entry. --- .../Core and Builtins/2021-12-30-15-50-39.bpo-46210.YRy67_.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-12-30-15-50-39.bpo-46210.YRy67_.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-12-30-15-50-39.bpo-46210.YRy67_.rst b/Misc/NEWS.d/next/Core and Builtins/2021-12-30-15-50-39.bpo-46210.YRy67_.rst new file mode 100644 index 00000000000000..c3ca26debf2a01 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-12-30-15-50-39.bpo-46210.YRy67_.rst @@ -0,0 +1,2 @@ +Add :c:func:`_at_fork_reinit` handler to ``buffer->lock`` of stdout and +stderr, to fix a deadlock in print. From 3409181c10363050dd0d5f59c52bd9dc306df912 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Thu, 30 Dec 2021 16:28:13 -0800 Subject: [PATCH 03/14] Only add _at_fork_reinit if fork is available. --- Modules/_io/bufferedio.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index c1b6e552d36563..270f9d8af69b5e 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -428,6 +428,7 @@ buffered_clear(buffered *self) return 0; } +#ifdef HAVE_FORK static PyObject * buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) { @@ -439,6 +440,7 @@ buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) return Py_False; } } +#endif /* Because this can call arbitrary code, it shouldn't be called when the refcount is 0 (that is, not directly from tp_dealloc unless @@ -2505,7 +2507,9 @@ static PyMethodDef bufferedwriter_methods[] = { _IO__BUFFERED_SEEK_METHODDEF {"tell", (PyCFunction)buffered_tell, METH_NOARGS}, {"__sizeof__", (PyCFunction)buffered_sizeof, METH_NOARGS}, +#ifdef HAVE_FORK {"_at_fork_reinit", (PyCFunction)buffered_at_fork_reinit, METH_NOARGS}, +#endif {NULL, NULL} }; From 19f19b7dec9a07c99ddae9f1f746f7c79977ffc8 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Thu, 30 Dec 2021 18:01:51 -0800 Subject: [PATCH 04/14] Use Py_RETURN_* in Modules/_io/bufferedio.c:buffered_at_fork_reinit --- Modules/_io/bufferedio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 270f9d8af69b5e..d6d0c52a88c339 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -435,9 +435,9 @@ buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) int ret = _PyThread_at_fork_reinit(&self->lock); self->owner = 0; if (ret == 0) { - return Py_True; + Py_RETURN_TRUE; } else { - return Py_False; + Py_RETURN_FALSE; } } #endif From f5334b40cea86a978715c7f13f7d31de97a7d1dc Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Thu, 30 Dec 2021 17:39:31 -0800 Subject: [PATCH 05/14] Fix up reference counting in Python/pystate.c:stdio_at_fork_reinit --- Python/pystate.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 25e9f4eff1c287..ad9fb837e4cdaa 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -179,27 +179,18 @@ static int stdio_at_fork_reinit(_Py_Identifier *key) int ret = 0; - PyObject *stdio, *closed, *isatty, *buffer, *result; + PyObject *isatty = NULL; + PyObject *buffer = NULL; + PyObject *result = NULL; - stdio = _PySys_GetObjectId(key); - - _Py_IDENTIFIER(closed); - if (_PyObject_LookupAttrId(stdio, &PyId_closed, &closed) < 0) { - PyErr_Clear(); - goto end; - } - - if (Py_IsTrue(closed)) { - goto end; - } + PyObject *stdio = _PySys_GetObjectId(key); _Py_IDENTIFIER(isatty); - if (_PyObject_GetAttrId(stdio, &PyId_isatty) == NULL) { + isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); + if (isatty == NULL) { PyErr_Clear(); goto end; } - - isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); if (Py_IsFalse(isatty)) { goto end; } @@ -219,14 +210,16 @@ static int stdio_at_fork_reinit(_Py_Identifier *key) } result = _PyObject_CallMethodIdNoArgs(buffer, &PyId__at_fork_reinit); - - if (result == Py_True) { + if (Py_IsTrue(result)) { goto end; } /* error */ ret = -1; end: + Py_XDECREF(isatty); + Py_XDECREF(buffer); + Py_XDECREF(result); return ret; } From 385800657009ababd9d17b798da4b8a92b9a43b4 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Wed, 5 Jan 2022 11:19:18 -0800 Subject: [PATCH 06/14] Formatting for PEP 7. --- Modules/_io/bufferedio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index d6d0c52a88c339..da191f16fafc75 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -436,7 +436,8 @@ buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) self->owner = 0; if (ret == 0) { Py_RETURN_TRUE; - } else { + } + else { Py_RETURN_FALSE; } } From fb0bd1645042e85139df4f92ca8636e682b78ffa Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Wed, 5 Jan 2022 11:42:55 -0800 Subject: [PATCH 07/14] Add _PySys_ReInitStdio to Python/sysmodule.c --- Include/internal/pycore_sysmodule.h | 4 ++ Python/pystate.c | 61 -------------------------- Python/sysmodule.c | 66 +++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 61 deletions(-) diff --git a/Include/internal/pycore_sysmodule.h b/Include/internal/pycore_sysmodule.h index 738a7746a03842..9ea20c08b4ebc6 100644 --- a/Include/internal/pycore_sysmodule.h +++ b/Include/internal/pycore_sysmodule.h @@ -8,6 +8,10 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +#ifdef HAVE_FORK +extern PyStatus _PySys_ReInitStdio(void); +#endif + PyAPI_FUNC(int) _PySys_Audit( PyThreadState *tstate, const char *event, diff --git a/Python/pystate.c b/Python/pystate.c index ad9fb837e4cdaa..68fae8d283091d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -171,58 +171,6 @@ _PyRuntimeState_Fini(_PyRuntimeState *runtime) } #ifdef HAVE_FORK -_Py_IDENTIFIER(stdout); -_Py_IDENTIFIER(stderr); - -static int stdio_at_fork_reinit(_Py_Identifier *key) -{ - - int ret = 0; - - PyObject *isatty = NULL; - PyObject *buffer = NULL; - PyObject *result = NULL; - - PyObject *stdio = _PySys_GetObjectId(key); - - _Py_IDENTIFIER(isatty); - isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); - if (isatty == NULL) { - PyErr_Clear(); - goto end; - } - if (Py_IsFalse(isatty)) { - goto end; - } - - _Py_IDENTIFIER(buffer); - if (_PyObject_LookupAttrId(stdio, &PyId_buffer, &buffer) < 0) { - /* stdout.buffer and stderr.buffer are not part of the - * TextIOBase API and may not exist in some implementations. - * If not present, no need to reinitialize their locks. */ - goto end; - } - - _Py_IDENTIFIER(_at_fork_reinit); - if (_PyObject_GetAttrId(buffer, &PyId__at_fork_reinit) == NULL) { - PyErr_Clear(); - goto end; - } - - result = _PyObject_CallMethodIdNoArgs(buffer, &PyId__at_fork_reinit); - if (Py_IsTrue(result)) { - goto end; - } - - /* error */ - ret = -1; -end: - Py_XDECREF(isatty); - Py_XDECREF(buffer); - Py_XDECREF(result); - return ret; -} - /* This function is called from PyOS_AfterFork_Child to ensure that newly created child processes do not share locks with the parent. */ PyStatus @@ -254,15 +202,6 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) return _PyStatus_ERR("Failed to reinitialize runtime locks"); } - - int reinit_stdout = stdio_at_fork_reinit(&PyId_stdout); - int reinit_stderr = stdio_at_fork_reinit(&PyId_stderr); - - if (reinit_stdout < 0 || reinit_stderr < 0) { - return _PyStatus_ERR("Failed to reinitialize stdout and stderr"); - } - - return _PyStatus_OK(); } #endif diff --git a/Python/sysmodule.c b/Python/sysmodule.c index f912115560704c..56850cadca61a9 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2861,6 +2861,72 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict) return _PyStatus_ERR("can't initialize sys module"); } +#ifdef HAVE_FORK +static int +stdio_at_fork_reinit(_Py_Identifier *key) +{ + + int ret = 0; + + PyObject *isatty = NULL; + PyObject *buffer = NULL; + PyObject *result = NULL; + + PyObject *stdio = _PySys_GetObjectId(key); + + _Py_IDENTIFIER(isatty); + isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); + if (isatty == NULL) { + PyErr_Clear(); + goto end; + } + if (Py_IsFalse(isatty)) { + goto end; + } + + _Py_IDENTIFIER(buffer); + if (_PyObject_LookupAttrId(stdio, &PyId_buffer, &buffer) < 0) { + /* stdout.buffer and stderr.buffer are not part of the + * TextIOBase API and may not exist in some implementations. + * If not present, no need to reinitialize their locks. */ + goto end; + } + + _Py_IDENTIFIER(_at_fork_reinit); + if (_PyObject_GetAttrId(buffer, &PyId__at_fork_reinit) == NULL) { + PyErr_Clear(); + goto end; + } + + result = _PyObject_CallMethodIdNoArgs(buffer, &PyId__at_fork_reinit); + if (Py_IsTrue(result)) { + goto end; + } + + /* error */ + ret = -1; +end: + Py_XDECREF(isatty); + Py_XDECREF(buffer); + Py_XDECREF(result); + return ret; +} + +/* This function is called from PyOS_AfterFork_Child() to ensure that newly + created child processes do not share locks with the parent. */ +PyStatus +_PySys_ReInitStdio(void) +{ + int reinit_stdout = stdio_at_fork_reinit(&PyId_stdout); + int reinit_stderr = stdio_at_fork_reinit(&PyId_stderr); + + if (reinit_stdout < 0 || reinit_stderr < 0) { + return _PyStatus_ERR("Failed to reinitialize stdout and stderr"); + } + return _PyStatus_OK(); +} +#endif + static int sys_add_xoption(PyObject *opts, const wchar_t *s) { From 8c8d9c9997184caff123be37bb19c4a009ef811b Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Wed, 5 Jan 2022 11:45:01 -0800 Subject: [PATCH 08/14] Call _PySys_ReInitStdio in PyOS_AfterFork_Child. --- Modules/posixmodule.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b3a5757a8221dd..efa0c7a12ba6a1 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -26,6 +26,7 @@ #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_ceval.h" // _PyEval_ReInitThreads() #include "pycore_import.h" // _PyImport_ReInitLock() +#include "pycore_sysmodule.h" // _PySys_ReInitStdio() #include "pycore_initconfig.h" // _PyStatus_EXCEPTION() #include "pycore_pystate.h" // _PyInterpreterState_GET() @@ -593,6 +594,11 @@ PyOS_AfterFork_Child(void) goto fatal_error; } + status = _PySys_ReInitStdio(); + if (_PyStatus_EXCEPTION(status)) { + goto fatal_error; + } + _PySignal_AfterFork(); status = _PyRuntimeState_ReInitThreads(runtime); From a14bf74dcdc4a779ec8cc2e6dd4585988a292415 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Mon, 24 Jan 2022 13:41:54 -0800 Subject: [PATCH 09/14] Avoid adding a new python method. --- Modules/_io/bufferedio.c | 23 +++++++++++------------ Python/sysmodule.c | 16 ++++------------ 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index da191f16fafc75..65351d09a356d9 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -429,17 +429,19 @@ buffered_clear(buffered *self) } #ifdef HAVE_FORK -static PyObject * -buffered_at_fork_reinit(buffered *self, PyObject *Py_UNUSED(ignored)) +int +_PyIO_buffered_at_fork_reinit(PyObject *self) { - int ret = _PyThread_at_fork_reinit(&self->lock); - self->owner = 0; - if (ret == 0) { - Py_RETURN_TRUE; - } - else { - Py_RETURN_FALSE; + Py_INCREF(self); + if (!Py_IS_TYPE(self, &PyBufferedWriter_Type)) { + Py_DECREF(self); + return 0; } + buffered *buffer = (buffered *)self; + int ret = _PyThread_at_fork_reinit(&buffer->lock); + buffer->owner = 0; + Py_DECREF(self); + return ret; } #endif @@ -2508,9 +2510,6 @@ static PyMethodDef bufferedwriter_methods[] = { _IO__BUFFERED_SEEK_METHODDEF {"tell", (PyCFunction)buffered_tell, METH_NOARGS}, {"__sizeof__", (PyCFunction)buffered_sizeof, METH_NOARGS}, -#ifdef HAVE_FORK - {"_at_fork_reinit", (PyCFunction)buffered_at_fork_reinit, METH_NOARGS}, -#endif {NULL, NULL} }; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 56850cadca61a9..0aa31cf8a847f3 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2862,6 +2862,8 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict) } #ifdef HAVE_FORK +extern int _PyIO_buffered_at_fork_reinit(PyObject *); + static int stdio_at_fork_reinit(_Py_Identifier *key) { @@ -2892,19 +2894,9 @@ stdio_at_fork_reinit(_Py_Identifier *key) goto end; } - _Py_IDENTIFIER(_at_fork_reinit); - if (_PyObject_GetAttrId(buffer, &PyId__at_fork_reinit) == NULL) { - PyErr_Clear(); - goto end; - } - - result = _PyObject_CallMethodIdNoArgs(buffer, &PyId__at_fork_reinit); - if (Py_IsTrue(result)) { - goto end; - } + /* reinitialize buffer->lock */ + ret = _PyIO_buffered_at_fork_reinit(buffer); - /* error */ - ret = -1; end: Py_XDECREF(isatty); Py_XDECREF(buffer); From be40192b8ba06a8865c8704d44d6b108bb1eb669 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Mon, 24 Jan 2022 15:06:49 -0800 Subject: [PATCH 10/14] Add TestStdioAtForkReInit to test_thread.py --- Lib/test/test_thread.py | 104 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index 4ae8a833b990d7..9f5a297a28b20c 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -1,4 +1,6 @@ +import io import os +import sys import unittest import random from test import support @@ -265,5 +267,107 @@ def tearDown(self): pass +class TestStdioAtForkReInit(unittest.TestCase): + + class MockStdio(io.TextIOWrapper): + def __init__(self): + import _io, tempfile + self._file = tempfile.mktemp() + raw = _io.FileIO(self._file, mode='wb') + buffer = _io.BufferedWriter(raw) + super().__init__(buffer) + + def __del__(self): + try: + self.close() + os.remove(self._file) + except: + pass + + def isatty(self): + # pretend to be a tty, so _PySys_ReInitStdio + # will attempt to reinitialize our buffer lock. + return True + + def setUp(self): + # Replace stdout & stderr with mock objects with the + # same underlying buffer type to avoid polluting the + # test output stream. + self._saved_stdout = sys.stdout + self._saved_stderr = sys.stderr + sys.stdout = self.MockStdio() + sys.stderr = self.MockStdio() + + def tearDown(self): + sys.stdout = self._saved_stdout + sys.stderr = self._saved_stderr + + @unittest.skipUnless(hasattr(os, 'fork'), 'need os.fork') + @threading_helper.reap_threads + def test_stdio_at_fork_reinit(self): + + # bpo-46210: Added _PySys_ReInitStdio to prevent children + # from deadlocking here if printer_thread held the stdout + # (or stderr) buffer's lock when one of the children forked. + + num_prints = 100 + num_forks = 100 + msg = 'hello'*10 + + def printer_thread(stdio): + for n in range(num_prints): + print(msg, file=stdio) + stdio.flush() + + def fork_processes(stdio): + pids = [] + for n in range(num_forks): + if pid := os.fork(): + pids.append(pid) + else: + print(msg, file=stdio) + stdio.flush() + os._exit(0) + + return pids + + def main(stdio): + thread.start_new_thread(printer_thread, (stdio,)) + pids = fork_processes(stdio) + for pid in pids: + support.wait_process(pid, exitcode=0) + return + + # Forking below is not necessary to illustrate the bug + # in bpo-46210 and its fix. The issue in bpo-46210 is + # that calling main(sys.stdout) or main(sys.stderr) + # is sufficient to cause a deadlock in print. We fork + # here only to allow us to give a single timeout to the + # main() call, since its failure mode (absent the fix) + # is for some subset of the forked child processes to + # deadlock at the moment when they try to print, rather + # than to raise an exception. Therefore, simply looping + # over the child pids and calling support.wait_process + # with a separate nonzero timeout for each child leads + # to a rather unpredictable total wait time, whereas + # forking again here at the top (though not necessary + # to illustrate the bug) allows us to give a predictable + # timeout to the process of waiting for the children. + # + # bpo-46210 is present if and only if one or more of the + # children forked by main() deadlock when they call print. + # + # pr-30310 proposes a fix following the example of the + # import lock, by providing a function _PySys_ReInitStdio + # that is called alongside the other preexisting lock + # reinitialization functions in PyOS_AfterFork_Child. + for stdio in (sys.stdout, sys.stderr): + with threading_helper.wait_threads_exit(): + if main_pid := os.fork(): + support.wait_process(main_pid, exitcode=0, timeout=5) + else: + main(stdio) + os._exit(0) + if __name__ == "__main__": unittest.main() From bf9b39889f108901811c51a7805e4032e55bf44b Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Mon, 7 Feb 2022 11:37:53 -0800 Subject: [PATCH 11/14] Move INCREF in _PyIO_buffered_at_fork_reinit --- Modules/_io/bufferedio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 65351d09a356d9..0ca22209f65b0d 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -432,11 +432,10 @@ buffered_clear(buffered *self) int _PyIO_buffered_at_fork_reinit(PyObject *self) { - Py_INCREF(self); if (!Py_IS_TYPE(self, &PyBufferedWriter_Type)) { - Py_DECREF(self); return 0; } + Py_INCREF(self); buffered *buffer = (buffered *)self; int ret = _PyThread_at_fork_reinit(&buffer->lock); buffer->owner = 0; From 5681a021fb92a8fa886a758bab98f219ee8c16ba Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Mon, 7 Feb 2022 11:40:20 -0800 Subject: [PATCH 12/14] Remove unused variable in stdio_at_fork_reinit --- Python/sysmodule.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 0aa31cf8a847f3..e406aa56979ae9 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2872,7 +2872,6 @@ stdio_at_fork_reinit(_Py_Identifier *key) PyObject *isatty = NULL; PyObject *buffer = NULL; - PyObject *result = NULL; PyObject *stdio = _PySys_GetObjectId(key); @@ -2900,7 +2899,6 @@ stdio_at_fork_reinit(_Py_Identifier *key) end: Py_XDECREF(isatty); Py_XDECREF(buffer); - Py_XDECREF(result); return ret; } From 2054bfad3c5688341a28254834d93f8b9785e510 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Tue, 8 Feb 2022 09:02:46 -0800 Subject: [PATCH 13/14] Correct buffer->lock reinit logic. This commit: * Properly identifies whether we need to reinit a stream's lock. * Changes 'stdio' to 'stream' in several functions for clarity. * Also reinitialize stdin's buffer->lock at fork. --- Modules/_io/bufferedio.c | 8 +++---- Python/sysmodule.c | 52 ++++++++++++++++------------------------ 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 0ca22209f65b0d..0ee81ad35a11cc 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -432,15 +432,13 @@ buffered_clear(buffered *self) int _PyIO_buffered_at_fork_reinit(PyObject *self) { - if (!Py_IS_TYPE(self, &PyBufferedWriter_Type)) { + if (!Py_IS_TYPE(self, &PyBufferedWriter_Type) && + !Py_IS_TYPE(self, &PyBufferedReader_Type)) { return 0; } - Py_INCREF(self); buffered *buffer = (buffered *)self; - int ret = _PyThread_at_fork_reinit(&buffer->lock); buffer->owner = 0; - Py_DECREF(self); - return ret; + return _PyThread_at_fork_reinit(&buffer->lock); } #endif diff --git a/Python/sysmodule.c b/Python/sysmodule.c index e406aa56979ae9..dd16909b719bd6 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -62,6 +62,7 @@ _Py_IDENTIFIER(buffer); _Py_IDENTIFIER(builtins); _Py_IDENTIFIER(encoding); _Py_IDENTIFIER(path); +_Py_IDENTIFIER(stdin); _Py_IDENTIFIER(stdout); _Py_IDENTIFIER(stderr); _Py_IDENTIFIER(warnoptions); @@ -2865,40 +2866,26 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict) extern int _PyIO_buffered_at_fork_reinit(PyObject *); static int -stdio_at_fork_reinit(_Py_Identifier *key) +stream_at_fork_reinit(_Py_Identifier *stream_id) { + PyObject *stream = _PySys_GetObjectId(stream_id); - int ret = 0; - - PyObject *isatty = NULL; - PyObject *buffer = NULL; - - PyObject *stdio = _PySys_GetObjectId(key); - - _Py_IDENTIFIER(isatty); - isatty = _PyObject_CallMethodIdNoArgs(stdio, &PyId_isatty); - if (isatty == NULL) { - PyErr_Clear(); - goto end; - } - if (Py_IsFalse(isatty)) { - goto end; + if (stream == NULL || Py_IsNone(stream)) { + return 0; } - _Py_IDENTIFIER(buffer); - if (_PyObject_LookupAttrId(stdio, &PyId_buffer, &buffer) < 0) { - /* stdout.buffer and stderr.buffer are not part of the - * TextIOBase API and may not exist in some implementations. - * If not present, no need to reinitialize their locks. */ - goto end; + /* The buffer attribute is not part of the TextIOBase API + * and may not exist in some implementations. If not present, + * we have no locks to reinitialize. */ + PyObject *buffer = _PyObject_GetAttrId(stream, &PyId_buffer); + if (buffer == NULL) { + PyErr_Clear(); + return 0; } - /* reinitialize buffer->lock */ - ret = _PyIO_buffered_at_fork_reinit(buffer); - -end: - Py_XDECREF(isatty); - Py_XDECREF(buffer); + /* Reinitialize buffer->lock */ + int ret = _PyIO_buffered_at_fork_reinit(buffer); + Py_DECREF(buffer); return ret; } @@ -2907,11 +2894,12 @@ stdio_at_fork_reinit(_Py_Identifier *key) PyStatus _PySys_ReInitStdio(void) { - int reinit_stdout = stdio_at_fork_reinit(&PyId_stdout); - int reinit_stderr = stdio_at_fork_reinit(&PyId_stderr); + int reinit_stdin = stream_at_fork_reinit(&PyId_stdin); + int reinit_stdout = stream_at_fork_reinit(&PyId_stdout); + int reinit_stderr = stream_at_fork_reinit(&PyId_stderr); - if (reinit_stdout < 0 || reinit_stderr < 0) { - return _PyStatus_ERR("Failed to reinitialize stdout and stderr"); + if (reinit_stdin < 0 || reinit_stdout < 0 || reinit_stderr < 0) { + return _PyStatus_ERR("Failed to reinitialize standard streams"); } return _PyStatus_OK(); } From 56a405f8bcc5ba74129f61d42ddad9b798df21c9 Mon Sep 17 00:00:00 2001 From: Jason Wilkes Date: Wed, 16 Feb 2022 17:08:04 -0800 Subject: [PATCH 14/14] stream_at_fork_reinit now uses _Py_ID instead of _Py_Identifier. Update for consistency with gh-30928 (bpo-46541). --- Python/sysmodule.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 0ebae0958ec20e..aa0d2d5f03877b 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2899,10 +2899,8 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict) extern int _PyIO_buffered_at_fork_reinit(PyObject *); static int -stream_at_fork_reinit(_Py_Identifier *stream_id) +stream_at_fork_reinit(PyObject *stream) { - PyObject *stream = _PySys_GetObjectId(stream_id); - if (stream == NULL || Py_IsNone(stream)) { return 0; } @@ -2910,7 +2908,7 @@ stream_at_fork_reinit(_Py_Identifier *stream_id) /* The buffer attribute is not part of the TextIOBase API * and may not exist in some implementations. If not present, * we have no locks to reinitialize. */ - PyObject *buffer = _PyObject_GetAttrId(stream, &PyId_buffer); + PyObject *buffer = PyObject_GetAttr(stream, &_Py_ID(buffer)); if (buffer == NULL) { PyErr_Clear(); return 0; @@ -2927,9 +2925,15 @@ stream_at_fork_reinit(_Py_Identifier *stream_id) PyStatus _PySys_ReInitStdio(void) { - int reinit_stdin = stream_at_fork_reinit(&PyId_stdin); - int reinit_stdout = stream_at_fork_reinit(&PyId_stdout); - int reinit_stderr = stream_at_fork_reinit(&PyId_stderr); + PyThreadState *tstate = _PyThreadState_GET(); + + PyObject *sys_stdin = _PySys_GetAttr(tstate, &_Py_ID(stdin)); + PyObject *sys_stdout = _PySys_GetAttr(tstate, &_Py_ID(stdout)); + PyObject *sys_stderr = _PySys_GetAttr(tstate, &_Py_ID(stderr)); + + int reinit_stdin = stream_at_fork_reinit(sys_stdin); + int reinit_stdout = stream_at_fork_reinit(sys_stdout); + int reinit_stderr = stream_at_fork_reinit(sys_stderr); if (reinit_stdin < 0 || reinit_stdout < 0 || reinit_stderr < 0) { return _PyStatus_ERR("Failed to reinitialize standard streams");