review

dpdani · Jun 5, 2024 · 99b1bce · 99b1bce
1 parent 79396ce
commit 99b1bce
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 32 deletions.
diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h
@@ -59,8 +59,6 @@ extern "C" {
     _Py_atomic_store_uint16_relaxed(&value, new_value)
 #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \
     _Py_atomic_store_uint32_relaxed(&value, new_value)
-#define FT_ATOMIC_EXCHANGE_PTR(value, new_value) \
-    _Py_atomic_exchange_ptr(value, new_value)
 
 #else
 #define FT_ATOMIC_LOAD_PTR(value) value
@@ -85,17 +83,6 @@ extern "C" {
 #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
 #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value
 
-static inline void *
-_Py_non_atomic_exchange_ptr(void **value, void *new_value)
-{
-    void *current = *value;
-    *value = new_value;
-    return current;
-}
-
-#define FT_ATOMIC_EXCHANGE_PTR(value, new_value) \
-    _Py_non_atomic_exchange_ptr(value, new_value)
-
 #endif
 
 #ifdef __cplusplus

diff --git a/Python/structmember.c b/Python/structmember.c
@@ -4,13 +4,12 @@
 #include "Python.h"
 #include "pycore_abstract.h"      // _PyNumber_Index()
 #include "pycore_long.h"          // _PyLong_IsNegative()
-#include "pycore_pyatomic_ft_wrappers.h"
-#include "pycore_object.h"
+#include "pycore_object.h"        // _Py_TryIncrefCompare(), FT_ATOMIC_*()
 #include "pycore_critical_section.h"
 
 
 static inline PyObject *
-_PyMember_GetOneObject(const char *addr, const char *obj_addr, PyMemberDef *l)
+member_get_object(const char *addr, const char *obj_addr, PyMemberDef *l)
 {
     PyObject *v = FT_ATOMIC_LOAD_PTR(*(PyObject **) addr);
     if (v == NULL) {
@@ -23,9 +22,6 @@ _PyMember_GetOneObject(const char *addr, const char *obj_addr, PyMemberDef *l)
     return v;
 }
 
-
-
-
 PyObject *
 PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
 {
@@ -96,16 +92,18 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
         break;
     case Py_T_OBJECT_EX:
 #ifndef Py_GIL_DISABLED
-        v = _PyMember_GetOneObject(addr, obj_addr, l);
+        v = member_get_object(addr, obj_addr, l);
         Py_XINCREF(v);
 #else
-            v = _PyMember_GetOneObject(addr, obj_addr, l);
-            if (v != NULL && !_Py_TryIncrefCompare((PyObject **) addr, v)) {
-                Py_BEGIN_CRITICAL_SECTION((PyObject *)obj_addr);
-                        v = _PyMember_GetOneObject(addr, obj_addr, l);
-                        Py_XINCREF(v);
+        v = member_get_object(addr, obj_addr, l);
+        if (v != NULL) {
+            if (!_Py_TryIncrefCompare((PyObject **) addr, v)) {
+                Py_BEGIN_CRITICAL_SECTION((PyObject *) obj_addr);
+                v = member_get_object(addr, obj_addr, l);
+                Py_XINCREF(v);
                 Py_END_CRITICAL_SECTION();
             }
+        }
 #endif
         break;
     case Py_T_LONGLONG:
@@ -142,7 +140,9 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         return -1;
     }
 
+#ifdef Py_GIL_DISABLED
     PyObject *obj = (PyObject *) addr;
+#endif
     addr += l->offset;
 
     if ((l->flags & Py_READONLY))
@@ -306,14 +306,10 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         break;
     case _Py_T_OBJECT:
     case Py_T_OBJECT_EX:
-#ifdef Py_GIL_DISABLED
         Py_BEGIN_CRITICAL_SECTION(obj);
-#endif
-        oldv = FT_ATOMIC_LOAD_PTR(*(PyObject **)addr);
-        FT_ATOMIC_STORE_PTR(*(PyObject **)addr, Py_XNewRef(v));
-#ifdef Py_GIL_DISABLED
+        oldv = *(PyObject **)addr;
+        FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v));
         Py_END_CRITICAL_SECTION();
-#endif
         Py_XDECREF(oldv);
         break;
     case Py_T_CHAR: {

diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt
@@ -32,7 +32,6 @@ race_top:_PyType_HasFeature
 race_top:assign_version_tag
 race_top:insertdict
 race_top:lookup_tp_dict
-race_top:mi_heap_visit_pages
 race_top:new_reference
 race_top:set_contains_key
 # https://gist.github.com/colesbury/d13d033f413b4ad07929d044bed86c35