diff --git a/Lib/test/test_threading_local.py b/Lib/test/test_threading_local.py index 9888e631881fa6..65d9fed7780b07 100644 --- a/Lib/test/test_threading_local.py +++ b/Lib/test/test_threading_local.py @@ -3,6 +3,7 @@ from doctest import DocTestSuite from test import support from test.support import threading_helper +from test.support.import_helper import import_module import weakref import gc @@ -197,6 +198,18 @@ class X: self.assertIsNone(wr()) + def test_threading_local_clear_race(self): + # See https://github.com/python/cpython/issues/100892 + + _testcapi = import_module('_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 00000000000000..f2576becc2fcfc --- /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 bb7de220a2db9a..dce948a8f0f155 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2746,6 +2746,7 @@ get_date_fromdate(PyObject *self, PyObject *args) return rv; } + static PyObject * get_datetime_fromdateandtime(PyObject *self, PyObject *args) { @@ -4526,12 +4527,19 @@ temporary_c_thread(void *data) PyThread_release_lock(test_c_thread->exit_event); } +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(); @@ -4541,8 +4549,7 @@ call_in_temporary_c_thread(PyObject *self, PyObject *callback) goto exit; } - Py_INCREF(callback); - test_c_thread.callback = callback; + test_c_thread.callback = Py_NewRef(callback); PyThread_acquire_lock(test_c_thread.start_event, 1); PyThread_acquire_lock(test_c_thread.exit_event, 1); @@ -4558,23 +4565,45 @@ 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); Py_END_ALLOW_THREADS - Py_INCREF(Py_None); - res = Py_None; + res = Py_NewRef(Py_None); 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* @@ -6418,8 +6447,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 4ac90dc8068934..879c94a109690d 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -839,6 +839,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) { @@ -849,18 +854,23 @@ local_clear(localobject *self) /* Remove all strong references to dummies from the thread states */ if (self->key) { PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyRuntimeState *runtime = &_PyRuntime; + HEAD_LOCK(runtime); PyThreadState *tstate = PyInterpreterState_ThreadHead(interp); - for(; tstate; tstate = PyThreadState_Next(tstate)) { - if (tstate->dict == NULL) { - continue; - } - PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None); - if (v != NULL) { - Py_DECREF(v); - } - else { - PyErr_Clear(); + 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;