From b915df3747f65becaa3a3fd354c5120425cbdbc2 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 12 Mar 2025 09:14:00 +0000 Subject: [PATCH 1/3] Fix _Py_RefcntAdd to handle objects becoming immortal --- Include/internal/pycore_object.h | 31 +++++++------------------------ Include/refcount.h | 3 ++- Objects/unicodeobject.c | 2 +- 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 1c4c3e30fd2f6d..972f042b699ddd 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -135,31 +135,14 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n) _Py_INCREF_IMMORTAL_STAT_INC(); return; } -#ifdef Py_REF_DEBUG - _Py_AddRefTotal(_PyThreadState_GET(), n); -#endif -#if !defined(Py_GIL_DISABLED) -#if SIZEOF_VOID_P > 4 - op->ob_refcnt += (PY_UINT32_T)n; -#else - op->ob_refcnt += n; -#endif -#else - if (_Py_IsOwnedByCurrentThread(op)) { - uint32_t local = op->ob_ref_local; - Py_ssize_t refcnt = (Py_ssize_t)local + n; -# if PY_SSIZE_T_MAX > UINT32_MAX - if (refcnt > (Py_ssize_t)UINT32_MAX) { - // Make the object immortal if the 32-bit local reference count - // would overflow. - refcnt = _Py_IMMORTAL_REFCNT_LOCAL; - } -# endif - _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, (uint32_t)refcnt); - } - else { - _Py_atomic_add_ssize(&op->ob_ref_shared, (n << _Py_REF_SHARED_SHIFT)); + Py_ssize_t refcnt = _Py_REFCNT(op); + Py_ssize_t new_refcnt = refcnt + n; + if (new_refcnt >= (Py_ssize_t)_Py_IMMORTAL_MINIMUM_REFCNT) { + new_refcnt = _Py_IMMORTAL_INITIAL_REFCNT; } + Py_SET_REFCNT(op, new_refcnt); +#ifdef Py_REF_DEBUG + _Py_AddRefTotal(_PyThreadState_GET(), new_refcnt - refcnt); #endif // Although the ref count was increased by `n` (which may be greater than 1) // it is only a single increment (i.e. addition) operation, so only 1 refcnt diff --git a/Include/refcount.h b/Include/refcount.h index e66e4aaace3140..ba14bc6965ce3e 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -42,7 +42,8 @@ beyond the refcount limit. Immortality checks for reference count decreases will be done by checking the bit sign flag in the lower 32 bits. */ -#define _Py_IMMORTAL_INITIAL_REFCNT (3UL << 30) +#define _Py_IMMORTAL_INITIAL_REFCNT (3ULL << 30) +#define _Py_IMMORTAL_MINIMUM_REFCNT (1ULL << 31) #define _Py_STATIC_FLAG_BITS ((Py_ssize_t)(_Py_STATICALLY_ALLOCATED_FLAG | _Py_IMMORTAL_FLAGS)) #define _Py_STATIC_IMMORTAL_INITIAL_REFCNT (((Py_ssize_t)_Py_IMMORTAL_INITIAL_REFCNT) | (_Py_STATIC_FLAG_BITS << 48)) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 21ccb01f86bc61..085944cd6bd989 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15982,7 +15982,7 @@ _PyUnicode_ClearInterned(PyInterpreterState *interp) case SSTATE_INTERNED_MORTAL: // Restore 2 references held by the interned dict; these will // be decref'd by clear_interned_dict's PyDict_Clear. - Py_SET_REFCNT(s, Py_REFCNT(s) + 2); + _Py_RefcntAdd(s, 2); #ifdef Py_REF_DEBUG /* let's be pedantic with the ref total */ _Py_IncRefTotal(_PyThreadState_GET()); From c4bf588e38c5296804eccbe2ac0990976b86b5e1 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 12 Mar 2025 09:51:48 +0000 Subject: [PATCH 2/3] Restore no-gil behavior --- Include/internal/pycore_object.h | 29 +++++++++++++++++++++++++++-- Include/refcount.h | 3 +++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 972f042b699ddd..0b686b416ec193 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -135,14 +135,39 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n) _Py_INCREF_IMMORTAL_STAT_INC(); return; } +#ifndef Py_GIL_DISABLED Py_ssize_t refcnt = _Py_REFCNT(op); Py_ssize_t new_refcnt = refcnt + n; if (new_refcnt >= (Py_ssize_t)_Py_IMMORTAL_MINIMUM_REFCNT) { new_refcnt = _Py_IMMORTAL_INITIAL_REFCNT; } - Py_SET_REFCNT(op, new_refcnt); -#ifdef Py_REF_DEBUG +# if SIZEOF_VOID_P > 4 + op->ob_refcnt = (PY_UINT32_T)new_refcnt; +# else + op->ob_refcnt = new_refcnt; +# endif +# ifdef Py_REF_DEBUG _Py_AddRefTotal(_PyThreadState_GET(), new_refcnt - refcnt); +# endif +#else + if (_Py_IsOwnedByCurrentThread(op)) { + uint32_t local = op->ob_ref_local; + Py_ssize_t refcnt = (Py_ssize_t)local + n; +# if PY_SSIZE_T_MAX > UINT32_MAX + if (refcnt > (Py_ssize_t)UINT32_MAX) { + // Make the object immortal if the 32-bit local reference count + // would overflow. + refcnt = _Py_IMMORTAL_REFCNT_LOCAL; + } +# endif + _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, (uint32_t)refcnt); + } + else { + _Py_atomic_add_ssize(&op->ob_ref_shared, (n << _Py_REF_SHARED_SHIFT)); + } +# ifdef Py_REF_DEBUG + _Py_AddRefTotal(_PyThreadState_GET(), n); +# endif #endif // Although the ref count was increased by `n` (which may be greater than 1) // it is only a single increment (i.e. addition) operation, so only 1 refcnt diff --git a/Include/refcount.h b/Include/refcount.h index ba14bc6965ce3e..3740045480a17f 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -158,6 +158,9 @@ static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { return; } #ifndef Py_GIL_DISABLED + if (refcnt >= (Py_ssize_t)_Py_IMMORTAL_MINIMUM_REFCNT) { + refcnt = _Py_IMMORTAL_INITIAL_REFCNT; + } #if SIZEOF_VOID_P > 4 ob->ob_refcnt = (PY_UINT32_T)refcnt; #else From 13d5be0c5f1552c6137eac579cd793ea28a67adb Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 12 Mar 2025 10:17:53 +0000 Subject: [PATCH 3/3] Remove redundant check --- Include/refcount.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/Include/refcount.h b/Include/refcount.h index 3740045480a17f..ba14bc6965ce3e 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -158,9 +158,6 @@ static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { return; } #ifndef Py_GIL_DISABLED - if (refcnt >= (Py_ssize_t)_Py_IMMORTAL_MINIMUM_REFCNT) { - refcnt = _Py_IMMORTAL_INITIAL_REFCNT; - } #if SIZEOF_VOID_P > 4 ob->ob_refcnt = (PY_UINT32_T)refcnt; #else