[pypy-commit] pypy default: rawrefcount fix: pyobjs waiting on the dead list of the GC should not

[arigo]

Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r83122:0173cdbbbacc
Date: 2016-03-18 09:46 +0000
http://bitbucket.org/pypy/pypy/changeset/0173cdbbbacc/

Log:	rawrefcount fix: pyobjs waiting on the dead list of the GC should
	not have refcnt == 0. See comment.

diff --git a/pypy/module/cpyext/api.py b/pypy/module/cpyext/api.py
--- a/pypy/module/cpyext/api.py
+++ b/pypy/module/cpyext/api.py
@@ -833,14 +833,14 @@
     modulename = py.path.local(eci.libraries[-1])
 
     def dealloc_trigger():
-        from pypy.module.cpyext.pyobject import _Py_Dealloc
+        from pypy.module.cpyext.pyobject import decref
         print 'dealloc_trigger...'
         while True:
             ob = rawrefcount.next_dead(PyObject)
             if not ob:
                 break
             print ob
-            _Py_Dealloc(space, ob)
+            decref(space, ob)
         print 'dealloc_trigger DONE'
         return "RETRY"
     rawrefcount.init(dealloc_trigger)
diff --git a/pypy/module/cpyext/state.py b/pypy/module/cpyext/state.py
--- a/pypy/module/cpyext/state.py
+++ b/pypy/module/cpyext/state.py
@@ -147,10 +147,10 @@
     """
 
     def perform(self, executioncontext, frame):
-        from pypy.module.cpyext.pyobject import PyObject, _Py_Dealloc
+        from pypy.module.cpyext.pyobject import PyObject, decref
 
         while True:
             py_obj = rawrefcount.next_dead(PyObject)
             if not py_obj:
                 break
-            _Py_Dealloc(self.space, py_obj)
+            decref(self.space, py_obj)
diff --git a/rpython/memory/gc/incminimark.py b/rpython/memory/gc/incminimark.py
--- a/rpython/memory/gc/incminimark.py
+++ b/rpython/memory/gc/incminimark.py
@@ -2929,10 +2929,19 @@
             ll_assert(rc < int(REFCNT_FROM_PYPY_LIGHT * 0.99),
                       "refcount underflow from REFCNT_FROM_PYPY_LIGHT?")
             rc -= REFCNT_FROM_PYPY
-            self._pyobj(pyobject).ob_refcnt = rc
             self._pyobj(pyobject).ob_pypy_link = 0
             if rc == 0:
                 self.rrc_dealloc_pending.append(pyobject)
+                # an object with refcnt == 0 cannot stay around waiting
+                # for its deallocator to be called.  Some code (lxml)
+                # expects that tp_dealloc is called immediately when
+                # the refcnt drops to 0.  If it isn't, we get some
+                # uncleared raw pointer that can still be used to access
+                # the object; but (PyObject *)raw_pointer is then bogus
+                # because after a Py_INCREF()/Py_DECREF() on it, its
+                # tp_dealloc is also called!
+                rc = 1
+            self._pyobj(pyobject).ob_refcnt = rc
     _rrc_free._always_inline_ = True
 
     def rrc_major_collection_trace(self):
diff --git a/rpython/memory/gc/test/test_rawrefcount.py b/rpython/memory/gc/test/test_rawrefcount.py
--- a/rpython/memory/gc/test/test_rawrefcount.py
+++ b/rpython/memory/gc/test/test_rawrefcount.py
@@ -174,7 +174,7 @@
         p1 = check_alive(0)
         self._collect(major=True, expected_trigger=1)
         py.test.raises(RuntimeError, "p1.x")            # dead
-        assert r1.ob_refcnt == 0
+        assert r1.ob_refcnt == 1       # in the pending list
         assert r1.ob_pypy_link == 0
         assert self.gc.rawrefcount_next_dead() == r1addr
         assert self.gc.rawrefcount_next_dead() == llmemory.NULL
@@ -197,7 +197,7 @@
         assert p1.x == 42
         self._collect(major=True, expected_trigger=1)
         py.test.raises(RuntimeError, "p1.x")            # dead
-        assert r1.ob_refcnt == 0
+        assert r1.ob_refcnt == 1
         assert r1.ob_pypy_link == 0
         assert self.gc.rawrefcount_next_dead() == r1addr
         self.gc.check_no_more_rawrefcount_state()
@@ -214,7 +214,7 @@
         else:
             self._collect(major=False, expected_trigger=1)
         py.test.raises(RuntimeError, "p1.x")            # dead
-        assert r1.ob_refcnt == 0
+        assert r1.ob_refcnt == 1
         assert r1.ob_pypy_link == 0
         assert self.gc.rawrefcount_next_dead() == r1addr
         self.gc.check_no_more_rawrefcount_state()
@@ -252,7 +252,7 @@
             self._collect(major=True, expected_trigger=1)
         else:
             self._collect(major=False, expected_trigger=1)
-        assert r1.ob_refcnt == 0     # refcnt dropped to 0
+        assert r1.ob_refcnt == 1     # refcnt 1, in the pending list
         assert r1.ob_pypy_link == 0  # detached
         assert self.gc.rawrefcount_next_dead() == r1addr
         self.gc.check_no_more_rawrefcount_state()
@@ -277,7 +277,7 @@
         assert self.trigger == []
         self._collect(major=True, expected_trigger=1)
         py.test.raises(RuntimeError, "p1.x")            # dead
-        assert r1.ob_refcnt == 0
+        assert r1.ob_refcnt == 1
         assert r1.ob_pypy_link == 0
         assert self.gc.rawrefcount_next_dead() == r1addr
         self.gc.check_no_more_rawrefcount_state()
diff --git a/rpython/rlib/rawrefcount.py b/rpython/rlib/rawrefcount.py
--- a/rpython/rlib/rawrefcount.py
+++ b/rpython/rlib/rawrefcount.py
@@ -136,6 +136,7 @@
                 ob.c_ob_refcnt -= REFCNT_FROM_PYPY
                 ob.c_ob_pypy_link = 0
                 if ob.c_ob_refcnt == 0:
+                    ob.c_ob_refcnt = 1
                     _d_list.append(ob)
             return None
 
diff --git a/rpython/rlib/test/test_rawrefcount.py b/rpython/rlib/test/test_rawrefcount.py
--- a/rpython/rlib/test/test_rawrefcount.py
+++ b/rpython/rlib/test/test_rawrefcount.py
@@ -116,7 +116,7 @@
         assert rawrefcount.next_dead(PyObject) == lltype.nullptr(PyObjectS)
         assert rawrefcount._o_list == []
         assert wr_p() is None
-        assert ob.c_ob_refcnt == 0
+        assert ob.c_ob_refcnt == 1       # from the pending list
         assert ob.c_ob_pypy_link == 0
         lltype.free(ob, flavor='raw')
 
@@ -173,7 +173,7 @@
         assert rawrefcount._d_list == [ob]
         assert rawrefcount._p_list == []
         assert wr_p() is None
-        assert ob.c_ob_refcnt == 0
+        assert ob.c_ob_refcnt == 1       # from _d_list
         assert ob.c_ob_pypy_link == 0
         lltype.free(ob, flavor='raw')