I noticed that thread._local can leak references if objects are
being stored inside the thread._local object whose destructors
might release the GIL.
The way this happens is that in Modules/threadmodule.c, in the
_ldict() function, it does things like this:
Py_CLEAR(self->dict);
Py_INCREF(ldict);
self->dict = ldict;
If the Py_CLEAR ends up taking away the last reference to an object
contained in the dict, and a thread context switch occurs during that
object's deallocation, then self->dict might not be NULL on return
from Py_CLEAR; another thread might have run, accessed something in
the same thread._local object, and caused self->dict to be set to
something else (and Py_INCREF'ed). So when we blindly do the
assignment into self->dict, we may be overwriting a valid reference,
and not properly Py_DECREFing it.
The recent change (revision 64601 to threadmodule.c) did not address
context switches during the Py_CLEAR call; only context switches
during tp_init.
The attached patch (against trunk) is my first attempt at fixing this.
It detects if self->dict has been set to something else after the
Py_CLEAR, and retries the Py_CLEAR (because _ldict really only cares
about installing the proper value of self->dict for the currently
running thread).
However, I am still uncomfortable about the fact that local_getattro
and local_setattro discard the value returned from _ldict, and instead
hand off control to the PyObject_Generic layer and trust that by the
time self->dict is actually used, it still has the correct value for
the current thread. Would it be better to, say, inline a copy of the
PyObject_Generic* functions inside local_getattro/local_setattro,
and force the operations to be done on the actual dict returned by
_ldict()?
Thanks,
~Ben
Index: Modules/threadmodule.c
===================================================================
--- Modules/threadmodule.c (revision 66050)
+++ Modules/threadmodule.c (working copy)
@@ -278,7 +278,9 @@
return NULL;
}
- Py_CLEAR(self->dict);
+ while (self->dict != NULL) {
+ Py_CLEAR(self->dict);
+ }
Py_INCREF(ldict);
self->dict = ldict; /* still borrowed */
@@ -297,7 +299,9 @@
/* The call to tp_init above may have caused another thread to run.
Install our ldict again. */
if (self->dict != ldict) {
- Py_CLEAR(self->dict);
+ while (self->dict != NULL) {
+ Py_CLEAR(self->dict);
+ }
Py_INCREF(ldict);
self->dict = ldict;
}
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
