pretty subtle bug: when iterating over a map dict, some items would go missing!

2 files changed, 24 insertions, 7 deletions

diff --git a/pypy/objspace/std/mapdict.py b/pypy/objspace/std/mapdict.py
index 6eb944753b..e04266cc98 100644
--- a/pypy/objspace/std/mapdict.py
+++ b/pypy/objspace/std/mapdict.py
@@ -1095,7 +1095,7 @@ class IteratorMixin(object):
     def _init(self, strategy, w_dict):
         w_obj = strategy.unerase(w_dict.dstorage)
         self.w_obj = w_obj
-        self.orig_map = curr_map = w_obj._get_mapdict_map()
+        curr_map = w_obj._get_mapdict_map()
         # We enumerate non-lazily the attributes, and store them in the
         # 'attrs' list.  We then walk that list in opposite order.  This
         # gives an ordering that is more natural (roughly corresponding
@@ -1122,8 +1122,6 @@ class MapDictIteratorKeys(BaseKeyIterator):
 
     def next_key_entry(self):
         assert isinstance(self.w_dict.get_strategy(), MapDictStrategy)
-        if self.orig_map is not self.w_obj._get_mapdict_map():
-            return None
         attrs = self.attrs
         if len(attrs) > 0:
             attr = attrs.pop()
@@ -1141,8 +1139,6 @@ class MapDictIteratorValues(BaseValueIterator):
 
     def next_value_entry(self):
         assert isinstance(self.w_dict.get_strategy(), MapDictStrategy)
-        if self.orig_map is not self.w_obj._get_mapdict_map():
-            return None
         attrs = self.attrs
         if len(attrs) > 0:
             attr = attrs.pop()
@@ -1159,8 +1155,6 @@ class MapDictIteratorItems(BaseItemIterator):
 
     def next_item_entry(self):
         assert isinstance(self.w_dict.get_strategy(), MapDictStrategy)
-        if self.orig_map is not self.w_obj._get_mapdict_map():
-            return None, None
         attrs = self.attrs
         if len(attrs) > 0:
             attr = attrs.pop()
diff --git a/pypy/objspace/std/test/test_mapdict.py b/pypy/objspace/std/test/test_mapdict.py
index 6cf363fcdb..ebc92bc1df 100644
--- a/pypy/objspace/std/test/test_mapdict.py
+++ b/pypy/objspace/std/test/test_mapdict.py
@@ -1197,6 +1197,29 @@ class AppTestWithMapDict(object):
         for key in d:
             assert d[key] == int(key)
 
+    def test_bug_iter_checks_map_is_wrong(self):
+        # obvious in hindsight, but this test shows that checking that the map
+        # stays the same during a.__dict__ iterations is too strict now
+        class A(object):
+            pass
+
+        # an instance with unboxed storage
+        a = A()
+        a.x = "a"
+        a.y = 1
+        a.z = "b"
+
+        a1 = A()
+        a1.x = "a"
+        a1.y = 1
+        a1.z = "b"
+        a1.y = None # mark the terminator as allow_unboxing = False
+
+        d = a.__dict__
+        # reading a.y during iteration changes the map! now that the iterators
+        # store all the attrs anyway, just remove the check
+        res = list(d.iteritems())
+        assert res == [('x', 'a'), ('y', 1), ('z', 'b')]
 
 
 class AppTestWithMapDictAndCounters(object):