From 4e1d6df44c6f556bb54590d8d2a67fb4a656be56 Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Mon, 25 Sep 2023 01:12:43 +0300 Subject: [PATCH 1/6] _thread.start_new_thread: allocate/free thread bootstate using raw memory allocator When new thread is starting, but Python is finalizing, PyMem_Free can't be called on new thread bootstate, because this created thread doesn't own the GIL. We should use raw memory allocator to allocate/free bootstate to avoid crashes/other unexpected behaviour. --- Modules/_threadmodule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 9c915488f6e0de..3c3783bd3ea322 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1066,7 +1066,7 @@ thread_bootstate_free(struct bootstate *boot, int decref) Py_DECREF(boot->args); Py_XDECREF(boot->kwargs); } - PyMem_Free(boot); + PyMem_RawFree(boot); } @@ -1184,13 +1184,13 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } - struct bootstate *boot = PyMem_NEW(struct bootstate, 1); + struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate)); if (boot == NULL) { return PyErr_NoMemory(); } boot->tstate = _PyThreadState_New(interp); if (boot->tstate == NULL) { - PyMem_Free(boot); + PyMem_RawFree(boot); if (!PyErr_Occurred()) { return PyErr_NoMemory(); } From 96659986ca22c8fd5e9db5e10a2a4245ab6637c7 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 24 Sep 2023 22:26:59 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst new file mode 100644 index 00000000000000..159ceda6e8c706 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst @@ -0,0 +1 @@ +:func:`_thread.start_new_thread` now uses raw memory allocator to allocate new thread bootstate. This helps to avoid crashes in case when new thread gets started at interpreter finalization. From 9b32580c005b85e15aa0e04b0b978d5570bde4a8 Mon Sep 17 00:00:00 2001 From: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com> Date: Mon, 25 Sep 2023 01:27:43 +0300 Subject: [PATCH 3/6] Update 2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst fixed news entry --- .../2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst index 159ceda6e8c706..96eed013707e89 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst @@ -1 +1 @@ -:func:`_thread.start_new_thread` now uses raw memory allocator to allocate new thread bootstate. This helps to avoid crashes in case when new thread gets started at interpreter finalization. +:func:`!_thread.start_new_thread` now uses raw memory allocator to allocate new thread bootstate. This helps to avoid crashes in case when new thread gets started at interpreter finalization. From e5c0f9281753c65d054907dc1834e4389b4ab60c Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Mon, 25 Sep 2023 12:43:48 +0300 Subject: [PATCH 4/6] added comment that refers the changes --- Modules/_threadmodule.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 3c3783bd3ea322..fe006e42315f6a 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1184,6 +1184,10 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } + // gh-109795: Use PyMem_RawMalloc() to allocate new thread bootstate. + // If new thread is started at Python finalization, + // i.e. thread_run() routine is invoked in newly created OS thread, + // it should be able to free bootstate without holding the GIL. struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate)); if (boot == NULL) { return PyErr_NoMemory(); From 0fdd943b0f37ad73b0ea6143063cae43191320a2 Mon Sep 17 00:00:00 2001 From: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:22:13 +0300 Subject: [PATCH 5/6] Update Modules/_threadmodule.c Co-authored-by: Victor Stinner --- Modules/_threadmodule.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index fe006e42315f6a..fa98df516b8b10 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1184,10 +1184,9 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } - // gh-109795: Use PyMem_RawMalloc() to allocate new thread bootstate. - // If new thread is started at Python finalization, - // i.e. thread_run() routine is invoked in newly created OS thread, - // it should be able to free bootstate without holding the GIL. + // gh-109795: Use PyMem_RawMalloc() instead of PyMem_Malloc(), + // because it should be possible to call thread_bootstate_free() + // without holding the GIL. struct bootstate *boot = PyMem_RawMalloc(sizeof(struct bootstate)); if (boot == NULL) { return PyErr_NoMemory(); From 1d9c95a7376c92d5f31c00ac02bcf30c746c6725 Mon Sep 17 00:00:00 2001 From: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:34:50 +0300 Subject: [PATCH 6/6] Delete Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst --- .../2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst deleted file mode 100644 index 96eed013707e89..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2023-09-24-22-26-56.gh-issue-109795.gI7KW6.rst +++ /dev/null @@ -1 +0,0 @@ -:func:`!_thread.start_new_thread` now uses raw memory allocator to allocate new thread bootstate. This helps to avoid crashes in case when new thread gets started at interpreter finalization.