[Python-checkins] cpython (merge 3.5 -> 3.6): Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and

[antoine.pitrou]

https://hg.python.org/cpython/rev/f3706a9430da
changeset:   105739:f3706a9430da
branch:      3.6
parent:      105736:781c56168484
parent:      105738:5a542a2bca08
user:        Antoine Pitrou <solipsis at pitrou.net>
date:        Mon Dec 19 10:58:14 2016 +0100
summary:
  Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and WeakValueDictionary.pop()
when a GC collection happens in another thread.

Original patch and report by Armin Rigo.

files:
  Lib/test/test_weakref.py |  41 ++++++++++++++++++++++++++++
  Lib/weakref.py           |  13 +++++---
  Misc/NEWS                |   4 ++
  3 files changed, 53 insertions(+), 5 deletions(-)


diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -6,6 +6,7 @@
 import operator
 import contextlib
 import copy
+import time
 
 from test import support
 from test.support import script_helper
@@ -72,6 +73,29 @@
         self.cbcalled += 1
 
 
+ at contextlib.contextmanager
+def collect_in_thread(period=0.0001):
+    """
+    Ensure GC collections happen in a different thread, at a high frequency.
+    """
+    threading = support.import_module('threading')
+    please_stop = False
+
+    def collect():
+        while not please_stop:
+            time.sleep(period)
+            gc.collect()
+
+    with support.disable_gc():
+        t = threading.Thread(target=collect)
+        t.start()
+        try:
+            yield
+        finally:
+            please_stop = True
+            t.join()
+
+
 class ReferencesTestCase(TestBase):
 
     def test_basic_ref(self):
@@ -1636,6 +1660,23 @@
         dict = weakref.WeakKeyDictionary()
         self.assertRegex(repr(dict), '<WeakKeyDictionary at 0x.*>')
 
+    def test_threaded_weak_valued_setdefault(self):
+        d = weakref.WeakValueDictionary()
+        with collect_in_thread():
+            for i in range(100000):
+                x = d.setdefault(10, RefCycle())
+                self.assertIsNot(x, None)  # we never put None in there!
+                del x
+
+    def test_threaded_weak_valued_pop(self):
+        d = weakref.WeakValueDictionary()
+        with collect_in_thread():
+            for i in range(100000):
+                d[10] = RefCycle()
+                x = d.pop(10, 10)
+                self.assertIsNot(x, None)  # we never put None in there!
+
+
 from test import mapping_tests
 
 class WeakValueDictionaryTestCase(mapping_tests.BasicTestMappingProtocol):
diff --git a/Lib/weakref.py b/Lib/weakref.py
--- a/Lib/weakref.py
+++ b/Lib/weakref.py
@@ -239,24 +239,27 @@
         try:
             o = self.data.pop(key)()
         except KeyError:
+            o = None
+        if o is None:
             if args:
                 return args[0]
-            raise
-        if o is None:
-            raise KeyError(key)
+            else:
+                raise KeyError(key)
         else:
             return o
 
     def setdefault(self, key, default=None):
         try:
-            wr = self.data[key]
+            o = self.data[key]()
         except KeyError:
+            o = None
+        if o is None:
             if self._pending_removals:
                 self._commit_removals()
             self.data[key] = KeyedRef(default, self._remove, key)
             return default
         else:
-            return wr()
+            return o
 
     def update(*args, **kwargs):
         if not args:
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -38,6 +38,10 @@
 Library
 -------
 
+- Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and
+  WeakValueDictionary.pop() when a GC collection happens in another
+  thread.
+
 - Issue #20191: Fixed a crash in resource.prlimit() when pass a sequence that
   doesn't own its elements as limits.
 

-- 
Repository URL: https://hg.python.org/cpython