From 262ed9fc8db7f08f359025288e0356f8fc9d9f77 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 18 May 2023 17:23:11 -0700 Subject: [PATCH 1/5] Only call PyOS_* hooks from the main interpreter --- Parser/myreadline.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 3f0e29f051a438..4db097459624e3 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -45,7 +45,10 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp) #endif while (1) { - if (PyOS_InputHook != NULL) { + if (PyOS_InputHook != NULL && + // See the comment for PyOS_ReadlineFunctionPointer below: + _Py_IsMainInterpreter(tstate->interp)) + { (void)(PyOS_InputHook)(); } @@ -131,7 +134,10 @@ _PyOS_WindowsConsoleReadline(PyThreadState *tstate, HANDLE hStdIn) wbuf = wbuf_local; wbuflen = sizeof(wbuf_local) / sizeof(wbuf_local[0]) - 1; while (1) { - if (PyOS_InputHook != NULL) { + if (PyOS_InputHook != NULL && + // See the comment for PyOS_ReadlineFunctionPointer below: + _Py_IsMainInterpreter(tstate->interp)) + { (void)(PyOS_InputHook)(); } if (!ReadConsoleW(hStdIn, &wbuf[total_read], wbuflen - total_read, &n_read, NULL)) { @@ -389,11 +395,23 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) * a tty. This can happen, for example if python is run like * this: python -i < test1.py */ - if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout))) - rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt); - else - rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout, - prompt); + if (!isatty(fileno(sys_stdin)) || !isatty(fileno(sys_stdout)) || + // Don't call globally-registered callbacks like PyOS_InputHook or + // PyOS_ReadlineFunctionPointer from subinterpreters, since it seems + // like there's no good way for users (like readline and tkinter) to + // avoid using global state to manage them. Plus, we generally don't + // want to cause trouble for libraries that don't know/care about + // subinterpreter support. If libraries really need better APIs that + // work per-interpreter and have ways to access module state, we can + // certainly add them later (but for now we'll cross our fingers and + // hope that nobody actually cares): + !_Py_IsMainInterpreter(tstate->interp)) + { + rv = PyOS_StdioReadline(sys_stdin, sys_stdout, prompt); + } + else { + rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout, prompt); + } Py_END_ALLOW_THREADS PyThread_release_lock(_PyOS_ReadlineLock); From 83d92ae9258da6fa3452bfa04de874790d120294 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 18 May 2023 17:23:20 -0700 Subject: [PATCH 2/5] Document the change --- Doc/c-api/veryhigh.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Doc/c-api/veryhigh.rst b/Doc/c-api/veryhigh.rst index 513856d8a48d70..000a2d3d8790bb 100644 --- a/Doc/c-api/veryhigh.rst +++ b/Doc/c-api/veryhigh.rst @@ -167,6 +167,10 @@ the same library that the Python runtime is using. event loops, as done in the :file:`Modules/_tkinter.c` in the Python source code. + .. versionchanged:: 3.12 + This function is only called from the + :ref:`main interpreter `. + .. c:var:: char* (*PyOS_ReadlineFunctionPointer)(FILE *, FILE *, const char *) @@ -187,6 +191,10 @@ the same library that the Python runtime is using. :c:func:`PyMem_RawRealloc`, instead of being allocated by :c:func:`PyMem_Malloc` or :c:func:`PyMem_Realloc`. + .. versionchanged:: 3.12 + This function is only called from the + :ref:`main interpreter `. + .. c:function:: PyObject* PyRun_String(const char *str, int start, PyObject *globals, PyObject *locals) This is a simplified interface to :c:func:`PyRun_StringFlags` below, leaving From 7fba174fce8560d3ce8b7e7b9df9ea1b83d7bf87 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 19 May 2023 10:22:49 -0700 Subject: [PATCH 3/5] blurb add --- .../C API/2023-05-19-10-22-34.gh-issue-104668.MLX1g9.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2023-05-19-10-22-34.gh-issue-104668.MLX1g9.rst diff --git a/Misc/NEWS.d/next/C API/2023-05-19-10-22-34.gh-issue-104668.MLX1g9.rst b/Misc/NEWS.d/next/C API/2023-05-19-10-22-34.gh-issue-104668.MLX1g9.rst new file mode 100644 index 00000000000000..7b882afd7f81a0 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-05-19-10-22-34.gh-issue-104668.MLX1g9.rst @@ -0,0 +1,5 @@ +Don't call :c:var:`PyOS_InputHook` or :c:var:`PyOS_ReadlineFunctionPointer` +in subinterpreters, since it's generally difficult to avoid using global +state in their registered callbacks. This also avoids situations where +extensions may find themselves running in a subinterpreter they don't +support (or haven't yet been loaded in). From 9800546d4362e508a32851bb2b5749a47675e884 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 19 May 2023 10:23:03 -0700 Subject: [PATCH 4/5] Add issue numbers --- Parser/myreadline.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 4db097459624e3..7074aba74b728c 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -46,7 +46,7 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp) while (1) { if (PyOS_InputHook != NULL && - // See the comment for PyOS_ReadlineFunctionPointer below: + // GH-104668: See PyOS_ReadlineFunctionPointer's comment below... _Py_IsMainInterpreter(tstate->interp)) { (void)(PyOS_InputHook)(); @@ -135,7 +135,7 @@ _PyOS_WindowsConsoleReadline(PyThreadState *tstate, HANDLE hStdIn) wbuflen = sizeof(wbuf_local) / sizeof(wbuf_local[0]) - 1; while (1) { if (PyOS_InputHook != NULL && - // See the comment for PyOS_ReadlineFunctionPointer below: + // GH-104668: See PyOS_ReadlineFunctionPointer's comment below... _Py_IsMainInterpreter(tstate->interp)) { (void)(PyOS_InputHook)(); @@ -396,7 +396,7 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) * this: python -i < test1.py */ if (!isatty(fileno(sys_stdin)) || !isatty(fileno(sys_stdout)) || - // Don't call globally-registered callbacks like PyOS_InputHook or + // GH-104668: Don't call global callbacks like PyOS_InputHook or // PyOS_ReadlineFunctionPointer from subinterpreters, since it seems // like there's no good way for users (like readline and tkinter) to // avoid using global state to manage them. Plus, we generally don't From b42f86a3a9539d86918b8777ad8a7a6dd02c7dc3 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 22 May 2023 12:08:06 -0700 Subject: [PATCH 5/5] What's New --- Doc/whatsnew/3.12.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index efbd2ca3de122a..4ff90664bb790b 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -1476,6 +1476,15 @@ Porting to Python 3.12 Note that :c:func:`PyType_FromMetaclass` (added in Python 3.12) already disallows creating classes whose metaclass overrides ``tp_new``. +* :c:var:`PyOS_InputHook` and :c:var:`PyOS_ReadlineFunctionPointer` are no + longer called in :ref:`subinterpreters `. This is + because clients generally rely on process-wide global state (since these + callbacks have no way of recovering extension module state). + + This also avoids situations where extensions may find themselves running in a + subinterpreter that they don't support (or haven't yet been loaded in). See + :gh:`104668` for more info. + Deprecated ----------