with the objective of completely destroying these objects. Roughly, the process
follows these steps in order:
-1. Handle and clear weak references (if any). Weak references to unreachable objects
- are set to `None`. If the weak reference has an associated callback, the callback
- is enqueued to be called once the clearing of weak references is finished. We only
- invoke callbacks for weak references that are themselves reachable. If both the weak
- reference and the pointed-to object are unreachable we do not execute the callback.
- This is partly for historical reasons: the callback could resurrect an unreachable
- object and support for weak references predates support for object resurrection.
- Ignoring the weak reference's callback is fine because both the object and the weakref
- are going away, so it's legitimate to say the weak reference is going away first.
+1. Handle weak references with callbacks (if any). If the weak reference has
+ an associated callback, the callback is enqueued to be called after the weak
+ reference is cleared. We only invoke callbacks for weak references that
+ are themselves reachable. If both the weak reference and the pointed-to
+ object are unreachable we do not execute the callback. This is partly for
+ historical reasons: the callback could resurrect an unreachable object
+ and support for weak references predates support for object resurrection.
+ Ignoring the weak reference's callback is fine because both the object and
+ the weakref are going away, so it's legitimate to say the weak reference is
+ going away first.
2. If an object has legacy finalizers (`tp_del` slot) move it to the
`gc.garbage` list.
3. Call the finalizers (`tp_finalize` slot) and mark the objects as already
4. Deal with resurrected objects. If some objects have been resurrected, the GC
finds the new subset of objects that are still unreachable by running the cycle
detection algorithm again and continues with them.
-5. Call the `tp_clear` slot of every object so all internal links are broken and
+5. Clear any weak references that still refer to unreachable objects. The
+ `wr_object` attribute for these weakrefs are set to `None`. Note that some
+ of these weak references maybe have been newly created during the running of
+ finalizers in step 3. Also, clear any weak references that are part of the
+ unreachable set.
+6. Call the `tp_clear` slot of every object so all internal links are broken and
the reference counts fall to 0, triggering the destruction of all unreachable
objects.
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(ids)
- self.assertIsNot(wr(), None)
+ self.assertIsNotNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
@support.cpython_only
def test_non_gc(self):
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
s = SelfCycleResurrector()
ids = [id(s)]
wr = weakref.ref(s)
+ wrc = weakref.ref(s, lambda x: None)
del s
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(ids)
- # XXX is this desirable?
- self.assertIs(wr(), None)
+ # This used to be None because weakrefs were cleared before
+ # calling finalizers. Now they are cleared after.
+ self.assertIsNotNone(wr())
+ # A weakref with a callback is still cleared before calling
+ # finalizers.
+ self.assertIsNone(wrc())
# When trying to destroy the object a second time, __del__
# isn't called anymore (and the object isn't resurrected).
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
def test_simple_suicide(self):
# Test the GC is able to deal with an object that kills its last
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
class ChainedBase:
def check_resurrecting_chain(self, classes):
N = len(classes)
+ def dummy_callback(ref):
+ pass
with SimpleBase.test():
nodes = self.build_chain(classes)
N = len(nodes)
ids = [id(s) for s in nodes]
survivor_ids = [id(s) for s in nodes if isinstance(s, SimpleResurrector)]
wrs = [weakref.ref(s) for s in nodes]
+ wrcs = [weakref.ref(s, dummy_callback) for s in nodes]
del nodes
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(survivor_ids)
- # XXX desirable?
- self.assertEqual([wr() for wr in wrs], [None] * N)
+ for wr in wrs:
+ # These values used to be None because weakrefs were cleared
+ # before calling finalizers. Now they are cleared after.
+ self.assertIsNotNone(wr())
+ for wr in wrcs:
+ # Weakrefs with callbacks are still cleared before calling
+ # finalizers.
+ self.assertIsNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids)
self.assert_survivors([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids)
self.assert_tp_del_calls(ids)
self.assert_survivors(ids)
# weakrefs are cleared before tp_del is called.
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids * 2)
self.assert_survivors(ids)
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
def test_legacy_self_cycle(self):
# Self-cycles with legacy finalizers end up in gc.garbage.
self.assert_tp_del_calls([])
self.assert_survivors([])
self.assert_garbage(ids)
- self.assertIsNot(wr(), None)
+ self.assertIsNotNone(wr())
# Break the cycle to allow collection
gc.garbage[0].ref = None
self.assert_garbage([])
- self.assertIs(wr(), None)
+ self.assertIsNone(wr())
if __name__ == "__main__":
self.assertRegex(stdout, rb"""\A\s*func=None""")
self.assertFalse(stderr)
+ def test_datetime_weakref_cycle(self):
+ # https://github.com/python/cpython/issues/132413
+ # If the weakref used by the datetime extension gets cleared by the GC (due to being
+ # in an unreachable cycle) then datetime functions would crash (get_module_state()
+ # was returning a NULL pointer). This bug is fixed by clearing weakrefs without
+ # callbacks *after* running finalizers.
+ code = """if 1:
+ import _datetime
+ class C:
+ def __del__(self):
+ print('__del__ called')
+ _datetime.timedelta(days=1) # crash?
+
+ l = [C()]
+ l.append(l)
+ """
+ rc, stdout, stderr = assert_python_ok("-c", code)
+ self.assertEqual(rc, 0)
+ self.assertEqual(stdout.strip(), b'__del__ called')
+
@refcount_test
def test_frame(self):
def f():
gc.collect()
self.assertEqual(len(ouch), 2) # else the callbacks didn't run
for x in ouch:
- # If the callback resurrected one of these guys, the instance
- # would be damaged, with an empty __dict__.
- self.assertEqual(x, None)
+ # The weakref should be cleared before executing the callback.
+ self.assertIsNone(x)
def test_bug21435(self):
# This is a poor test - its only virtue is that it happened to
def tearDown(self):
gc.disable()
+ @unittest.skipIf(Py_GIL_DISABLED, "requires GC generations or increments")
def test_bug1055820c(self):
# Corresponds to temp2c.py in the bug report. This is pretty
# elaborate.
self.assertEqual(x, None)
@gc_threshold(1000, 0, 0)
+ @unittest.skipIf(Py_GIL_DISABLED, "requires GC generations or increments")
def test_bug1055820d(self):
# Corresponds to temp2d.py in the bug report. This is very much like
# test_bug1055820c, but uses a __del__ method instead of a weakref
def test_garbage_collection(self):
# FileIO objects are collected, and collecting them flushes
# all data to disk.
- with warnings_helper.check_warnings(('', ResourceWarning)):
+ #
+ # Note that using warnings_helper.check_warnings() will keep the
+ # file alive due to the `source` argument to warn(). So, use
+ # catch_warnings() instead.
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore", ResourceWarning)
f = self.FileIO(os_helper.TESTFN, "wb")
f.write(b"abcxxx")
f.f = f
# C BufferedReader objects are collected.
# The Python version has __del__, so it ends into gc.garbage instead
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
- with warnings_helper.check_warnings(('', ResourceWarning)):
+ # Note that using warnings_helper.check_warnings() will keep the
+ # file alive due to the `source` argument to warn(). So, use
+ # catch_warnings() instead.
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore", ResourceWarning)
rawio = self.FileIO(os_helper.TESTFN, "w+b")
f = self.tp(rawio)
f.f = f
# all data to disk.
# The Python version has __del__, so it ends into gc.garbage instead
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
- with warnings_helper.check_warnings(('', ResourceWarning)):
+ # Note that using warnings_helper.check_warnings() will keep the
+ # file alive due to the `source` argument to warn(). So, use
+ # catch_warnings() instead.
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore", ResourceWarning)
rawio = self.FileIO(os_helper.TESTFN, "w+b")
f = self.tp(rawio)
f.write(b"123xxx")
# C TextIOWrapper objects are collected, and collecting them flushes
# all data to disk.
# The Python version has __del__, so it ends in gc.garbage instead.
- with warnings_helper.check_warnings(('', ResourceWarning)):
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore", ResourceWarning)
rawio = self.FileIO(os_helper.TESTFN, "wb")
b = self.BufferedWriter(rawio)
t = self.TextIOWrapper(b, encoding="ascii")
--- /dev/null
+Fix a bug caused by the garbage collector clearing weakrefs too early. The
+weakrefs in the ``tp_subclasses`` dictionary are needed in order to correctly
+invalidate type caches (for example, by calling ``PyType_Modified()``).
+Clearing weakrefs before calling finalizers causes the caches to not be
+correctly invalidated. That can cause crashes since the caches can refer to
+invalid objects. Defer the clearing of weakrefs without callbacks until after
+finalizers are executed.
if (PyDict_GetItemRef(dict, INTERP_KEY, &ref) < 0) {
goto error;
}
- if (ref != NULL) {
+ if (ref != NULL && ref != Py_None) {
PyObject *current = NULL;
int rc = PyWeakref_GetRef(ref, ¤t);
/* We only need "current" for pointer comparison. */
Intro
=====
+**************************************************************************
+Note: this document was written long ago, before PEP 442 (safe object
+finalization) was implemented. While that has changed some things, this
+document is still mostly accurate. Just note that the rules being discussed
+here apply to the unreachable set of objects *after* non-legacy finalizers
+have been called. Also, the clearing of weakrefs has been changed to happen
+later in the collection (after running finalizers but before tp_clear is
+called).
+**************************************************************************
+
The basic rule for dealing with weakref callbacks (and __del__ methods too,
for that matter) during cyclic gc:
}
}
-/* Clear all weakrefs to unreachable objects, and if such a weakref has a
- * callback, invoke it if necessary. Note that it's possible for such
- * weakrefs to be outside the unreachable set -- indeed, those are precisely
- * the weakrefs whose callbacks must be invoked. See gc_weakref.txt for
- * overview & some details. Some weakrefs with callbacks may be reclaimed
- * directly by this routine; the number reclaimed is the return value. Other
- * weakrefs with callbacks may be moved into the `old` generation. Objects
- * moved into `old` have gc_refs set to GC_REACHABLE; the objects remaining in
- * unreachable are left at GC_TENTATIVELY_UNREACHABLE. When this returns,
- * no object in `unreachable` is weakly referenced anymore.
+/* Handle weakref callbacks. Note that it's possible for such weakrefs to be
+ * outside the unreachable set -- indeed, those are precisely the weakrefs
+ * whose callbacks must be invoked. See gc_weakref.txt for overview & some
+ * details.
+ *
+ * The clearing of weakrefs is suble and must be done carefully, as there was
+ * previous bugs related to this. First, weakrefs to the unreachable set of
+ * objects must be cleared before we start calling `tp_clear`. If we don't,
+ * those weakrefs can reveal unreachable objects to Python-level code and that
+ * is not safe. Many objects are not usable after `tp_clear` has been called
+ * and could even cause crashes if accessed (see bpo-38006 and gh-91636 as
+ * example bugs).
+ *
+ * Weakrefs with callbacks always need to be cleared before executing the
+ * callback. That's because sometimes a callback will call the ref object,
+ * to check if the reference is actually dead (KeyedRef does this, for
+ * example). We want to indicate that it is dead, even though it is possible
+ * a finalizer might resurrect it. Clearing also prevents the callback from
+ * executing more than once.
+ *
+ * Since Python 2.3, all weakrefs to cyclic garbage have been cleared *before*
+ * calling finalizers. However, since tp_subclasses started being necessary
+ * to invalidate caches (e.g. by PyType_Modified), that clearing has created
+ * a bug. If the weakref to the subclass is cleared before a finalizer is
+ * called, the cache may not be correctly invalidated. That can lead to
+ * segfaults since the caches can refer to deallocated objects (GH-91636
+ * is an example). Now, we delay the clear of weakrefs without callbacks
+ * until *after* finalizers have been executed. That means weakrefs without
+ * callbacks are still usable while finalizers are being executed.
+ *
+ * The weakrefs that are inside the unreachable set must also be cleared.
+ * The reason this is required is not immediately obvious. If the weakref
+ * refers to an object outside of the unreachable set, that object might go
+ * away when we start clearing objects. Normally, the object should also be
+ * part of the unreachable set but that's not true in the case of incomplete
+ * or missing `tp_traverse` methods. When that object goes away, the callback
+ * for weakref can be executed and that could reveal unreachable objects to
+ * Python-level code. See bpo-38006 as an example bug.
*/
static int
-handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old, bool allow_callbacks)
+handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old)
{
PyGC_Head *gc;
- PyObject *op; /* generally FROM_GC(gc) */
- PyWeakReference *wr; /* generally a cast of op */
PyGC_Head wrcb_to_call; /* weakrefs with callbacks to call */
PyGC_Head *next;
int num_freed = 0;
- if (allow_callbacks) {
- gc_list_init(&wrcb_to_call);
- }
+ gc_list_init(&wrcb_to_call);
- /* Clear all weakrefs to the objects in unreachable. If such a weakref
- * also has a callback, move it into `wrcb_to_call` if the callback
- * needs to be invoked. Note that we cannot invoke any callbacks until
- * all weakrefs to unreachable objects are cleared, lest the callback
- * resurrect an unreachable object via a still-active weakref. We
- * make another pass over wrcb_to_call, invoking callbacks, after this
- * pass completes.
+ /* Find all weakrefs with callbacks and move into `wrcb_to_call` if the
+ * callback needs to be invoked. We make another pass over wrcb_to_call,
+ * invoking callbacks, after this pass completes.
*/
for (gc = GC_NEXT(unreachable); gc != unreachable; gc = next) {
PyWeakReference **wrlist;
- op = FROM_GC(gc);
+ PyObject *op = FROM_GC(gc);
next = GC_NEXT(gc);
- if (PyWeakref_Check(op)) {
- /* A weakref inside the unreachable set must be cleared. If we
- * allow its callback to execute inside delete_garbage(), it
- * could expose objects that have tp_clear already called on
- * them. Or, it could resurrect unreachable objects. One way
- * this can happen is if some container objects do not implement
- * tp_traverse. Then, wr_object can be outside the unreachable
- * set but can be deallocated as a result of breaking the
- * reference cycle. If we don't clear the weakref, the callback
- * will run and potentially cause a crash. See bpo-38006 for
- * one example.
- */
- _PyWeakref_ClearRef((PyWeakReference *)op);
- }
-
if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) {
continue;
}
*/
wrlist = _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(op);
- /* `op` may have some weakrefs. March over the list, clear
- * all the weakrefs, and move the weakrefs with callbacks
- * that must be called into wrcb_to_call.
+ /* `op` may have some weakrefs. March over the list and move the
+ * weakrefs with callbacks that must be called into wrcb_to_call.
*/
- for (wr = *wrlist; wr != NULL; wr = *wrlist) {
- PyGC_Head *wrasgc; /* AS_GC(wr) */
-
- /* _PyWeakref_ClearRef clears the weakref but leaves
- * the callback pointer intact. Obscure: it also
- * changes *wrlist.
- */
- _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
- _PyWeakref_ClearRef(wr);
- _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
-
- if (!allow_callbacks) {
- continue;
- }
+ PyWeakReference *next_wr;
+ for (PyWeakReference *wr = *wrlist; wr != NULL; wr = next_wr) {
+ // Get the next list element to get iterator progress if we omit
+ // clearing of the weakref (because _PyWeakref_ClearRef changes
+ // next pointer in the wrlist).
+ next_wr = wr->wr_next;
if (wr->wr_callback == NULL) {
/* no callback */
continue;
}
+ // Clear the weakref. See the comments above this function for
+ // reasons why we need to clear weakrefs that have callbacks.
+ // Note that _PyWeakref_ClearRef clears the weakref but leaves the
+ // callback pointer intact. Obscure: it also changes *wrlist.
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
+ _PyWeakref_ClearRef(wr);
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
+
/* Headache time. `op` is going away, and is weakly referenced by
* `wr`, which has a callback. Should the callback be invoked? If wr
* is also trash, no:
* outside the current generation, CT may be reachable from the
* callback. Then the callback could resurrect insane objects.
*
- * Since the callback is never needed and may be unsafe in this case,
- * wr is simply left in the unreachable set. Note that because we
- * already called _PyWeakref_ClearRef(wr), its callback will never
- * trigger.
+ * Since the callback is never needed and may be unsafe in this
+ * case, wr is simply left in the unreachable set. Note that
+ * clear_weakrefs() will ensure its callback will not trigger
+ * inside delete_garbage().
*
* OTOH, if wr isn't part of CT, we should invoke the callback: the
* weakref outlived the trash. Note that since wr isn't CT in this
* is moved to wrcb_to_call in this case.
*/
if (gc_is_collecting(AS_GC((PyObject *)wr))) {
- /* it should already have been cleared above */
- _PyObject_ASSERT((PyObject*)wr, wr->wr_object == Py_None);
continue;
}
Py_INCREF(wr);
/* Move wr to wrcb_to_call, for the next pass. */
- wrasgc = AS_GC((PyObject *)wr);
+ PyGC_Head *wrasgc = AS_GC((PyObject *)wr);
// wrasgc is reachable, but next isn't, so they can't be the same
_PyObject_ASSERT((PyObject *)wr, wrasgc != next);
gc_list_move(wrasgc, &wrcb_to_call);
}
}
- if (!allow_callbacks) {
- return 0;
- }
-
/* Invoke the callbacks we decided to honor. It's safe to invoke them
* because they can't reference unreachable objects.
*/
PyObject *callback;
gc = (PyGC_Head*)wrcb_to_call._gc_next;
- op = FROM_GC(gc);
+ PyObject *op = FROM_GC(gc);
_PyObject_ASSERT(op, PyWeakref_Check(op));
- wr = (PyWeakReference *)op;
+ PyWeakReference *wr = (PyWeakReference *)op;
callback = wr->wr_callback;
_PyObject_ASSERT(op, callback != NULL);
return num_freed;
}
+/* Clear all weakrefs to unreachable objects. When this returns, no object in
+ * `unreachable` is weakly referenced anymore. See the comments above
+ * handle_weakref_callbacks() for why these weakrefs need to be cleared.
+ */
+static void
+clear_weakrefs(PyGC_Head *unreachable)
+{
+ PyGC_Head *gc;
+ PyGC_Head *next;
+
+ for (gc = GC_NEXT(unreachable); gc != unreachable; gc = next) {
+ PyWeakReference **wrlist;
+
+ PyObject *op = FROM_GC(gc);
+ next = GC_NEXT(gc);
+
+ if (PyWeakref_Check(op)) {
+ /* A weakref inside the unreachable set is always cleared. See
+ * the comments above handle_weakref_callbacks() for why these
+ * must be cleared.
+ */
+ _PyWeakref_ClearRef((PyWeakReference *)op);
+ }
+
+ if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) {
+ continue;
+ }
+
+ /* It supports weakrefs. Does it have any?
+ *
+ * This is never triggered for static types so we can avoid the
+ * (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR().
+ */
+ wrlist = _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(op);
+
+ /* `op` may have some weakrefs. March over the list, clear
+ * all the weakrefs.
+ */
+ for (PyWeakReference *wr = *wrlist; wr != NULL; wr = *wrlist) {
+ /* _PyWeakref_ClearRef clears the weakref but leaves
+ * the callback pointer intact. Obscure: it also
+ * changes *wrlist.
+ */
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
+ _PyWeakref_ClearRef(wr);
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
+ }
+ }
+}
+
static void
debug_cycle(const char *msg, PyObject *op)
{
}
}
- /* Clear weakrefs and invoke callbacks as necessary. */
- stats->collected += handle_weakrefs(&unreachable, to, true);
+ /* Invoke weakref callbacks as necessary. */
+ stats->collected += handle_weakref_callbacks(&unreachable, to);
gc_list_validate_space(to, gcstate->visited_space);
validate_list(to, collecting_clear_unreachable_clear);
validate_list(&unreachable, collecting_set_unreachable_clear);
gc_list_init(&final_unreachable);
handle_resurrected_objects(&unreachable, &final_unreachable, to);
- /* Clear weakrefs to objects in the unreachable set. No Python-level
- * code must be allowed to access those unreachable objects. During
- * delete_garbage(), finalizers outside the unreachable set might run
- * and create new weakrefs. If those weakrefs were not cleared, they
- * could reveal unreachable objects. Callbacks are not executed.
+ /* Clear weakrefs to objects in the unreachable set. See the comments
+ * above handle_weakref_callbacks() for details.
*/
- handle_weakrefs(&final_unreachable, NULL, false);
+ clear_weakrefs(&final_unreachable);
/* Call tp_clear on objects in the final_unreachable set. This will cause
* the reference cycles to be broken. It may also cause some objects
return 0;
}
-// Clear all weakrefs to unreachable objects. Weakrefs with callbacks are
-// optionally enqueued in `wrcb_to_call`, but not invoked yet.
+// Weakrefs with callbacks are enqueued in `wrcb_to_call`, but not invoked
+// yet. Note that it's possible for such weakrefs to be outside the
+// unreachable set -- indeed, those are precisely the weakrefs whose callbacks
+// must be invoked. See gc_weakref.txt for overview & some details.
+//
+// The clearing of weakrefs is suble and must be done carefully, as there was
+// previous bugs related to this. First, weakrefs to the unreachable set of
+// objects must be cleared before we start calling `tp_clear`. If we don't,
+// those weakrefs can reveal unreachable objects to Python-level code and that
+// is not safe. Many objects are not usable after `tp_clear` has been called
+// and could even cause crashes if accessed (see bpo-38006 and gh-91636 as
+// example bugs).
+//
+// Weakrefs with callbacks always need to be cleared before executing the
+// callback. That's because sometimes a callback will call the ref object,
+// to check if the reference is actually dead (KeyedRef does this, for
+// example). We want to indicate that it is dead, even though it is possible
+// a finalizer might resurrect it. Clearing also prevents the callback from
+// executing more than once.
+//
+// Since Python 2.3, all weakrefs to cyclic garbage have been cleared *before*
+// calling finalizers. However, since tp_subclasses started being necessary
+// to invalidate caches (e.g. by PyType_Modified), that clearing has created
+// a bug. If the weakref to the subclass is cleared before a finalizer is
+// called, the cache may not be correctly invalidated. That can lead to
+// segfaults since the caches can refer to deallocated objects (GH-91636
+// is an example). Now, we delay the clear of weakrefs without callbacks
+// until *after* finalizers have been executed. That means weakrefs without
+// callbacks are still usable while finalizers are being executed.
+//
+// The weakrefs that are inside the unreachable set must also be cleared.
+// The reason this is required is not immediately obvious. If the weakref
+// refers to an object outside of the unreachable set, that object might go
+// away when we start clearing objects. Normally, the object should also be
+// part of the unreachable set but that's not true in the case of incomplete
+// or missing `tp_traverse` methods. When that object goes away, the callback
+// for weakref can be executed and that could reveal unreachable objects to
+// Python-level code. See bpo-38006 as an example bug.
static void
-clear_weakrefs(struct collection_state *state, bool enqueue_callbacks)
+find_weakref_callbacks(struct collection_state *state)
{
PyObject *op;
WORKSTACK_FOR_EACH(&state->unreachable, op) {
- if (PyWeakref_Check(op)) {
- // Clear weakrefs that are themselves unreachable to ensure their
- // callbacks will not be executed later from a `tp_clear()`
- // inside delete_garbage(). That would be unsafe: it could
- // resurrect a dead object or access a an already cleared object.
- // See bpo-38006 for one example.
- _PyWeakref_ClearRef((PyWeakReference *)op);
- }
-
if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) {
continue;
}
// `op` may have some weakrefs. March over the list, clear
// all the weakrefs, and enqueue the weakrefs with callbacks
// that must be called into wrcb_to_call.
- for (PyWeakReference *wr = *wrlist; wr != NULL; wr = *wrlist) {
- // _PyWeakref_ClearRef clears the weakref but leaves
- // the callback pointer intact. Obscure: it also
- // changes *wrlist.
- _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
- _PyWeakref_ClearRef(wr);
- _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
-
- if (!enqueue_callbacks) {
- continue;
+ PyWeakReference *next_wr;
+ for (PyWeakReference *wr = *wrlist; wr != NULL; wr = next_wr) {
+ // Get the next list element to get iterator progress if we omit
+ // clearing of the weakref (because _PyWeakref_ClearRef changes
+ // next pointer in the wrlist).
+ next_wr = wr->wr_next;
+
+ // Weakrefs with callbacks always need to be cleared before
+ // executing the callback.
+ if (wr->wr_callback != NULL) {
+ // _PyWeakref_ClearRef clears the weakref but leaves the
+ // callback pointer intact. Obscure: it also changes *wrlist.
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
+ _PyWeakref_ClearRef(wr);
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
}
// We do not invoke callbacks for weakrefs that are themselves
}
}
+// Clear weakrefs to objects in the unreachable set. See comments
+// above find_weakref_callbacks() for why this clearing is required.
+static void
+clear_weakrefs(struct collection_state *state)
+{
+ PyObject *op;
+ WORKSTACK_FOR_EACH(&state->unreachable, op) {
+ if (PyWeakref_Check(op)) {
+ // Clear weakrefs that are themselves unreachable.
+ _PyWeakref_ClearRef((PyWeakReference *)op);
+ }
+
+ if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) {
+ continue;
+ }
+
+ // NOTE: This is never triggered for static types so we can avoid the
+ // (slightly) more costly _PyObject_GET_WEAKREFS_LISTPTR().
+ PyWeakReference **wrlist = _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(op);
+
+ // `op` may have some weakrefs. March over the list, clear
+ // all the weakrefs.
+ for (PyWeakReference *wr = *wrlist; wr != NULL; wr = *wrlist) {
+ // _PyWeakref_ClearRef clears the weakref but leaves
+ // the callback pointer intact. Obscure: it also
+ // changes *wrlist.
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == op);
+ _PyWeakref_ClearRef(wr);
+ _PyObject_ASSERT((PyObject *)wr, wr->wr_object == Py_None);
+ }
+ }
+}
+
static void
call_weakref_callbacks(struct collection_state *state)
{
// Record the number of live GC objects
interp->gc.long_lived_total = state->long_lived_total;
- // Clear weakrefs and enqueue callbacks (but do not call them).
- clear_weakrefs(state, true);
+ // Find weakref callbacks we will honor (but do not call them).
+ find_weakref_callbacks(state);
_PyEval_StartTheWorld(interp);
// Deallocate any object from the refcount merge step
// Clear free lists in all threads
_PyGC_ClearAllFreeLists(interp);
if (err == 0) {
- // Clear weakrefs to objects in the unreachable set. No Python-level
- // code must be allowed to access those unreachable objects. During
- // delete_garbage(), finalizers outside the unreachable set might
- // run and create new weakrefs. If those weakrefs were not cleared,
- // they could reveal unreachable objects.
- clear_weakrefs(state, false);
+ clear_weakrefs(state);
}
_PyEval_StartTheWorld(interp);
projects / thirdparty / Python / cpython.git / commitdiff
