Skip to content

Commit 8e41e91

Browse files
kumaraditya303mgorny
kumaraditya303
authored and
mgorny
committed
[3.9] pythonGH-100892: Fix race in clearing threading.local (pythonGH-100922) (python#100939)
[3.9] [3.10] pythonGH-100892: Fix race in clearing `threading.local` (pythonGH-100922). (cherry picked from commit 762745a) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>. (cherry picked from commit 683e9fe) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> [adjusted for 3.8 by Michał Górny]
1 parent 86ee08a commit 8e41e91

4 files changed

Lines changed: 75 additions & 13 deletions

File tree

‎Lib/test/test_threading_local.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,22 @@ class X:
193193
self.assertIsNone(wr())
194194

195195

196+
def test_threading_local_clear_race(self):
197+
# See https://github.com/python/cpython/issues/100892
198+
199+
try:
200+
import _testcapi
201+
except ImportError:
202+
unittest.skip("requires _testcapi")
203+
204+
_testcapi.call_in_temporary_c_thread(lambda: None, False)
205+
206+
for _ in range(1000):
207+
_ = threading.local()
208+
209+
_testcapi.join_temporary_c_thread()
210+
211+
196212
class ThreadLocalTest(unittest.TestCase, BaseLocalTest):
197213
_local = _thread._local
198214

‎Misc/NEWS.d/next/Core and Builtins/2023-01-10-14-11-17.gh-issue-100892.qfBVYI.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix race while iterating over thread states in clearing :class:`threading.local`. Patch by Kumar Aditya.

‎Modules/_testcapimodule.c‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4083,12 +4083,19 @@ temporary_c_thread(void *data)
40834083
PyThread_exit_thread();
40844084
}
40854085

4086+
static test_c_thread_t test_c_thread;
4087+
40864088
static PyObject *
4087-
call_in_temporary_c_thread(PyObject *self, PyObject *callback)
4089+
call_in_temporary_c_thread(PyObject *self, PyObject *args)
40884090
{
40894091
PyObject *res = NULL;
4090-
test_c_thread_t test_c_thread;
4092+
PyObject *callback = NULL;
40914093
long thread;
4094+
int wait = 1;
4095+
if (!PyArg_ParseTuple(args, "O|i", &callback, &wait))
4096+
{
4097+
return NULL;
4098+
}
40924099

40934100
PyEval_InitThreads();
40944101

@@ -4117,6 +4124,10 @@ call_in_temporary_c_thread(PyObject *self, PyObject *callback)
41174124
PyThread_acquire_lock(test_c_thread.start_event, 1);
41184125
PyThread_release_lock(test_c_thread.start_event);
41194126

4127+
if (!wait) {
4128+
Py_RETURN_NONE;
4129+
}
4130+
41204131
Py_BEGIN_ALLOW_THREADS
41214132
PyThread_acquire_lock(test_c_thread.exit_event, 1);
41224133
PyThread_release_lock(test_c_thread.exit_event);
@@ -4127,13 +4138,32 @@ call_in_temporary_c_thread(PyObject *self, PyObject *callback)
41274138

41284139
exit:
41294140
Py_CLEAR(test_c_thread.callback);
4130-
if (test_c_thread.start_event)
4141+
if (test_c_thread.start_event) {
41314142
PyThread_free_lock(test_c_thread.start_event);
4132-
if (test_c_thread.exit_event)
4143+
test_c_thread.start_event = NULL;
4144+
}
4145+
if (test_c_thread.exit_event) {
41334146
PyThread_free_lock(test_c_thread.exit_event);
4147+
test_c_thread.exit_event = NULL;
4148+
}
41344149
return res;
41354150
}
41364151

4152+
static PyObject *
4153+
join_temporary_c_thread(PyObject *self, PyObject *Py_UNUSED(ignored))
4154+
{
4155+
Py_BEGIN_ALLOW_THREADS
4156+
PyThread_acquire_lock(test_c_thread.exit_event, 1);
4157+
PyThread_release_lock(test_c_thread.exit_event);
4158+
Py_END_ALLOW_THREADS
4159+
Py_CLEAR(test_c_thread.callback);
4160+
PyThread_free_lock(test_c_thread.start_event);
4161+
test_c_thread.start_event = NULL;
4162+
PyThread_free_lock(test_c_thread.exit_event);
4163+
test_c_thread.exit_event = NULL;
4164+
Py_RETURN_NONE;
4165+
}
4166+
41374167
/* marshal */
41384168

41394169
static PyObject*
@@ -5281,8 +5311,9 @@ static PyMethodDef TestMethods[] = {
52815311
{"docstring_with_signature_with_defaults",
52825312
(PyCFunction)test_with_docstring, METH_NOARGS,
52835313
docstring_with_signature_with_defaults},
5284-
{"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_O,
5314+
{"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_VARARGS,
52855315
PyDoc_STR("set_error_class(error_class) -> None")},
5316+
{"join_temporary_c_thread", join_temporary_c_thread, METH_NOARGS},
52865317
{"pymarshal_write_long_to_file",
52875318
pymarshal_write_long_to_file, METH_VARARGS},
52885319
{"pymarshal_write_object_to_file",

‎Modules/_threadmodule.c‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,11 @@ local_traverse(localobject *self, visitproc visit, void *arg)
764764
return 0;
765765
}
766766

767+
#define HEAD_LOCK(runtime) \
768+
PyThread_acquire_lock((runtime)->interpreters.mutex, WAIT_LOCK)
769+
#define HEAD_UNLOCK(runtime) \
770+
PyThread_release_lock((runtime)->interpreters.mutex)
771+
767772
static int
768773
local_clear(localobject *self)
769774
{
@@ -773,17 +778,26 @@ local_clear(localobject *self)
773778
Py_CLEAR(self->dummies);
774779
Py_CLEAR(self->wr_callback);
775780
/* Remove all strong references to dummies from the thread states */
776-
if (self->key
777-
&& (tstate = PyThreadState_Get())
778-
&& tstate->interp) {
779-
for(tstate = PyInterpreterState_ThreadHead(tstate->interp);
780-
tstate;
781-
tstate = PyThreadState_Next(tstate))
782-
if (tstate->dict && PyDict_GetItem(tstate->dict, self->key)) {
783-
if (PyDict_DelItem(tstate->dict, self->key)) {
781+
if (self->key) {
782+
PyInterpreterState *interp = _PyInterpreterState_GET_UNSAFE();
783+
_PyRuntimeState *runtime = &_PyRuntime;
784+
HEAD_LOCK(runtime);
785+
PyThreadState *tstate = PyInterpreterState_ThreadHead(interp);
786+
HEAD_UNLOCK(runtime);
787+
while (tstate) {
788+
if (tstate->dict) {
789+
PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None);
790+
if (v != NULL) {
791+
Py_DECREF(v);
792+
}
793+
else {
784794
PyErr_Clear();
785795
}
786796
}
797+
HEAD_LOCK(runtime);
798+
tstate = PyThreadState_Next(tstate);
799+
HEAD_UNLOCK(runtime);
800+
}
787801
}
788802
return 0;
789803
}

0 commit comments

Comments
 (0)