[New-bugs-announce] [issue3710] Reference leak in thread._local

[Ben Cottrell]

New submission from Ben Cottrell <tamino at wolfhut.org>:

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()?

----------
components: Extension Modules
files: threadmodule.c.diff
keywords: patch
messages: 72052
nosy: tamino
severity: normal
status: open
title: Reference leak in thread._local
type: behavior
versions: Python 2.5
Added file: http://bugs.python.org/file11278/threadmodule.c.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue3710>
_______________________________________