Author tamino
Recipients tamino
Date 2008-08-28.00:08:56
SpamBayes Score 1.11022e-16
Marked as misclassified No
Message-id <1219882144.91.0.399316525956.issue3710@psf.upfronthosting.co.za>
In-reply-to
Content
This is a copy of a message I sent to the python-dev mailing list; it
was suggested in a reply that I file a bug for this issue. I'm
filing it against Python 2.5 because that's where I noticed it,
but it doesn't look like this code has changed much in trunk.

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()?
History
Date User Action Args
2008-08-28 00:09:05taminosetrecipients: + tamino
2008-08-28 00:09:04taminosetmessageid: <1219882144.91.0.399316525956.issue3710@psf.upfronthosting.co.za>
2008-08-28 00:09:01taminolinkissue3710 messages
2008-08-28 00:08:59taminocreate