GeezerVison is showing github.com — [3.13] gh-117657: Fix race involving immortalizing objects (GH-119927… · python/cpython@ae70531
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: Include/internal/pycore_gc.h +5-9Lines changed: 5 additions & 9 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -347,15 +347,11 @@ struct _gc_runtime_state { | 347 | 347 |
| 348 | 348 | /* gh-117783: Deferred reference counting is not fully implemented yet, so | 349 | 349 | as a temporary measure we treat objects using deferred referenence | 350 | | - counting as immortal. */ | 351 | | - struct { | 352 | | - /* Immortalize objects instead of marking them as using deferred | 353 | | - reference counting. */ | 354 | | - int enabled; | 355 | | - | 356 | | - /* Set enabled=1 when the first background thread is created. */ | 357 | | - int enable_on_thread_created; | 358 | | - } immortalize; | | 350 | + counting as immortal. The value may be zero, one, or a negative number: | | 351 | + 0: immortalize deferred RC objects once the first thread is created | | 352 | + 1: immortalize all deferred RC objects immediately | | 353 | + <0: suppressed; don't immortalize objects */ | | 354 | + int immortalize; | 359 | 355 | #endif | 360 | 356 | }; | 361 | 357 |
| |
Collapse file Copy file name to clipboardExpand all lines: Lib/test/support/__init__.py +2-2Lines changed: 2 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -529,11 +529,11 @@ def suppress_immortalization(suppress=True): | 529 | 529 | yield | 530 | 530 | return | 531 | 531 |
| 532 | | - old_values = _testinternalcapi.set_immortalize_deferred(False) | | 532 | + _testinternalcapi.suppress_immortalization(True) | 533 | 533 | try: | 534 | 534 | yield | 535 | 535 | finally: | 536 | | - _testinternalcapi.set_immortalize_deferred(*old_values) | | 536 | + _testinternalcapi.suppress_immortalization(False) | 537 | 537 |
| 538 | 538 | def skip_if_suppress_immortalization(): | 539 | 539 | try: | |
Collapse file Copy file name to clipboardExpand all lines: Modules/_testinternalcapi.c +9-15Lines changed: 9 additions & 15 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1965,32 +1965,26 @@ get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored)) | 1965 | 1965 | #endif | 1966 | 1966 |
| 1967 | 1967 | static PyObject * | 1968 | | -set_immortalize_deferred(PyObject *self, PyObject *value) | | 1968 | +suppress_immortalization(PyObject *self, PyObject *value) | 1969 | 1969 | { | 1970 | 1970 | #ifdef Py_GIL_DISABLED | 1971 | | - PyInterpreterState *interp = PyInterpreterState_Get(); | 1972 | | - int old_enabled = interp->gc.immortalize.enabled; | 1973 | | - int old_enabled_on_thread = interp->gc.immortalize.enable_on_thread_created; | 1974 | | - int enabled_on_thread = 0; | 1975 | | - if (!PyArg_ParseTuple(value, "i|i", | 1976 | | - &interp->gc.immortalize.enabled, | 1977 | | - &enabled_on_thread)) | 1978 | | - { | | 1971 | + int suppress = PyObject_IsTrue(value); | | 1972 | + if (suppress < 0) { | 1979 | 1973 | return NULL; | 1980 | 1974 | } | 1981 | | - interp->gc.immortalize.enable_on_thread_created = enabled_on_thread; | 1982 | | - return Py_BuildValue("ii", old_enabled, old_enabled_on_thread); | 1983 | | -#else | 1984 | | - return Py_BuildValue("OO", Py_False, Py_False); | | 1975 | + PyInterpreterState *interp = PyInterpreterState_Get(); | | 1976 | + // Subtract two to suppress immortalization (so that 1 -> -1) | | 1977 | + _Py_atomic_add_int(&interp->gc.immortalize, suppress ? -2 : 2); | 1985 | 1978 | #endif | | 1979 | + Py_RETURN_NONE; | 1986 | 1980 | } | 1987 | 1981 |
| 1988 | 1982 | static PyObject * | 1989 | 1983 | get_immortalize_deferred(PyObject *self, PyObject *Py_UNUSED(ignored)) | 1990 | 1984 | { | 1991 | 1985 | #ifdef Py_GIL_DISABLED | 1992 | 1986 | PyInterpreterState *interp = PyInterpreterState_Get(); | 1993 | | - return PyBool_FromLong(interp->gc.immortalize.enable_on_thread_created); | | 1987 | + return PyBool_FromLong(_Py_atomic_load_int(&interp->gc.immortalize) >= 0); | 1994 | 1988 | #else | 1995 | 1989 | Py_RETURN_FALSE; | 1996 | 1990 | #endif | @@ -2110,7 +2104,7 @@ static PyMethodDef module_functions[] = { | 2110 | 2104 | #ifdef Py_GIL_DISABLED | 2111 | 2105 | {"py_thread_id", get_py_thread_id, METH_NOARGS}, | 2112 | 2106 | #endif | 2113 | | - {"set_immortalize_deferred", set_immortalize_deferred, METH_VARARGS}, | | 2107 | + {"suppress_immortalization", suppress_immortalization, METH_O}, | 2114 | 2108 | {"get_immortalize_deferred", get_immortalize_deferred, METH_NOARGS}, | 2115 | 2109 | #ifdef _Py_TIER2 | 2116 | 2110 | {"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS}, | |
Collapse file Copy file name to clipboardExpand all lines: Objects/codeobject.c +2-2Lines changed: 2 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -110,7 +110,7 @@ should_intern_string(PyObject *o) | 110 | 110 | // unless we've disabled immortalizing objects that use deferred reference | 111 | 111 | // counting. | 112 | 112 | PyInterpreterState *interp = _PyInterpreterState_GET(); | 113 | | - if (interp->gc.immortalize.enable_on_thread_created) { | | 113 | + if (_Py_atomic_load_int(&interp->gc.immortalize) < 0) { | 114 | 114 | return 1; | 115 | 115 | } | 116 | 116 | #endif | @@ -240,7 +240,7 @@ intern_constants(PyObject *tuple, int *modified) | 240 | 240 | PyThreadState *tstate = PyThreadState_GET(); | 241 | 241 | if (!_Py_IsImmortal(v) && !PyCode_Check(v) && | 242 | 242 | !PyUnicode_CheckExact(v) && | 243 | | - tstate->interp->gc.immortalize.enable_on_thread_created) | | 243 | + _Py_atomic_load_int(&tstate->interp->gc.immortalize) >= 0) | 244 | 244 | { | 245 | 245 | PyObject *interned = intern_one_constant(v); | 246 | 246 | if (interned == NULL) { | |
Collapse file Copy file name to clipboardExpand all lines: Objects/object.c +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -2433,7 +2433,7 @@ _PyObject_SetDeferredRefcount(PyObject *op) | 2433 | 2433 | assert(op->ob_ref_shared == 0); | 2434 | 2434 | _PyObject_SET_GC_BITS(op, _PyGC_BITS_DEFERRED); | 2435 | 2435 | PyInterpreterState *interp = _PyInterpreterState_GET(); | 2436 | | - if (interp->gc.immortalize.enabled) { | | 2436 | + if (_Py_atomic_load_int_relaxed(&interp->gc.immortalize) == 1) { | 2437 | 2437 | // gh-117696: immortalize objects instead of using deferred reference | 2438 | 2438 | // counting for now. | 2439 | 2439 | _Py_SetImmortal(op); | |
Collapse file Copy file name to clipboardExpand all lines: Python/bltinmodule.c +3-3Lines changed: 3 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -870,15 +870,15 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, | 870 | 870 | // gh-118527: Disable immortalization of code constants for explicit | 871 | 871 | // compile() calls to get consistent frozen outputs between the default | 872 | 872 | // and free-threaded builds. | | 873 | + // Subtract two to suppress immortalization (so that 1 -> -1) | 873 | 874 | PyInterpreterState *interp = _PyInterpreterState_GET(); | 874 | | - int old_value = interp->gc.immortalize.enable_on_thread_created; | 875 | | - interp->gc.immortalize.enable_on_thread_created = 0; | | 875 | + _Py_atomic_add_int(&interp->gc.immortalize, -2); | 876 | 876 | #endif | 877 | 877 |
| 878 | 878 | result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize); | 879 | 879 |
| 880 | 880 | #ifdef Py_GIL_DISABLED | 881 | | - interp->gc.immortalize.enable_on_thread_created = old_value; | | 881 | + _Py_atomic_add_int(&interp->gc.immortalize, 2); | 882 | 882 | #endif | 883 | 883 |
| 884 | 884 | Py_XDECREF(source_copy); | |
Collapse file Copy file name to clipboardExpand all lines: Python/gc_free_threading.c +7-7Lines changed: 7 additions & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -703,11 +703,9 @@ _PyGC_Init(PyInterpreterState *interp) | 703 | 703 | { | 704 | 704 | GCState *gcstate = &interp->gc; | 705 | 705 |
| 706 | | - if (_Py_IsMainInterpreter(interp)) { | 707 | | - // gh-117783: immortalize objects that would use deferred refcounting | 708 | | - // once the first non-main thread is created. | 709 | | - gcstate->immortalize.enable_on_thread_created = 1; | 710 | | - } | | 706 | + // gh-117783: immortalize objects that would use deferred refcounting | | 707 | + // once the first non-main thread is created (but not in subinterpreters). | | 708 | + gcstate->immortalize = _Py_IsMainInterpreter(interp) ? 0 : -1; | 711 | 709 |
| 712 | 710 | gcstate->garbage = PyList_New(0); | 713 | 711 | if (gcstate->garbage == NULL) { | @@ -1808,8 +1806,10 @@ _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp) | 1808 | 1806 | { | 1809 | 1807 | struct visitor_args args; | 1810 | 1808 | _PyEval_StopTheWorld(interp); | 1811 | | - gc_visit_heaps(interp, &immortalize_visitor, &args); | 1812 | | - interp->gc.immortalize.enabled = 1; | | 1809 | + if (interp->gc.immortalize == 0) { | | 1810 | + gc_visit_heaps(interp, &immortalize_visitor, &args); | | 1811 | + interp->gc.immortalize = 1; | | 1812 | + } | 1813 | 1813 | _PyEval_StartTheWorld(interp); | 1814 | 1814 | } | 1815 | 1815 |
| |
Collapse file Copy file name to clipboardExpand all lines: Python/pystate.c +1-3Lines changed: 1 addition & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1583,9 +1583,7 @@ new_threadstate(PyInterpreterState *interp, int whence) | 1583 | 1583 | } | 1584 | 1584 | else { | 1585 | 1585 | #ifdef Py_GIL_DISABLED | 1586 | | - if (interp->gc.immortalize.enable_on_thread_created && | 1587 | | - !interp->gc.immortalize.enabled) | 1588 | | - { | | 1586 | + if (_Py_atomic_load_int(&interp->gc.immortalize) == 0) { | 1589 | 1587 | // Immortalize objects marked as using deferred reference counting | 1590 | 1588 | // the first time a non-main thread is created. | 1591 | 1589 | _PyGC_ImmortalizeDeferredObjects(interp); | |
Collapse file Copy file name to clipboardExpand all lines: Tools/tsan/suppressions_free_threading.txt -2Lines changed: 0 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -47,7 +47,6 @@ race_top:_PyImport_AcquireLock | 47 | 47 | race_top:_Py_dict_lookup_threadsafe | 48 | 48 | race_top:_imp_release_lock | 49 | 49 | race_top:_multiprocessing_SemLock_acquire_impl | 50 | | -race_top:builtin_compile_impl | 51 | 50 | race_top:count_next | 52 | 51 | race_top:dictiter_new | 53 | 52 | race_top:dictresize | @@ -56,7 +55,6 @@ race_top:insertdict | 56 | 55 | race_top:list_get_item_ref | 57 | 56 | race_top:make_pending_calls | 58 | 57 | race_top:set_add_entry | 59 | | -race_top:should_intern_string | 60 | 58 | race_top:_Py_slot_tp_getattr_hook | 61 | 59 | race_top:add_threadstate | 62 | 60 | race_top:dump_traceback | |
|