Skip to content

Commit

Permalink
pythongh-126312: Don't traverse frozen objects on the free-threaded b…
Browse files Browse the repository at this point in the history
…uild (python#126338)

Also, _PyGC_Freeze() no longer freezes unreachable objects.

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
  • Loading branch information
ZeroIntensity and skirpichev authored Nov 15, 2024
1 parent 8717f79 commit d4c72fe
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 5 deletions.
38 changes: 38 additions & 0 deletions Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,44 @@ def __del__(self):
gc.collect()
self.assertTrue(collected)

def test_traverse_frozen_objects(self):
# See GH-126312: Objects that were not frozen could traverse over
# a frozen object on the free-threaded build, which would cause
# a negative reference count.
x = [1, 2, 3]
gc.freeze()
y = [x]
y.append(y)
del y
gc.collect()
gc.unfreeze()

def test_deferred_refcount_frozen(self):
# Also from GH-126312: objects that use deferred reference counting
# weren't ignored if they were frozen. Unfortunately, it's pretty
# difficult to come up with a case that triggers this.
#
# Calling gc.collect() while the garbage collector is frozen doesn't
# trigger this normally, but it *does* if it's inside unittest for whatever
# reason. We can't call unittest from inside a test, so it has to be
# in a subprocess.
source = textwrap.dedent("""
import gc
import unittest
class Test(unittest.TestCase):
def test_something(self):
gc.freeze()
gc.collect()
gc.unfreeze()
if __name__ == "__main__":
unittest.main()
""")
assert_python_ok("-c", source)


class IncrementalGCTests(unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix crash during garbage collection on an object frozen by :func:`gc.freeze` on the
free-threaded build.
19 changes: 14 additions & 5 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ worklist_remove(struct worklist_iter *iter)
iter->next = iter->ptr;
}

static inline int
gc_is_frozen(PyObject *op)
{
return (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0;
}

static inline int
gc_is_unreachable(PyObject *op)
{
Expand Down Expand Up @@ -277,7 +283,7 @@ op_from_block(void *block, void *arg, bool include_frozen)
if (!_PyObject_GC_IS_TRACKED(op)) {
return NULL;
}
if (!include_frozen && (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0) {
if (!include_frozen && gc_is_frozen(op)) {
return NULL;
}
return op;
Expand Down Expand Up @@ -358,7 +364,7 @@ gc_visit_stackref(_PyStackRef stackref)
// being dead already.
if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNull(stackref)) {
PyObject *obj = PyStackRef_AsPyObjectBorrow(stackref);
if (_PyObject_GC_IS_TRACKED(obj)) {
if (_PyObject_GC_IS_TRACKED(obj) && !gc_is_frozen(obj)) {
gc_add_refs(obj, 1);
}
}
Expand Down Expand Up @@ -439,7 +445,10 @@ process_delayed_frees(PyInterpreterState *interp)
static int
visit_decref(PyObject *op, void *arg)
{
if (_PyObject_GC_IS_TRACKED(op) && !_Py_IsImmortal(op)) {
if (_PyObject_GC_IS_TRACKED(op)
&& !_Py_IsImmortal(op)
&& !gc_is_frozen(op))
{
// If update_refs hasn't reached this object yet, mark it
// as (tentatively) unreachable and initialize ob_tid to zero.
gc_maybe_init_refs(op);
Expand Down Expand Up @@ -1539,7 +1548,7 @@ visit_freeze(const mi_heap_t *heap, const mi_heap_area_t *area,
void *block, size_t block_size, void *args)
{
PyObject *op = op_from_block(block, args, true);
if (op != NULL) {
if (op != NULL && !gc_is_unreachable(op)) {
op->ob_gc_bits |= _PyGC_BITS_FROZEN;
}
return true;
Expand Down Expand Up @@ -1584,7 +1593,7 @@ visit_count_frozen(const mi_heap_t *heap, const mi_heap_area_t *area,
void *block, size_t block_size, void *args)
{
PyObject *op = op_from_block(block, args, true);
if (op != NULL && (op->ob_gc_bits & _PyGC_BITS_FROZEN) != 0) {
if (op != NULL && gc_is_frozen(op)) {
struct count_frozen_args *arg = (struct count_frozen_args *)args;
arg->count++;
}
Expand Down

0 comments on commit d4c72fe

Please sign in to comment.