Commits

Armin Rigo committed dc505e0

Another attempt to make it so that weakrefs in RPython are cleared as
soon as the finalizer is enqueued. So if the __del__ is called, we know
that we really have no more strong references to the object anywhere at
that point in time. Before, due to the incremental GC, it would be
possible to fetch the content of the weakref at just the wrong point in
time, and still have the finalizer called afterwards. This is explained
in https://bugs.pypy.org/issue1687, where this causes surprizes.

  • Participants
  • Parent commits 9b8d5bb

Comments (0)

Files changed (3)

rpython/memory/gc/incminimark.py

                 #
                 if self.objects_with_finalizers.non_empty():
                     self.deal_with_objects_with_finalizers()
+                elif self.old_objects_with_weakrefs.non_empty():
+                    # Weakref support: clear the weak pointers to dying objects
+                    # (if we call deal_with_objects_with_finalizers(), it will
+                    # invoke invalidate_old_weakrefs() itself directly)
+                    self.invalidate_old_weakrefs()
 
                 ll_assert(not self.objects_to_trace.non_empty(),
                           "objects_to_trace should be empty")
                 self.more_objects_to_trace.delete()
 
                 #
-                # Weakref support: clear the weak pointers to dying objects
-                if self.old_objects_with_weakrefs.non_empty():
-                    self.invalidate_old_weakrefs()
+                # Light finalizers
                 if self.old_objects_with_light_finalizers.non_empty():
                     self.deal_with_old_objects_with_finalizers()
                 #objects_to_trace processed fully, can move on to sweeping
                     self._recursively_bump_finalization_state_from_2_to_3(y)
             self._recursively_bump_finalization_state_from_1_to_2(x)
 
+        # Clear the weak pointers to dying objects.  Also clears them if
+        # they point to objects which have the GCFLAG_FINALIZATION_ORDERING
+        # bit set here.  These are objects which will be added to
+        # run_finalizers().
+        self.invalidate_old_weakrefs()
+
         while marked.non_empty():
             x = marked.popleft()
             state = self._finalization_state(x)
             ll_assert((self.header(pointing_to).tid & GCFLAG_NO_HEAP_PTRS)
                       == 0, "registered old weakref should not "
                             "point to a NO_HEAP_PTRS obj")
-            if self.header(pointing_to).tid & GCFLAG_VISITED:
+            tid = self.header(pointing_to).tid
+            if ((tid & (GCFLAG_VISITED | GCFLAG_FINALIZATION_ORDERING)) ==
+                        GCFLAG_VISITED):
                 new_with_weakref.append(obj)
             else:
                 (obj + offset).address[0] = llmemory.NULL

rpython/memory/test/gc_test_base.py

     GC_CAN_SHRINK_ARRAY = False
     GC_CAN_SHRINK_BIG_ARRAY = False
     BUT_HOW_BIG_IS_A_BIG_STRING = 3*WORD
+    WREF_IS_INVALID_BEFORE_DEL_IS_CALLED = False
 
     def setup_class(cls):
         cls._saved_logstate = py.log._getstate()
         class A(object):
             count = 0
         a = A()
+        expected_invalid = self.WREF_IS_INVALID_BEFORE_DEL_IS_CALLED
         class B(object):
             def __del__(self):
                 # when __del__ is called, the weakref to myself is still valid
-                # in RPython (at least with most GCs; this test might be
-                # skipped for specific GCs)
-                if self.ref() is self:
-                    a.count += 10  # ok
+                # in RPython with most GCs.  However, this can lead to strange
+                # bugs with incminimark.  https://bugs.pypy.org/issue1687
+                # So with incminimark, we expect the opposite.
+                if expected_invalid:
+                    if self.ref() is None:
+                        a.count += 10  # ok
+                    else:
+                        a.count = 666  # not ok
                 else:
-                    a.count = 666  # not ok
+                    if self.ref() is self:
+                        a.count += 10  # ok
+                    else:
+                        a.count = 666  # not ok
         def g():
             b = B()
             ref = weakref.ref(b)

rpython/memory/test/test_incminimark_gc.py

-from rpython.rlib.rarithmetic import LONG_BIT
+from rpython.rtyper.lltypesystem import lltype
+from rpython.rtyper.lltypesystem.lloperation import llop
 
 from rpython.memory.test import test_minimark_gc
 
 class TestIncrementalMiniMarkGC(test_minimark_gc.TestMiniMarkGC):
     from rpython.memory.gc.incminimark import IncrementalMiniMarkGC as GCClass
+    WREF_IS_INVALID_BEFORE_DEL_IS_CALLED = True
+
+    def test_weakref_not_in_stack(self):
+        import weakref
+        class A(object):
+            pass
+        class B(object):
+            def __init__(self, next):
+                self.next = next
+        def g():
+            a = A()
+            a.x = 5
+            wr = weakref.ref(a)
+            llop.gc__collect(lltype.Void)   # make everything old
+            assert wr() is not None
+            assert a.x == 5
+            return wr
+        def f():
+            ref = g()
+            llop.gc__collect(lltype.Void, 1)    # start a major cycle
+            # at this point the stack is scanned, and the weakref points
+            # to an object not found, but still reachable:
+            b = ref()
+            llop.debug_print(lltype.Void, b)
+            assert b is not None
+            llop.gc__collect(lltype.Void)   # finish the major cycle
+            # assert does not crash, because 'b' is still kept alive
+            b.x = 42
+            return ref() is b
+        res = self.interpret(f, [])
+        assert res == True