New submission from Ben Cottrell <[EMAIL PROTECTED]>: 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 <[EMAIL PROTECTED]> <http://bugs.python.org/issue3710> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com