diff options
-rw-r--r-- | Lib/test/test_threading_local.py | 16 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst | 1 | ||||
-rw-r--r-- | Modules/_testcapimodule.c | 41 | ||||
-rw-r--r-- | Modules/_threadmodule.c | 30 |
4 files changed, 75 insertions, 13 deletions
diff --git a/Lib/test/test_threading_local.py b/Lib/test/test_threading_local.py index 5ddb8c41c5f..8b24e3a28bf 100644 --- a/Lib/test/test_threading_local.py +++ b/Lib/test/test_threading_local.py @@ -193,6 +193,22 @@ class BaseLocalTest: self.assertIsNone(wr()) + def test_threading_local_clear_race(self): + # See https://github.com/python/cpython/issues/100892 + + try: + import _testcapi + except ImportError: + unittest.skip("requires _testcapi") + + _testcapi.call_in_temporary_c_thread(lambda: None, False) + + for _ in range(1000): + _ = threading.local() + + _testcapi.join_temporary_c_thread() + + class ThreadLocalTest(unittest.TestCase, BaseLocalTest): _local = _thread._local diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst new file mode 100644 index 00000000000..f2576becc2f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst @@ -0,0 +1 @@ +Fix race while iterating over thread states in clearing :class:`threading.local`. Patch by Kumar Aditya. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 90841270e47..361ce7b55a2 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4239,12 +4239,19 @@ temporary_c_thread(void *data) PyThread_exit_thread(); } +static test_c_thread_t test_c_thread; + static PyObject * -call_in_temporary_c_thread(PyObject *self, PyObject *callback) +call_in_temporary_c_thread(PyObject *self, PyObject *args) { PyObject *res = NULL; - test_c_thread_t test_c_thread; + PyObject *callback = NULL; long thread; + int wait = 1; + if (!PyArg_ParseTuple(args, "O|i", &callback, &wait)) + { + return NULL; + } test_c_thread.start_event = PyThread_allocate_lock(); test_c_thread.exit_event = PyThread_allocate_lock(); @@ -4271,6 +4278,10 @@ call_in_temporary_c_thread(PyObject *self, PyObject *callback) PyThread_acquire_lock(test_c_thread.start_event, 1); PyThread_release_lock(test_c_thread.start_event); + if (!wait) { + Py_RETURN_NONE; + } + Py_BEGIN_ALLOW_THREADS PyThread_acquire_lock(test_c_thread.exit_event, 1); PyThread_release_lock(test_c_thread.exit_event); @@ -4281,13 +4292,32 @@ call_in_temporary_c_thread(PyObject *self, PyObject *callback) exit: Py_CLEAR(test_c_thread.callback); - if (test_c_thread.start_event) + if (test_c_thread.start_event) { PyThread_free_lock(test_c_thread.start_event); - if (test_c_thread.exit_event) + test_c_thread.start_event = NULL; + } + if (test_c_thread.exit_event) { PyThread_free_lock(test_c_thread.exit_event); + test_c_thread.exit_event = NULL; + } return res; } +static PyObject * +join_temporary_c_thread(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + Py_BEGIN_ALLOW_THREADS + PyThread_acquire_lock(test_c_thread.exit_event, 1); + PyThread_release_lock(test_c_thread.exit_event); + Py_END_ALLOW_THREADS + Py_CLEAR(test_c_thread.callback); + PyThread_free_lock(test_c_thread.start_event); + test_c_thread.start_event = NULL; + PyThread_free_lock(test_c_thread.exit_event); + test_c_thread.exit_event = NULL; + Py_RETURN_NONE; +} + /* marshal */ static PyObject* @@ -5532,8 +5562,9 @@ static PyMethodDef TestMethods[] = { {"docstring_with_signature_with_defaults", (PyCFunction)test_with_docstring, METH_NOARGS, docstring_with_signature_with_defaults}, - {"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_O, + {"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_VARARGS, PyDoc_STR("set_error_class(error_class) -> None")}, + {"join_temporary_c_thread", join_temporary_c_thread, METH_NOARGS}, {"pymarshal_write_long_to_file", pymarshal_write_long_to_file, METH_VARARGS}, {"pymarshal_write_object_to_file", diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index a370352238d..1ff31a452ca 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -801,6 +801,11 @@ local_traverse(localobject *self, visitproc visit, void *arg) return 0; } +#define HEAD_LOCK(runtime) \ + PyThread_acquire_lock((runtime)->interpreters.mutex, WAIT_LOCK) +#define HEAD_UNLOCK(runtime) \ + PyThread_release_lock((runtime)->interpreters.mutex) + static int local_clear(localobject *self) { @@ -810,17 +815,26 @@ local_clear(localobject *self) Py_CLEAR(self->dummies); Py_CLEAR(self->wr_callback); /* Remove all strong references to dummies from the thread states */ - if (self->key - && (tstate = PyThreadState_Get()) - && tstate->interp) { - for(tstate = PyInterpreterState_ThreadHead(tstate->interp); - tstate; - tstate = PyThreadState_Next(tstate)) - if (tstate->dict && PyDict_GetItem(tstate->dict, self->key)) { - if (PyDict_DelItem(tstate->dict, self->key)) { + if (self->key) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyRuntimeState *runtime = &_PyRuntime; + HEAD_LOCK(runtime); + PyThreadState *tstate = PyInterpreterState_ThreadHead(interp); + HEAD_UNLOCK(runtime); + while (tstate) { + if (tstate->dict) { + PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None); + if (v != NULL) { + Py_DECREF(v); + } + else { PyErr_Clear(); } } + HEAD_LOCK(runtime); + tstate = PyThreadState_Next(tstate); + HEAD_UNLOCK(runtime); + } } return 0; } |