Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

[3.13] gh-120973: Fix thread-safety issues with `threading.local` (GH-121655) (#122042) 

This is a small refactoring to the current design that allows us to
avoid manually iterating over threads.

This should also fix gh-118490.
(cherry picked from commit e059aa6b01)

Co-authored-by: mpage <mpage@meta.com>
This commit is contained in:
Miss Islington (bot) 2024-07-19 19:49:34 +02:00 committed by GitHub
parent 159ea47ed8
commit bf7f4f357b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 245 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Include/cpython/pystate.h
View File

@ -192,6 +192,14 @@ struct _ts {
PyObject *previous_executor;
uint64_t dict_global_version;
/* Used to store/retrieve `threading.local` keys/values for this thread */
PyObject *threading_local_key;
/* Used by `threading.local`s to be remove keys/values for dying threads.
The PyThreadObject must hold the only reference to this value.
*/
PyObject *threading_local_sentinel;
};
#ifdef Py_DEBUG

384
Modules/_threadmodule.c
View File

@ -1350,33 +1350,44 @@ newlockobject(PyObject *module)
Our implementation uses small "localdummy" objects in order to break
the reference chain. These trivial objects are hashable (using the
default scheme of identity hashing) and weakrefable.
Each thread-state holds a separate localdummy for each local object
(as a /strong reference/),
and each thread-local object holds a dict mapping /weak references/
of localdummies to local dicts.
Each thread-state holds two separate localdummy objects:
- `threading_local_key` is used as a key to retrieve the locals dictionary
for the thread in any `threading.local` object.
- `threading_local_sentinel` is used to signal when a thread is being
destroyed. Consequently, the associated thread-state must hold the only
reference.
Each `threading.local` object contains a dict mapping localdummy keys to
locals dicts and a set containing weak references to localdummy
sentinels. Each sentinel weak reference has a callback that removes itself
and the locals dict for the key from the `threading.local` object when
called.
Therefore:
- only the thread-state dict holds a strong reference to the dummies
- only the thread-local object holds a strong reference to the local dicts
- only outside objects (application- or library-level) hold strong
references to the thread-local objects
- as soon as a thread-state dict is destroyed, the weakref callbacks of all
dummies attached to that thread are called, and destroy the corresponding
local dicts from thread-local objects
- as soon as a thread-local object is destroyed, its local dicts are
destroyed and its dummies are manually removed from all thread states
- the GC can do its work correctly when a thread-local object is dangling,
without any interference from the thread-state dicts
- The thread-state only holds strong references to localdummy objects, which
cannot participate in cycles.
- Only outside objects (application- or library-level) hold strong
references to the thread-local objects.
- As soon as thread-state's sentinel dummy is destroyed the callbacks for
all weakrefs attached to the sentinel are called, and destroy the
corresponding local dicts from thread-local objects.
- As soon as a thread-local object is destroyed, its local dicts are
destroyed.
- The GC can do its work correctly when a thread-local object is dangling,
without any interference from the thread-state dicts.
As an additional optimization, each localdummy holds a borrowed reference
to the corresponding localdict. This borrowed reference is only used
by the thread-local object which has created the localdummy, which should
guarantee that the localdict still exists when accessed.
This dual key arrangement is necessary to ensure that `threading.local`
values can be retrieved from finalizers. If we were to only keep a mapping
of localdummy weakrefs to locals dicts it's possible that the weakrefs would
be cleared before finalizers were called (GC currently clears weakrefs that
are garbage before invoking finalizers), causing lookups in finalizers to
fail.
*/
typedef struct {
PyObject_HEAD
PyObject *localdict; /* Borrowed reference! */
PyObject *weakreflist; /* List of weak references to self */
} localdummyobject;
@ -1413,80 +1424,60 @@ static PyType_Spec local_dummy_type_spec = {
typedef struct {
PyObject_HEAD
PyObject *key;
PyObject *args;
PyObject *kw;
PyObject *weakreflist; /* List of weak references to self */
/* A {localdummy weakref -> localdict} dict */
PyObject *dummies;
/* The callback for weakrefs to localdummies */
PyObject *wr_callback;
/* A {localdummy -> localdict} dict */
PyObject *localdicts;
/* A set of weakrefs to thread sentinels localdummies*/
PyObject *thread_watchdogs;
} localobject;
/* Forward declaration */
static PyObject *_ldict(localobject *self, thread_module_state *state);
static PyObject *_localdummy_destroyed(PyObject *meth_self, PyObject *dummyweakref);
static int create_localsdict(localobject *self, thread_module_state *state,
PyObject **localsdict, PyObject **sentinel_wr);
static PyObject *clear_locals(PyObject *meth_self, PyObject *dummyweakref);
/* Create and register the dummy for the current thread.
Returns a borrowed reference of the corresponding local dict */
/* Create a weakref to the sentinel localdummy for the current thread */
static PyObject *
_local_create_dummy(localobject *self, thread_module_state *state)
create_sentinel_wr(localobject *self)
{
PyObject *ldict = NULL, *wr = NULL;
localdummyobject *dummy = NULL;
PyTypeObject *type = state->local_dummy_type;
static PyMethodDef wr_callback_def = {
"clear_locals", (PyCFunction) clear_locals, METH_O
};
PyObject *tdict = PyThreadState_GetDict();
if (tdict == NULL) {
PyErr_SetString(PyExc_SystemError,
"Couldn't get thread-state dictionary");
goto err;
PyThreadState *tstate = PyThreadState_Get();
/* We use a weak reference to self in the callback closure
in order to avoid spurious reference cycles */
PyObject *self_wr = PyWeakref_NewRef((PyObject *) self, NULL);
if (self_wr == NULL) {
return NULL;
}
ldict = PyDict_New();
if (ldict == NULL) {
goto err;
PyObject *args = PyTuple_New(2);
if (args == NULL) {
Py_DECREF(self_wr);
return NULL;
}
dummy = (localdummyobject *) type->tp_alloc(type, 0);
if (dummy == NULL) {
goto err;
}
dummy->localdict = ldict;
wr = PyWeakref_NewRef((PyObject *) dummy, self->wr_callback);
if (wr == NULL) {
goto err;
PyTuple_SET_ITEM(args, 0, self_wr);
PyTuple_SET_ITEM(args, 1, Py_NewRef(tstate->threading_local_key));
PyObject *cb = PyCFunction_New(&wr_callback_def, args);
Py_DECREF(args);
if (cb == NULL) {
return NULL;
}
/* As a side-effect, this will cache the weakref's hash before the
dummy gets deleted */
int r = PyDict_SetItem(self->dummies, wr, ldict);
if (r < 0) {
goto err;
}
Py_CLEAR(wr);
r = PyDict_SetItem(tdict, self->key, (PyObject *) dummy);
if (r < 0) {
goto err;
}
Py_CLEAR(dummy);
PyObject *wr = PyWeakref_NewRef(tstate->threading_local_sentinel, cb);
Py_DECREF(cb);
Py_DECREF(ldict);
return ldict;
err:
Py_XDECREF(ldict);
Py_XDECREF(wr);
Py_XDECREF(dummy);
return NULL;
return wr;
}
static PyObject *
local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
{
static PyMethodDef wr_callback_def = {
"_localdummy_destroyed", (PyCFunction) _localdummy_destroyed, METH_O
};
if (type->tp_init == PyBaseObject_Type.tp_init) {
int rc = 0;
if (args != NULL)
@ -1513,30 +1504,25 @@ local_new(PyTypeObject *type, PyObject *args, PyObject *kw)
self->args = Py_XNewRef(args);
self->kw = Py_XNewRef(kw);
self->key = PyUnicode_FromFormat("thread.local.%p", self);
if (self->key == NULL) {
self->localdicts = PyDict_New();
if (self->localdicts == NULL) {
goto err;
}
self->dummies = PyDict_New();
if (self->dummies == NULL) {
self->thread_watchdogs = PySet_New(NULL);
if (self->thread_watchdogs == NULL) {
goto err;
}
/* We use a weak reference to self in the callback closure
in order to avoid spurious reference cycles */
PyObject *wr = PyWeakref_NewRef((PyObject *) self, NULL);
if (wr == NULL) {
goto err;
}
self->wr_callback = PyCFunction_NewEx(&wr_callback_def, wr, NULL);
Py_DECREF(wr);
if (self->wr_callback == NULL) {
goto err;
}
if (_local_create_dummy(self, state) == NULL) {
PyObject *localsdict = NULL;
PyObject *sentinel_wr = NULL;
if (create_localsdict(self, state, &localsdict, &sentinel_wr) < 0) {
goto err;
}
Py_DECREF(localsdict);
Py_DECREF(sentinel_wr);
return (PyObject *)self;
err:
@ -1550,7 +1536,8 @@ local_traverse(localobject *self, visitproc visit, void *arg)
Py_VISIT(Py_TYPE(self));
Py_VISIT(self->args);
Py_VISIT(self->kw);
Py_VISIT(self->dummies);
Py_VISIT(self->localdicts);
Py_VISIT(self->thread_watchdogs);
return 0;
}
@ -1559,27 +1546,8 @@ local_clear(localobject *self)
{
Py_CLEAR(self->args);
Py_CLEAR(self->kw);
Py_CLEAR(self->dummies);
Py_CLEAR(self->wr_callback);
/* 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);
HEAD_UNLOCK(runtime);
while (tstate) {
if (tstate->dict) {
if (PyDict_Pop(tstate->dict, self->key, NULL) < 0) {
// Silently ignore error
PyErr_Clear();
}
}
HEAD_LOCK(runtime);
tstate = PyThreadState_Next(tstate);
HEAD_UNLOCK(runtime);
}
}
Py_CLEAR(self->localdicts);
Py_CLEAR(self->thread_watchdogs);
return 0;
}
@ -1595,48 +1563,142 @@ local_dealloc(localobject *self)
PyObject_GC_UnTrack(self);
local_clear(self);
Py_XDECREF(self->key);
PyTypeObject *tp = Py_TYPE(self);
tp->tp_free((PyObject*)self);
Py_DECREF(tp);
}
/* Returns a borrowed reference to the local dict, creating it if necessary */
/* Create the TLS key and sentinel if they don't exist */
static int
create_localdummies(thread_module_state *state)
{
PyThreadState *tstate = _PyThreadState_GET();
if (tstate->threading_local_key != NULL) {
return 0;
}
PyTypeObject *ld_type = state->local_dummy_type;
tstate->threading_local_key = ld_type->tp_alloc(ld_type, 0);
if (tstate->threading_local_key == NULL) {
return -1;
}
tstate->threading_local_sentinel = ld_type->tp_alloc(ld_type, 0);
if (tstate->threading_local_sentinel == NULL) {
Py_CLEAR(tstate->threading_local_key);
return -1;
}
return 0;
}
/* Insert a localsdict and sentinel weakref for the current thread, placing
strong references in localsdict and sentinel_wr, respectively.
*/
static int
create_localsdict(localobject *self, thread_module_state *state,
PyObject **localsdict, PyObject **sentinel_wr)
{
PyThreadState *tstate = _PyThreadState_GET();
PyObject *ldict = NULL;
PyObject *wr = NULL;
if (create_localdummies(state) < 0) {
goto err;
}
/* Create and insert the locals dict and sentinel weakref */
ldict = PyDict_New();
if (ldict == NULL) {
goto err;
}
if (PyDict_SetItem(self->localdicts, tstate->threading_local_key, ldict) <
0) {
goto err;
}
wr = create_sentinel_wr(self);
if (wr == NULL) {
PyObject *exc = PyErr_GetRaisedException();
if (PyDict_DelItem(self->localdicts, tstate->threading_local_key) <
0) {
PyErr_WriteUnraisable((PyObject *)self);
}
PyErr_SetRaisedException(exc);
goto err;
}
if (PySet_Add(self->thread_watchdogs, wr) < 0) {
PyObject *exc = PyErr_GetRaisedException();
if (PyDict_DelItem(self->localdicts, tstate->threading_local_key) <
0) {
PyErr_WriteUnraisable((PyObject *)self);
}
PyErr_SetRaisedException(exc);
goto err;
}
*localsdict = ldict;
*sentinel_wr = wr;
return 0;
err:
Py_XDECREF(ldict);
Py_XDECREF(wr);
return -1;
}
/* Return a strong reference to the locals dict for the current thread,
creating it if necessary.
*/
static PyObject *
_ldict(localobject *self, thread_module_state *state)
{
PyObject *tdict = PyThreadState_GetDict();
if (tdict == NULL) {
PyErr_SetString(PyExc_SystemError,
"Couldn't get thread-state dictionary");
if (create_localdummies(state) < 0) {
return NULL;
}
/* Check if a localsdict already exists */
PyObject *ldict;
PyObject *dummy = PyDict_GetItemWithError(tdict, self->key);
if (dummy == NULL) {
if (PyErr_Occurred()) {
return NULL;
}
ldict = _local_create_dummy(self, state);
if (ldict == NULL)
return NULL;
PyThreadState *tstate = _PyThreadState_GET();
if (PyDict_GetItemRef(self->localdicts, tstate->threading_local_key,
&ldict) < 0) {
return NULL;
}
if (ldict != NULL) {
return ldict;
}
if (Py_TYPE(self)->tp_init != PyBaseObject_Type.tp_init &&
Py_TYPE(self)->tp_init((PyObject*)self,
self->args, self->kw) < 0) {
/* we need to get rid of ldict from thread so
we create a new one the next time we do an attr
access */
PyDict_DelItem(tdict, self->key);
return NULL;
/* threading.local hasn't been instantiated for this thread */
PyObject *wr;
if (create_localsdict(self, state, &ldict, &wr) < 0) {
return NULL;
}
/* run __init__ if we're a subtype of `threading.local` */
if (Py_TYPE(self)->tp_init != PyBaseObject_Type.tp_init &&
Py_TYPE(self)->tp_init((PyObject *)self, self->args, self->kw) < 0) {
/* we need to get rid of ldict from thread so
we create a new one the next time we do an attr
access */
PyObject *exc = PyErr_GetRaisedException();
if (PyDict_DelItem(self->localdicts, tstate->threading_local_key) <
0) {
PyErr_WriteUnraisable((PyObject *)self);
PyErr_Clear();
}
if (PySet_Discard(self->thread_watchdogs, wr) < 0) {
PyErr_WriteUnraisable((PyObject *)self);
}
PyErr_SetRaisedException(exc);
Py_DECREF(ldict);
Py_DECREF(wr);
return NULL;
}
else {
assert(Py_IS_TYPE(dummy, state->local_dummy_type));
ldict = ((localdummyobject *) dummy)->localdict;
}
Py_DECREF(wr);
return ldict;
}
@ -1650,21 +1712,28 @@ local_setattro(localobject *self, PyObject *name, PyObject *v)
PyObject *ldict = _ldict(self, state);
if (ldict == NULL) {
return -1;
goto err;
}
int r = PyObject_RichCompareBool(name, &_Py_ID(__dict__), Py_EQ);
if (r == -1) {
return -1;
goto err;
}
if (r == 1) {
PyErr_Format(PyExc_AttributeError,
"'%.100s' object attribute '%U' is read-only",
Py_TYPE(self)->tp_name, name);
return -1;
goto err;
}
return _PyObject_GenericSetAttrWithDict((PyObject *)self, name, v, ldict);
int st =
_PyObject_GenericSetAttrWithDict((PyObject *)self, name, v, ldict);
Py_DECREF(ldict);
return st;
err:
Py_XDECREF(ldict);
return -1;
}
static PyObject *local_getattro(localobject *, PyObject *);
@ -1707,34 +1776,42 @@ local_getattro(localobject *self, PyObject *name)
int r = PyObject_RichCompareBool(name, &_Py_ID(__dict__), Py_EQ);
if (r == 1) {
return Py_NewRef(ldict);
return ldict;
}
if (r == -1) {
Py_DECREF(ldict);
return NULL;
}
if (!Py_IS_TYPE(self, state->local_type)) {
/* use generic lookup for subtypes */
return _PyObject_GenericGetAttrWithDict((PyObject *)self, name,
ldict, 0);
PyObject *res =
_PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict, 0);
Py_DECREF(ldict);
return res;
}
/* Optimization: just look in dict ourselves */
PyObject *value;
if (PyDict_GetItemRef(ldict, name, &value) != 0) {
// found or error
Py_DECREF(ldict);
return value;
}
/* Fall back on generic to get __class__ and __dict__ */
return _PyObject_GenericGetAttrWithDict(
(PyObject *)self, name, ldict, 0);
PyObject *res =
_PyObject_GenericGetAttrWithDict((PyObject *)self, name, ldict, 0);
Py_DECREF(ldict);
return res;
}
/* Called when a dummy is destroyed. */
/* Called when a dummy is destroyed, indicating that the owning thread is being
* cleared. */
static PyObject *
_localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
clear_locals(PyObject *locals_and_key, PyObject *dummyweakref)
{
PyObject *localweakref = PyTuple_GetItem(locals_and_key, 0);
localobject *self = (localobject *)_PyWeakref_GET_REF(localweakref);
if (self == NULL) {
Py_RETURN_NONE;
@ -1742,11 +1819,18 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
/* If the thread-local object is still alive and not being cleared,
remove the corresponding local dict */
if (self->dummies != NULL) {
if (PyDict_Pop(self->dummies, dummyweakref, NULL) < 0) {
if (self->localdicts != NULL) {
PyObject *key = PyTuple_GetItem(locals_and_key, 1);
if (PyDict_Pop(self->localdicts, key, NULL) < 0) {
PyErr_WriteUnraisable((PyObject*)self);
}
}
if (self->thread_watchdogs != NULL) {
if (PySet_Discard(self->thread_watchdogs, dummyweakref) < 0) {
PyErr_WriteUnraisable((PyObject *)self);
}
}
Py_DECREF(self);
Py_RETURN_NONE;
}

3
Python/pystate.c
View File

@ -1702,6 +1702,9 @@ PyThreadState_Clear(PyThreadState *tstate)
/* Don't clear tstate->pyframe: it is a borrowed reference */
Py_CLEAR(tstate->threading_local_key);
Py_CLEAR(tstate->threading_local_sentinel);
Py_CLEAR(((_PyThreadStateImpl *)tstate)->asyncio_running_loop);
Py_CLEAR(tstate->dict);