[pypy-commit] pypy default: (arigo, antocuni): test and fix for an obscure bug when deleting attributes inside slots(), which caused random segfaults because you could end up with a NULL instead of an actual W_Root

[antocuni]

Author: Antonio Cuni <anto.cuni at gmail.com>
Branch: 
Changeset: r46242:34f36be0f080
Date: 2011-08-03 15:05 +0200
http://bitbucket.org/pypy/pypy/changeset/34f36be0f080/

Log:	(arigo, antocuni): test and fix for an obscure bug when deleting
	attributes inside slots(), which caused random segfaults because you
	could end up with a NULL instead of an actual W_Root

diff --git a/pypy/interpreter/baseobjspace.py b/pypy/interpreter/baseobjspace.py
--- a/pypy/interpreter/baseobjspace.py
+++ b/pypy/interpreter/baseobjspace.py
@@ -111,6 +111,9 @@
     def setslotvalue(self, index, w_val):
         raise NotImplementedError
 
+    def delslotvalue(self, index):
+        raise NotImplementedError
+
     def descr_call_mismatch(self, space, opname, RequiredClass, args):
         if RequiredClass is None:
             classname = '?'
diff --git a/pypy/interpreter/typedef.py b/pypy/interpreter/typedef.py
--- a/pypy/interpreter/typedef.py
+++ b/pypy/interpreter/typedef.py
@@ -258,6 +258,11 @@
                     self.slots_w = [None] * nslots
             def setslotvalue(self, index, w_value):
                 self.slots_w[index] = w_value
+            def delslotvalue(self, index):
+                if self.slots_w[index] is None:
+                    return False
+                self.slots_w[index] = None
+                return True
             def getslotvalue(self, index):
                 return self.slots_w[index]
         add(Proto)
@@ -530,11 +535,10 @@
         """member.__delete__(obj)
         Delete the value of the slot 'member' from the given 'obj'."""
         self.typecheck(space, w_obj)
-        w_oldresult = w_obj.getslotvalue(self.index)
-        if w_oldresult is None:
+        success = w_obj.delslotvalue(self.index)
+        if not success:
             raise OperationError(space.w_AttributeError,
                                  space.wrap(self.name)) # XXX better message
-        w_obj.setslotvalue(self.index, None)
 
 Member.typedef = TypeDef(
     "member_descriptor",
diff --git a/pypy/objspace/std/mapdict.py b/pypy/objspace/std/mapdict.py
--- a/pypy/objspace/std/mapdict.py
+++ b/pypy/objspace/std/mapdict.py
@@ -421,6 +421,14 @@
         key = ("slot", SLOTS_STARTING_FROM + index)
         self._get_mapdict_map().write(self, key, w_value)
 
+    def delslotvalue(self, index):
+        key = ("slot", SLOTS_STARTING_FROM + index)
+        new_obj = self._get_mapdict_map().delete(self, key)
+        if new_obj is None:
+            return False
+        self._become(new_obj)
+        return True
+
     # used by _weakref implemenation
 
     def getweakref(self):
diff --git a/pypy/objspace/std/test/test_mapdict.py b/pypy/objspace/std/test/test_mapdict.py
--- a/pypy/objspace/std/test/test_mapdict.py
+++ b/pypy/objspace/std/test/test_mapdict.py
@@ -210,6 +210,12 @@
     assert obj2.storage == [501, 601, 701, 51, 61, 71]
     assert obj.map is obj2.map
 
+    assert obj2.getslotvalue(b) == 601
+    assert obj2.delslotvalue(b)
+    assert obj2.getslotvalue(b) is None
+    assert obj2.storage == [501, 701, 51, 61, 71]
+    assert not obj2.delslotvalue(b)
+
 
 def test_slots_no_dict():
     cls = Class(hasdict=False)
@@ -631,6 +637,14 @@
         a.__dict__ = {}
         a.__dict__ = {}
 
+    def test_delete_slot(self):
+        class A(object):
+            __slots__ = ['x']
+        
+        a = A()
+        a.x = 42
+        del a.x
+        raises(AttributeError, "a.x")
 
 class AppTestWithMapDictAndCounters(object):
     def setup_class(cls):