summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKumar Aditya <59607654+kumaraditya303@users.noreply.github.com>2023-01-11 21:02:02 +0530
committerGitHub <noreply@github.com>2023-01-11 21:02:02 +0530
commita3b65770a0c048c3feaf9c289deba9a8f0e34f72 (patch)
tree99514d1dacc93786ccd1baf8827eab155c79d1a8
parent5aa8b9e70c44862cf3f600bdc329a20790b67056 (diff)
downloadcpython-git-a3b65770a0c048c3feaf9c289deba9a8f0e34f72.tar.gz
[3.10] GH-100892: Fix race in clearing `threading.local` (GH-100922). (#100938)
(cherry picked from commit 762745a124cbc297cf2fe6f3ec9ca1840bb2e873) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
-rw-r--r--Lib/test/test_threading_local.py13
-rw-r--r--Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst1
-rw-r--r--Modules/_testcapimodule.c41
-rw-r--r--Modules/_threadmodule.c30
4 files changed, 70 insertions, 15 deletions
diff --git a/Lib/test/test_threading_local.py b/Lib/test/test_threading_local.py
index 13facb5133..f1ef30e8f1 100644
--- a/Lib/test/test_threading_local.py
+++ b/Lib/test/test_threading_local.py
@@ -3,6 +3,7 @@ import unittest
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
@@ -194,6 +195,18 @@ class BaseLocalTest:
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 0000000000..f2576becc2
--- /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 c0c86722eb..bafd0c4e69 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4368,12 +4368,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();
@@ -4400,6 +4407,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);
@@ -4410,13 +4421,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*
@@ -5878,8 +5908,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 813d7ec232..8ac75b9109 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -842,6 +842,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)
{
@@ -852,18 +857,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;