Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd23be39

Browse files
authored
gh-121794: Don't setob_tid to zero in fast-path dealloc (#121799)
We should maintain the invariant that a zero `ob_tid` implies therefcount fields are merged.* Move the assignment in `_Py_MergeZeroLocalRefcount` to immediately before the refcount merge.* Update `_PyTrash_thread_destroy_chain` to set `ob_ref_shared` to `_Py_REF_MERGED` when setting `ob_tid` to zero.Also check this invariant with assertions in the GC in debug builds.That uncovered a bug when running out of memory during GC.
1 parent82a4dac commitd23be39

File tree

3 files changed

+62
-13
lines changed

3 files changed

+62
-13
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix bug in free-threaded Python where a resurrected object could lead to
2+
a negative ref count assertion failure.

‎Objects/object.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,14 +375,18 @@ _Py_MergeZeroLocalRefcount(PyObject *op)
375375
{
376376
assert(op->ob_ref_local==0);
377377

378-
_Py_atomic_store_uintptr_relaxed(&op->ob_tid,0);
379378
Py_ssize_tshared=_Py_atomic_load_ssize_acquire(&op->ob_ref_shared);
380379
if (shared==0) {
381380
// Fast-path: shared refcount is zero (including flags)
382381
_Py_Dealloc(op);
383382
return;
384383
}
385384

385+
// gh-121794: This must be before the store to `ob_ref_shared` (gh-119999),
386+
// but should outside the fast-path to maintain the invariant that
387+
// a zero `ob_tid` implies a merged refcount.
388+
_Py_atomic_store_uintptr_relaxed(&op->ob_tid,0);
389+
386390
// Slow-path: atomically set the flags (low two bits) to _Py_REF_MERGED.
387391
Py_ssize_tnew_shared;
388392
do {
@@ -2724,7 +2728,6 @@ _PyTrash_thread_deposit_object(PyThreadState *tstate, PyObject *op)
27242728
_PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op));
27252729
_PyObject_ASSERT(op,Py_REFCNT(op)==0);
27262730
#ifdefPy_GIL_DISABLED
2727-
_PyObject_ASSERT(op,op->ob_tid==0);
27282731
op->ob_tid= (uintptr_t)tstate->delete_later;
27292732
#else
27302733
_PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)tstate->delete_later);
@@ -2757,6 +2760,7 @@ _PyTrash_thread_destroy_chain(PyThreadState *tstate)
27572760
#ifdefPy_GIL_DISABLED
27582761
tstate->delete_later= (PyObject*)op->ob_tid;
27592762
op->ob_tid=0;
2763+
_Py_atomic_store_ssize_relaxed(&op->ob_ref_shared,_Py_REF_MERGED);
27602764
#else
27612765
tstate->delete_later= (PyObject*)_PyGCHead_PREV(_Py_AS_GC(op));
27622766
#endif

‎Python/gc_free_threading.c‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,30 @@ mark_reachable(PyObject *op)
455455
}
456456

457457
#ifdefGC_DEBUG
458+
staticbool
459+
validate_refcounts(constmi_heap_t*heap,constmi_heap_area_t*area,
460+
void*block,size_tblock_size,void*args)
461+
{
462+
PyObject*op=op_from_block(block,args, false);
463+
if (op==NULL) {
464+
return true;
465+
}
466+
467+
_PyObject_ASSERT_WITH_MSG(op, !gc_is_unreachable(op),
468+
"object should not be marked as unreachable yet");
469+
470+
if (_Py_REF_IS_MERGED(op->ob_ref_shared)) {
471+
_PyObject_ASSERT_WITH_MSG(op,op->ob_tid==0,
472+
"merged objects should have ob_tid == 0");
473+
}
474+
elseif (!_Py_IsImmortal(op)) {
475+
_PyObject_ASSERT_WITH_MSG(op,op->ob_tid!=0,
476+
"unmerged objects should have ob_tid != 0");
477+
}
478+
479+
return true;
480+
}
481+
458482
staticbool
459483
validate_gc_objects(constmi_heap_t*heap,constmi_heap_area_t*area,
460484
void*block,size_tblock_size,void*args)
@@ -498,6 +522,19 @@ mark_heap_visitor(const mi_heap_t *heap, const mi_heap_area_t *area,
498522
return true;
499523
}
500524

525+
staticbool
526+
restore_refs(constmi_heap_t*heap,constmi_heap_area_t*area,
527+
void*block,size_tblock_size,void*args)
528+
{
529+
PyObject*op=op_from_block(block,args, false);
530+
if (op==NULL) {
531+
return true;
532+
}
533+
gc_restore_tid(op);
534+
gc_clear_unreachable(op);
535+
return true;
536+
}
537+
501538
/* Return true if object has a pre-PEP 442 finalization method. */
502539
staticint
503540
has_legacy_finalizer(PyObject*op)
@@ -549,6 +586,13 @@ static int
549586
deduce_unreachable_heap(PyInterpreterState*interp,
550587
structcollection_state*state)
551588
{
589+
590+
#ifdefGC_DEBUG
591+
// Check that all objects are marked as unreachable and that the computed
592+
// reference count difference (stored in `ob_tid`) is non-negative.
593+
gc_visit_heaps(interp,&validate_refcounts,&state->base);
594+
#endif
595+
552596
// Identify objects that are directly reachable from outside the GC heap
553597
// by computing the difference between the refcount and the number of
554598
// incoming references.
@@ -563,6 +607,8 @@ deduce_unreachable_heap(PyInterpreterState *interp,
563607
// Transitively mark reachable objects by clearing the
564608
// _PyGC_BITS_UNREACHABLE flag.
565609
if (gc_visit_heaps(interp,&mark_heap_visitor,&state->base)<0) {
610+
// On out-of-memory, restore the refcounts and bail out.
611+
gc_visit_heaps(interp,&restore_refs,&state->base);
566612
return-1;
567613
}
568614

@@ -1066,7 +1112,8 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
10661112
interr=deduce_unreachable_heap(interp,state);
10671113
if (err<0) {
10681114
_PyEval_StartTheWorld(interp);
1069-
gotoerror;
1115+
PyErr_NoMemory();
1116+
return;
10701117
}
10711118

10721119
// Print debugging information.
@@ -1100,7 +1147,12 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
11001147
_PyEval_StartTheWorld(interp);
11011148

11021149
if (err<0) {
1103-
gotoerror;
1150+
cleanup_worklist(&state->unreachable);
1151+
cleanup_worklist(&state->legacy_finalizers);
1152+
cleanup_worklist(&state->wrcb_to_call);
1153+
cleanup_worklist(&state->objs_to_decref);
1154+
PyErr_NoMemory();
1155+
return;
11041156
}
11051157

11061158
// Call tp_clear on objects in the unreachable set. This will cause
@@ -1110,15 +1162,6 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state,
11101162

11111163
// Append objects with legacy finalizers to the "gc.garbage" list.
11121164
handle_legacy_finalizers(state);
1113-
return;
1114-
1115-
error:
1116-
cleanup_worklist(&state->unreachable);
1117-
cleanup_worklist(&state->legacy_finalizers);
1118-
cleanup_worklist(&state->wrcb_to_call);
1119-
cleanup_worklist(&state->objs_to_decref);
1120-
PyErr_NoMemory();
1121-
PyErr_FormatUnraisable("Out of memory during garbage collection");
11221165
}
11231166

11241167
/* This is the main function. Read this to understand how the

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp