From c7cb73694edfa588e58b5904c71493dbd4728d1f Mon Sep 17 00:00:00 2001 From: "Yves.Duprat" Date: Tue, 19 May 2026 15:56:28 +0200 Subject: [PATCH 1/4] Initial commit --- Lib/test/test_thread.py | 42 +++++++++++++++++++++++++++++++++++++++++ Modules/_threadmodule.c | 15 +++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index ac924728febc99..3776371ee21b63 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -345,6 +345,48 @@ def func(): handle = thread.start_joinable_thread(func, handle=None) handle.join() +class StartNewThreadKwargsRace(unittest.TestCase): + + @unittest.skipUnless(support.Py_GIL_DISABLED, "GIL must be disabled") + def test_dict_growsup_when_thread_start(self): + # See gh-149816 - (62) Concurrent kwargs growth causes heap overwrite + # This test is meant to be run under a free-threaded build, where the GIL is + # disabled and concurrent mutations of the same dict can cause heap + # corruption. + results = [] + def mutator(shared, stop, prefix, burst): + i = 0 + while not stop.locked(): + for _ in range(burst): + shared[f"{prefix}_{i}"] = i + i += 1 + time.sleep(0) + results.append(prefix) + + def nop(i, **kwargs): + pass + + DELAY = 1.0 + stop = thread.lock() + shared = {f"base_{i}": i for i in range(20000)} + n = 4 + for i in range(n): + args=(shared, stop, f"dynamic_{i}", 1000) + thread.start_new_thread(mutator, args) + + snt = 32 + for i in range(snt): + try: + thread.start_new_thread(nop, (i,), shared) + except RuntimeError: + break + + stop.acquire() + # wait for all mutator threads stop. + wait_t = time.monotonic() + while len(results) < n and time.monotonic() - wait_t < DELAY: + time.sleep(0.01) + class Barrier: def __init__(self, num_threads): diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 135b53111014d1..c70c454e171868 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -385,6 +385,21 @@ thread_run(void *boot_raw) PyEval_AcquireThread(tstate); _Py_atomic_add_ssize(&tstate->interp->threads.count, 1); +#ifdef Py_GIL_DISABLED + // See gh-149816 - (62) Concurrent kwargs growth causes heap overwrite + // So duplicate boot->kwargs to ensure that it won't be mutated concurrently + // by the caller. + if (boot->kwargs != NULL) { + PyObject *n_kwargs = PyDict_Copy(boot->kwargs); + if (n_kwargs == NULL) { + thread_bootstate_free(boot, 1); + goto exit; + } + Py_DECREF(boot->kwargs); // I am not pretty sure about this. + boot->kwargs = n_kwargs; + } +#endif /* Py_GIL_DISABLED */ + PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); if (res == NULL) { if (PyErr_ExceptionMatches(PyExc_SystemExit)) From 086593c0cef5e1495fdf1c0670249fa196c9fbab Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 19:10:21 +0000 Subject: [PATCH 2/4] =?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 --- .../next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst diff --git a/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst b/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst new file mode 100644 index 00000000000000..5c4c174b22ce89 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst @@ -0,0 +1 @@ +Fix a race condition on `kwargs` in `PyStack_UnpackDict` by duplicating the `kwargs` argument in the `thread.new_start_thread` function. From e75a12bfa0c6b41af55b1f4c4451c6a56cb13076 Mon Sep 17 00:00:00 2001 From: "Yves.Duprat" Date: Wed, 20 May 2026 21:13:28 +0200 Subject: [PATCH 3/4] Fix nits in news file --- .../next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst b/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst index 5c4c174b22ce89..50dc74c8beeaca 100644 --- a/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst +++ b/Misc/NEWS.d/next/Library/2026-05-20-19-10-09.gh-issue-149816.g_ycIN.rst @@ -1 +1 @@ -Fix a race condition on `kwargs` in `PyStack_UnpackDict` by duplicating the `kwargs` argument in the `thread.new_start_thread` function. +Fix a race condition on ``kwargs`` in ``PyStack_UnpackDict`` by duplicating the ``kwargs`` argument in the ``thread.new_start_thread`` function. From 73c58fb96b9e4c400291a917e1e152791c4ccfce Mon Sep 17 00:00:00 2001 From: "Yves.Duprat" Date: Fri, 22 May 2026 16:29:58 +0200 Subject: [PATCH 4/4] Refactor test --- Lib/test/test_thread.py | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index 3776371ee21b63..1dc15efb240c15 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -347,6 +347,10 @@ def func(): class StartNewThreadKwargsRace(unittest.TestCase): + def setUp(self): + key = threading_helper.threading_setup() + self.addCleanup(threading_helper.threading_cleanup, *key) + @unittest.skipUnless(support.Py_GIL_DISABLED, "GIL must be disabled") def test_dict_growsup_when_thread_start(self): # See gh-149816 - (62) Concurrent kwargs growth causes heap overwrite @@ -364,28 +368,29 @@ def mutator(shared, stop, prefix, burst): results.append(prefix) def nop(i, **kwargs): - pass + results.append(i) - DELAY = 1.0 - stop = thread.lock() - shared = {f"base_{i}": i for i in range(20000)} - n = 4 - for i in range(n): - args=(shared, stop, f"dynamic_{i}", 1000) - thread.start_new_thread(mutator, args) + with threading_helper.wait_threads_exit(): + stop = thread.lock() + shared = {f"base_{i}": i for i in range(20000)} + n = 4 + for i in range(n): + args=(shared, stop, f"dynamic_{i}", 1000) + thread.start_new_thread(mutator, args) + + snt = 16 + for i in range(snt): + try: + thread.start_new_thread(nop, (i,), shared) + except RuntimeError: + break - snt = 32 - for i in range(snt): - try: - thread.start_new_thread(nop, (i,), shared) - except RuntimeError: - break - - stop.acquire() - # wait for all mutator threads stop. - wait_t = time.monotonic() - while len(results) < n and time.monotonic() - wait_t < DELAY: - time.sleep(0.01) + stop.acquire() + # wait for all mutator/nop threads stop. + for _ in support.sleeping_retry(support.SHORT_TIMEOUT): + if len(results) == n+snt: + break + self.assertTrue(True, "successful test") class Barrier: