From fd540aa69e1dcf0d94dfd91b3444f2c26dd44e81 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 12:06:05 -0400 Subject: [PATCH 1/4] gh-133931: Introduce _PyObject_XSetRefDelayed to replace Py_XSETREF --- Include/internal/pycore_pymem.h | 9 +++++++++ Objects/genobject.c | 9 +++++++-- Objects/object.c | 8 +------- Objects/obmalloc.c | 11 +++++++++++ 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 02537bdfef8598..6d6608f0c70618 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -100,6 +100,15 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) } #endif +#ifdef Py_GIL_DISABLED +PyAPI_FUNC(void) _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj); +#else +static inline void _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj) +{ + Py_XSETREF(*p_obj, obj); +} +#endif + // Periodically process delayed free requests. extern void _PyMem_ProcessDelayed(PyThreadState *tstate); diff --git a/Objects/genobject.c b/Objects/genobject.c index 98b2c5004df8ac..b5269d2f5d71a5 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -10,6 +10,7 @@ #include "pycore_gc.h" // _PyGC_CLEAR_FINALIZED() #include "pycore_genobject.h" // _PyGen_SetStopIterationValue() #include "pycore_interpframe.h" // _PyFrame_GetCode() +#include "pycore_pymem.h" // _PyObject_XSetRefDelayed() #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM @@ -718,7 +719,9 @@ gen_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) "__name__ must be set to a string object"); return -1; } - Py_XSETREF(op->gi_name, Py_NewRef(value)); + Py_BEGIN_CRITICAL_SECTION(self); + _PyObject_XSetRefDelayed(&op->gi_name, value); + Py_END_CRITICAL_SECTION(); return 0; } @@ -740,7 +743,9 @@ gen_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) "__qualname__ must be set to a string object"); return -1; } - Py_XSETREF(op->gi_qualname, Py_NewRef(value)); + Py_BEGIN_CRITICAL_SECTION(self); + _PyObject_XSetRefDelayed(&op->gi_qualname, value); + Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/object.c b/Objects/object.c index af1aa217f75462..e707486c7d8a3b 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1931,13 +1931,7 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) return -1; } Py_BEGIN_CRITICAL_SECTION(obj); - PyObject *olddict = *dictptr; - FT_ATOMIC_STORE_PTR_RELEASE(*dictptr, Py_NewRef(value)); -#ifdef Py_GIL_DISABLED - _PyObject_XDecRefDelayed(olddict); -#else - Py_XDECREF(olddict); -#endif + _PyObject_XSetRefDelayed(dictptr, value); Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index b209808da902da..5501b900dc9b79 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1231,6 +1231,17 @@ _PyObject_XDecRefDelayed(PyObject *ptr) } #endif +#ifdef Py_GIL_DISABLED +void +_PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) +{ + PyObject *old = *ptr; + FT_ATOMIC_STORE_PTR_RELEASE(*ptr, Py_NewRef(value)); + _PyObject_XDecRefDelayed(old); + Py_XDECREF(old); +} +#endif + static struct _mem_work_chunk * work_queue_first(struct llist_node *head) { From 9d503b6d3bd5632af0585d9f1c64cd48b17d3e53 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 14:25:48 -0400 Subject: [PATCH 2/4] fix --- Objects/obmalloc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 5501b900dc9b79..9addb4cb47e6b4 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1238,7 +1238,6 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, Py_NewRef(value)); _PyObject_XDecRefDelayed(old); - Py_XDECREF(old); } #endif From 065ea5b9ecdcd3590648f9612602d60c09586085 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 21:36:09 -0400 Subject: [PATCH 3/4] Address code review --- Objects/genobject.c | 4 ++-- Objects/object.c | 2 +- Objects/obmalloc.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index b5269d2f5d71a5..32607d5b5e1504 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -720,7 +720,7 @@ gen_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) return -1; } Py_BEGIN_CRITICAL_SECTION(self); - _PyObject_XSetRefDelayed(&op->gi_name, value); + _PyObject_XSetRefDelayed(&op->gi_name, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; } @@ -744,7 +744,7 @@ gen_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) return -1; } Py_BEGIN_CRITICAL_SECTION(self); - _PyObject_XSetRefDelayed(&op->gi_qualname, value); + _PyObject_XSetRefDelayed(&op->gi_qualname, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/object.c b/Objects/object.c index e707486c7d8a3b..3c4c1a58225985 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1931,7 +1931,7 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) return -1; } Py_BEGIN_CRITICAL_SECTION(obj); - _PyObject_XSetRefDelayed(dictptr, value); + _PyObject_XSetRefDelayed(dictptr, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 9addb4cb47e6b4..bc193053380f2c 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1236,7 +1236,7 @@ void _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; - FT_ATOMIC_STORE_PTR_RELEASE(*ptr, Py_NewRef(value)); + FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); _PyObject_XDecRefDelayed(old); } #endif From 71568140c247238acf1ba83813c2411adc040311 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 21 May 2025 00:48:01 -0400 Subject: [PATCH 4/4] fix --- Objects/obmalloc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index bc193053380f2c..59a0f07a599dd6 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1237,7 +1237,12 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); - _PyObject_XDecRefDelayed(old); + if (!_Py_IsImmortal(old)) { + _PyObject_XDecRefDelayed(old); + } + else { + Py_XDECREF(old); + } } #endif