From 14791690daf9763a19f70b50e53d370d40cdf9fe Mon Sep 17 00:00:00 2001 From: Adarsh Mishra <122873385+Adarsh-Me@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:08:53 +0530 Subject: [PATCH 1/2] gh-157841: Synchronize access to functools.partial's mutable state partialobject's fn, args, kw, phcount, dict and vectorcall fields were read without synchronization by partial_repr(), partial_vectorcall(), partial_call(), partial_reduce() and partial_new() while partial_setstate() republished them, so a reader could observe a mixture of two states, or index a tuple that a concurrent __setstate__() had just dropped the last reference to. Every write now happens under the object's critical section, with the displaced values released after it because their finalizers can run arbitrary code. Each reader takes one coherent snapshot of the mutable fields as owned references and works from that. partial_new()'s flatten path takes owned references from the inner partial under that partial's lock, which is how gh-157099 reaches the same crash without any shared mutation of the object being built. --- ...-09-21-17-35-00.gh-issue-157841.Kp9zLm.rst | 7 + Modules/_functoolsmodule.c | 167 +++++++++++++----- 2 files changed, 126 insertions(+), 48 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-09-21-17-35-00.gh-issue-157841.Kp9zLm.rst diff --git a/Misc/NEWS.d/next/Library/2026-09-21-17-35-00.gh-issue-157841.Kp9zLm.rst b/Misc/NEWS.d/next/Library/2026-09-21-17-35-00.gh-issue-157841.Kp9zLm.rst new file mode 100644 index 00000000000000..ac98cbad7882bf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-09-21-17-35-00.gh-issue-157841.Kp9zLm.rst @@ -0,0 +1,7 @@ +Fix a data race in :class:`functools.partial` in free-threaded builds. +Mutating a partial shared between threads with ``__setstate__()``, as +:func:`copy.copy` and :mod:`pickle` do, could crash the interpreter when +another thread read it at the same time, and a reader could combine the +arguments of one state with the keywords of another. Readers of +``repr()``, ``partial()``, ``__reduce__()`` and calls now take all four +mutable fields as one snapshot under the object's critical section. diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 1ab230218124a4..79a08547221537 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -163,6 +163,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) PyObject *func, *pto_args, *new_args, *pto_kw, *phold; partialobject *pto; Py_ssize_t pto_phcount = 0; + Py_ssize_t phcount = 0; Py_ssize_t new_nargs = PyTuple_GET_SIZE(args) - 1; if (new_nargs < 0) { @@ -204,6 +205,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) } /* check wrapped function / object */ + PyObject *inner_func = NULL; pto_args = pto_kw = NULL; int res = PyObject_TypeCheck(func, state->partial_type); if (res == -1) { @@ -212,20 +214,28 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) if (res == 1) { // We can use its underlying function directly and merge the arguments. partialobject *part = (partialobject *)func; + /* Take owned references under the inner partial's lock. Its fields + * are mutable through __setstate__(), which can otherwise drop the + * last reference to args or kw while we are still reading them. */ + Py_BEGIN_CRITICAL_SECTION(part); if (part->dict == NULL) { - pto_args = part->args; - pto_kw = part->kw; - func = part->fn; + pto_args = Py_NewRef(part->args); + pto_kw = Py_NewRef(part->kw); + inner_func = Py_NewRef(part->fn); pto_phcount = part->phcount; assert(PyTuple_Check(pto_args)); assert(PyDict_Check(pto_kw)); } + Py_END_CRITICAL_SECTION(); + if (inner_func != NULL) { + func = inner_func; + } } /* create partialobject structure */ pto = (partialobject *)type->tp_alloc(type, 0); if (pto == NULL) - return NULL; + goto error; pto->fn = Py_NewRef(func); pto->placeholder = phold; @@ -233,11 +243,10 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) new_args = PyTuple_GetSlice(args, 1, new_nargs + 1); if (new_args == NULL) { Py_DECREF(pto); - return NULL; + goto error; } /* Count placeholders */ - Py_ssize_t phcount = 0; for (Py_ssize_t i = 0; i < new_nargs - 1; i++) { if (PyTuple_GET_ITEM(new_args, i) == phold) { phcount++; @@ -255,7 +264,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) if (tot_args == NULL) { Py_DECREF(new_args); Py_DECREF(pto); - return NULL; + goto error; } for (Py_ssize_t i = 0, j = 0; i < tot_nargs; i++) { if (i < npargs) { @@ -287,7 +296,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) Py_DECREF(new_args); if (pto->args == NULL) { Py_DECREF(pto); - return NULL; + goto error; } assert(PyTuple_Check(pto->args)); } @@ -308,17 +317,26 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) if (kw != NULL && pto->kw != NULL) { if (PyDict_Merge(pto->kw, kw, 1) != 0) { Py_DECREF(pto); - return NULL; + goto error; } } } if (pto->kw == NULL) { Py_DECREF(pto); - return NULL; + goto error; } partial_setvectorcall(pto); + Py_XDECREF(pto_args); + Py_XDECREF(pto_kw); + Py_XDECREF(inner_func); return (PyObject *)pto; + +error: + Py_XDECREF(pto_args); + Py_XDECREF(pto_kw); + Py_XDECREF(inner_func); + return NULL; } static int @@ -373,20 +391,30 @@ partial_vectorcall(PyObject *self, PyObject *const *args, PyThreadState *tstate = _PyThreadState_GET(); Py_ssize_t nargs = PyVectorcall_NARGS(nargsf); + PyObject *result = NULL; + PyObject *partial_function, *partial_args, *partial_keywords; + Py_ssize_t pto_phcount; + + /* One coherent snapshot: phcount must not be sampled apart from args, + * because tot_nargs below combines them. */ + Py_BEGIN_CRITICAL_SECTION(pto); + partial_function = Py_NewRef(pto->fn); + partial_args = Py_NewRef(pto->args); + partial_keywords = Py_NewRef(pto->kw); + pto_phcount = pto->phcount; + Py_END_CRITICAL_SECTION(); + /* Placeholder check */ - Py_ssize_t pto_phcount = pto->phcount; if (nargs < pto_phcount) { PyErr_Format(PyExc_TypeError, "missing positional arguments in 'partial' call; " "expected at least %zd, got %zd", pto_phcount, nargs); + Py_DECREF(partial_function); + Py_DECREF(partial_args); + Py_DECREF(partial_keywords); return NULL; } - PyObject *result = NULL; - PyObject *partial_function = Py_NewRef(pto->fn); - PyObject *partial_args = Py_NewRef(pto->args); - PyObject *partial_keywords = Py_NewRef(pto->kw); - PyObject **pto_args = _PyTuple_ITEMS(partial_args); Py_ssize_t pto_nargs = PyTuple_GET_SIZE(partial_args); Py_ssize_t pto_nkwds = PyDict_GET_SIZE(partial_keywords); @@ -592,22 +620,32 @@ static PyObject * partial_call(PyObject *self, PyObject *args, PyObject *kwargs) { partialobject *pto = partialobject_CAST(self); - assert(PyCallable_Check(pto->fn)); - assert(PyTuple_Check(pto->args)); - assert(PyDict_Check(pto->kw)); + PyObject *pto_fn, *pto_args, *pto_kw; + PyObject *tot_kw = NULL, *tot_args = NULL; + PyObject *res = NULL; + Py_ssize_t pto_phcount; + + Py_BEGIN_CRITICAL_SECTION(pto); + pto_fn = Py_NewRef(pto->fn); + pto_args = Py_NewRef(pto->args); + pto_kw = Py_NewRef(pto->kw); + pto_phcount = pto->phcount; + Py_END_CRITICAL_SECTION(); + + assert(PyCallable_Check(pto_fn)); + assert(PyTuple_Check(pto_args)); + assert(PyDict_Check(pto_kw)); Py_ssize_t nargs = PyTuple_GET_SIZE(args); - Py_ssize_t pto_phcount = pto->phcount; if (nargs < pto_phcount) { PyErr_Format(PyExc_TypeError, "missing positional arguments in 'partial' call; " "expected at least %zd, got %zd", pto_phcount, nargs); - return NULL; + goto done; } /* Merge keywords */ - PyObject *tot_kw; - if (PyDict_GET_SIZE(pto->kw) == 0) { + if (PyDict_GET_SIZE(pto_kw) == 0) { /* kwargs can be NULL */ tot_kw = Py_XNewRef(kwargs); } @@ -615,31 +653,27 @@ partial_call(PyObject *self, PyObject *args, PyObject *kwargs) /* bpo-27840, bpo-29318: dictionary of keyword parameters must be copied, because a function using "**kwargs" can modify the dictionary. */ - tot_kw = PyDict_Copy(pto->kw); + tot_kw = PyDict_Copy(pto_kw); if (tot_kw == NULL) { - return NULL; + goto done; } if (kwargs != NULL) { if (PyDict_Merge(tot_kw, kwargs, 1) != 0) { - Py_DECREF(tot_kw); - return NULL; + goto done; } } } /* Merge positional arguments */ - PyObject *tot_args; if (pto_phcount) { - Py_ssize_t pto_nargs = PyTuple_GET_SIZE(pto->args); + Py_ssize_t pto_nargs = PyTuple_GET_SIZE(pto_args); Py_ssize_t tot_nargs = pto_nargs + nargs - pto_phcount; assert(tot_nargs >= 0); tot_args = PyTuple_New(tot_nargs); if (tot_args == NULL) { - Py_XDECREF(tot_kw); - return NULL; + goto done; } - PyObject *pto_args = pto->args; PyObject *item; Py_ssize_t j = 0; // New args index for (Py_ssize_t i = 0; i < pto_nargs; i++) { @@ -661,16 +695,20 @@ partial_call(PyObject *self, PyObject *args, PyObject *kwargs) } else { /* Note: tupleconcat() is optimized for empty tuples */ - tot_args = PySequence_Concat(pto->args, args); + tot_args = PySequence_Concat(pto_args, args); if (tot_args == NULL) { - Py_XDECREF(tot_kw); - return NULL; + goto done; } } - PyObject *res = PyObject_Call(pto->fn, tot_args, tot_kw); - Py_DECREF(tot_args); + res = PyObject_Call(pto_fn, tot_args, tot_kw); + +done: + Py_XDECREF(tot_args); Py_XDECREF(tot_kw); + Py_DECREF(pto_fn); + Py_DECREF(pto_args); + Py_DECREF(pto_kw); return res; } @@ -720,10 +758,14 @@ partial_repr(PyObject *self) } return PyUnicode_FromString("..."); } - /* Reference arguments in case they change */ - PyObject *fn = Py_NewRef(pto->fn); - PyObject *args = Py_NewRef(pto->args); - PyObject *kw = Py_NewRef(pto->kw); + /* Hold the per-object lock while taking these: a concurrent + * __setstate__() may otherwise drop the last reference mid-acquisition. */ + PyObject *fn, *args, *kw; + Py_BEGIN_CRITICAL_SECTION(pto); + fn = Py_NewRef(pto->fn); + args = Py_NewRef(pto->args); + kw = Py_NewRef(pto->kw); + Py_END_CRITICAL_SECTION(); assert(PyTuple_Check(args)); assert(PyDict_Check(kw)); @@ -792,9 +834,23 @@ static PyObject * partial_reduce(PyObject *self, PyObject *Py_UNUSED(args)) { partialobject *pto = partialobject_CAST(self); - return Py_BuildValue("O(O)(OOOO)", Py_TYPE(pto), pto->fn, pto->fn, - pto->args, pto->kw, - pto->dict ? pto->dict : Py_None); + PyObject *fn, *args, *kw, *dict; + PyObject *result; + + Py_BEGIN_CRITICAL_SECTION(pto); + fn = Py_NewRef(pto->fn); + args = Py_NewRef(pto->args); + kw = Py_NewRef(pto->kw); + dict = Py_XNewRef(pto->dict); + Py_END_CRITICAL_SECTION(); + + result = Py_BuildValue("O(O)(OOOO)", Py_TYPE(pto), fn, fn, + args, kw, dict ? dict : Py_None); + Py_DECREF(fn); + Py_DECREF(args); + Py_DECREF(kw); + Py_XDECREF(dict); + return result; } static PyObject * @@ -853,12 +909,27 @@ partial_setstate(PyObject *self, PyObject *state) dict = NULL; else Py_INCREF(dict); - Py_SETREF(pto->fn, Py_NewRef(fn)); - Py_SETREF(pto->args, fnargs); - Py_SETREF(pto->kw, kw); + + PyObject *old_fn, *old_args, *old_kw, *old_dict; + Py_BEGIN_CRITICAL_SECTION(pto); + old_fn = pto->fn; + old_args = pto->args; + old_kw = pto->kw; + old_dict = pto->dict; + pto->fn = Py_NewRef(fn); + pto->args = fnargs; + pto->kw = kw; pto->phcount = phcount; - Py_XSETREF(pto->dict, dict); + pto->dict = dict; partial_setvectorcall(pto); + Py_END_CRITICAL_SECTION(); + + // The old values may have arbitrary finalizers, so they must be dropped + // after the critical section is released. + Py_XDECREF(old_fn); + Py_XDECREF(old_args); + Py_XDECREF(old_kw); + Py_XDECREF(old_dict); Py_RETURN_NONE; } From 51351e7e416f5402c8720807e7ee293f7e2cbb68 Mon Sep 17 00:00:00 2001 From: Adarsh Mishra <122873385+Adarsh-Me@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:09:16 +0530 Subject: [PATCH 2/2] gh-157841: Test concurrent functools.partial mutation Race __setstate__() against repr(), a call through both tp_call and vectorcall, __reduce__() and a nested partial(), and assert that a reader never combines the positional arguments of one state with the keywords of another. Every state the test installs carries the same value twice, once reachable as a positional argument and once as a keyword, so a torn read fails as a mismatch instead of only as a crash. test_concurrent_setstate_writers passes without the fix; it is there to guard the ordering of the displaced-value decrefs, not to reproduce a failure. --- .../test_free_threading/test_functools.py | 126 +++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_free_threading/test_functools.py b/Lib/test/test_free_threading/test_functools.py index a442fe056cef15..0c7000d9872a9e 100644 --- a/Lib/test/test_free_threading/test_functools.py +++ b/Lib/test/test_free_threading/test_functools.py @@ -1,7 +1,8 @@ import random +import sys import unittest -from functools import lru_cache +from functools import Placeholder, lru_cache, partial from threading import Barrier, Thread from test.support import threading_helper @@ -71,5 +72,128 @@ def test_reentrant_cache_clear_bounded(self): self._test_reentrant_cache_clear(maxsize=128) +class Callable: + # Instances have no vectorcall slot, so partial() dispatches them through + # tp_call (partial_call) instead of partial_vectorcall. + def __call__(self, x, y=0, *extra, z=0): + return (x, y, z) + + +def target(x, y=0, *extra, z=0): + return (x, y, z) + + +@threading_helper.requires_working_threading() +class TestPartial(unittest.TestCase): + # gh-157841: __setstate__() publishes func, args, keywords and the + # placeholder count as four independent stores with no lock, so a reader + # can observe a mixture of two states. Every state written below carries + # the same value twice -- once reachable as a positional argument, once as + # a keyword -- so a torn read shows up as a mismatch and not only as a + # crash. + + @staticmethod + def state(i): + if i % 2: + # one placeholder, so the caller's argument supplies x + return (target, (Placeholder, i), {"z": i}, None) + return (target, (i, i), {"z": i}, None) + + def check_coherent(self, p): + x, y, z = p(0) + self.assertEqual(y, z, "torn read: mixed the state of two setstate()s") + + def check_reduce_coherent(self, p): + (_, (func,), (func2, args, keywords, dict_)) = p.__reduce__() + self.assertIs(func, func2) + self.assertEqual(args[-1], keywords["z"], + "torn read: mixed the state of two setstate()s") + + def check_flatten_coherent(self, p): + # partial_new() reaches inside p to flatten it, so it reads func, args, + # keywords and the placeholder count out of a live, mutable object. + x, y, z = partial(p, 3)() + self.assertEqual(y, z, "torn read: mixed the state of two setstate()s") + + def _test_racing_setstate(self, p, state, readers): + num_threads = 10 + ops_per_thread = 1000 + old_interval = sys.getswitchinterval() + sys.setswitchinterval(1e-6) + self.addCleanup(sys.setswitchinterval, old_interval) + + b = Barrier(num_threads) + errors = [] + + def thread_func(n): + b.wait() + for j in range(ops_per_thread): + i = j * num_threads + n + try: + if i % 5 == 0: + p.__setstate__(state(i)) + else: + readers[i % len(readers)](p) + except Exception as exc: + errors.append(f"{type(exc).__name__}: {exc}") + + threads = [Thread(target=thread_func, args=(n,)) + for n in range(num_threads)] + with threading_helper.start_threads(threads): + pass + + if errors: + self.fail(f"{len(errors)} failures, first: {errors[0]}") + p.__setstate__(state(-1)) + self.check_coherent(p) + self.check_reduce_coherent(p) + self.check_flatten_coherent(p) + + def test_concurrent_setstate_and_reads(self): + readers = [self.check_coherent, self.check_reduce_coherent, + self.check_flatten_coherent, repr] + # The starting state is itself coherent, so a reader that runs before + # the first __setstate__() is not a false failure. + self._test_racing_setstate(partial(target, 0, 0, z=0), self.state, + readers) + + def test_concurrent_setstate_and_reads_no_vectorcall(self): + wrapped = Callable() + + def state(i): + func, args, keywords, dict_ = self.state(i) + return (wrapped, args, keywords, dict_) + + readers = [lambda p: p(0), self.check_reduce_coherent, + self.check_flatten_coherent, repr] + self._test_racing_setstate(partial(wrapped, 0, 0, z=0), state, readers) + + def test_concurrent_setstate_writers(self): + # Concurrent writers must not drop a displaced value's reference twice. + num_threads = 10 + ops_per_thread = 1000 + b = Barrier(num_threads) + p = partial(target, 0, 0, z=0) + errors = [] + + def thread_func(n): + b.wait() + for j in range(ops_per_thread): + try: + p.__setstate__(self.state(j * num_threads + n)) + except Exception as exc: + errors.append(f"{type(exc).__name__}: {exc}") + + threads = [Thread(target=thread_func, args=(n,)) + for n in range(num_threads)] + with threading_helper.start_threads(threads): + pass + + if errors: + self.fail(f"{len(errors)} failures, first: {errors[0]}") + self.check_coherent(p) + self.check_reduce_coherent(p) + + if __name__ == "__main__": unittest.main()