[Python-checkins] r75127 - in python/branches/release25-maint: Lib/test/test_threading_local.py Misc/NEWS Modules/threadmodule.c

[philip.jenvey]

Author: philip.jenvey
Date: Tue Sep 29 07:30:42 2009
New Revision: 75127

Log:
#6990: clear threading.local's key only after its thread state is removed:
fixes local subclasses leaving old state around after a ref cycle GC which
could be recycled by new locals
(backported from r75123)


Modified:
   python/branches/release25-maint/Lib/test/test_threading_local.py
   python/branches/release25-maint/Misc/NEWS
   python/branches/release25-maint/Modules/threadmodule.c

Modified: python/branches/release25-maint/Lib/test/test_threading_local.py
==============================================================================
--- python/branches/release25-maint/Lib/test/test_threading_local.py	(original)
+++ python/branches/release25-maint/Lib/test/test_threading_local.py	Tue Sep 29 07:30:42 2009
@@ -1,3 +1,5 @@
+import gc
+import threading
 import unittest
 from doctest import DocTestSuite
 from test import test_support
@@ -8,7 +10,6 @@
         # of a threading.local derived class, the per-thread dictionary
         # is created but not correctly set on the object.
         # The first member set may be bogus.
-        import threading
         import time
         class Local(threading.local):
             def __init__(self):
@@ -29,6 +30,44 @@
         for t in threads:
             t.join()
 
+    def test_derived_cycle_dealloc(self):
+        # http://bugs.python.org/issue6990
+        class Local(threading.local):
+            pass
+        locals = None
+        passed = [False]
+        e1 = threading.Event()
+        e2 = threading.Event()
+
+        def f():
+            # 1) Involve Local in a cycle
+            cycle = [Local()]
+            cycle.append(cycle)
+            cycle[0].foo = 'bar'
+
+            # 2) GC the cycle (triggers threadmodule.c::local_clear
+            # before local_dealloc)
+            del cycle
+            gc.collect()
+            e1.set()
+            e2.wait()
+
+            # 4) New Locals should be empty
+            passed[0] = all(not hasattr(local, 'foo') for local in locals)
+
+        t = threading.Thread(target=f)
+        t.start()
+        e1.wait()
+
+        # 3) New Locals should recycle the original's address. Creating
+        # them in the thread overwrites the thread state and avoids the
+        # bug
+        locals = [Local() for i in range(10)]
+        e2.set()
+        t.join()
+
+        self.assertTrue(passed[0])
+
 
 def test_main():
     suite = DocTestSuite('_threading_local')

Modified: python/branches/release25-maint/Misc/NEWS
==============================================================================
--- python/branches/release25-maint/Misc/NEWS	(original)
+++ python/branches/release25-maint/Misc/NEWS	Tue Sep 29 07:30:42 2009
@@ -9,6 +9,12 @@
 
 *Release date: XX-XXX-20XX*
 
+Core and builtins
+-----------------
+
+- Issue #6990: Fix threading.local subclasses leaving old state around
+  after a reference cycle GC which could be recycled by new locals.
+
 Library
 -------
 

Modified: python/branches/release25-maint/Modules/threadmodule.c
==============================================================================
--- python/branches/release25-maint/Modules/threadmodule.c	(original)
+++ python/branches/release25-maint/Modules/threadmodule.c	Tue Sep 29 07:30:42 2009
@@ -228,7 +228,6 @@
 static int
 local_clear(localobject *self)
 {
-	Py_CLEAR(self->key);
 	Py_CLEAR(self->args);
 	Py_CLEAR(self->kw);
 	Py_CLEAR(self->dict);
@@ -250,6 +249,7 @@
 				PyDict_DelItem(tstate->dict, self->key);
 	}
 
+	Py_XDECREF(self->key);
 	local_clear(self);
 	self->ob_type->tp_free((PyObject*)self);
 }