Data race in `weakref.getweakrefs()` under free-threading

[Naserume]

Bug report

Bug description:

weakref.getweakrefs(obj) reads Py_TYPE(obj) and the weakref list head without keeping obj alive against concurrent deallocation. In a free-threaded build, if another thread drops the last reference to that object concurrently, getweakrefs reads freed memory.

Reproducer:

import weakref
from threading import Thread

class Target:
    pass

obj = Target()
weakref.ref(obj)

def mutator():
    global obj
    for _ in range(20000):
        t = Target()
        weakref.ref(t)
        obj = t

def reader():
    for _ in range(20000):
        weakref.getweakrefs(obj)

threads  = [Thread(target=mutator)  for _ in range(1)]
threads += [Thread(target=reader)  for _ in range(8)]
for t in threads: t.start()
for t in threads: t.join()

TSAN Report:

==================
WARNING: ThreadSanitizer: data race (pid=1534965)
  Read of size 8 at 0x7fffb8190208 by thread T4:
    #0 _weakref_getweakrefs /cpython/./Modules/_weakref.c:78:36 
    #1 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:2612:35 
    #2 _PyEval_EvalFrame /cpython/./Include/internal/pycore_ceval.h:122:16 
    #3 _PyEval_Vector /cpython/Python/ceval.c:2142:12 
    #4 _PyFunction_Vectorcall /cpython/Objects/call.c 
    #5 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:144:11 
    #6 _PyObject_VectorcallPrepend /cpython/Objects/call.c:855:20 
    #7 method_vectorcall /cpython/Objects/classobject.c:55:12 
    #8 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:144:11 
    #9 context_run /cpython/Python/context.c:728:29 
    #10 method_vectorcall_FASTCALL_KEYWORDS /cpython/Objects/descrobject.c:421:24 

  Previous write of size 8 at 0x7fffb8190208 by thread T1:
    #0 _PyType_AllocNoTrack /cpython/Objects/typeobject.c 
    #1 PyType_GenericAlloc /cpython/Objects/typeobject.c:2554:21 
    #2 object_new /cpython/Objects/typeobject.c:7453:21 
    #3 type_call /cpython/Objects/typeobject.c:2467:11 
    #4 _PyObject_MakeTpCall /cpython/Objects/call.c:242:18 
    #5 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:142:16 
    #6 PyObject_Vectorcall /cpython/Objects/call.c:327:12 
    #7 _Py_VectorCall_StackRefSteal /cpython/Python/ceval.c:724:11 
    #8 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:4362:35 
    #9 _PyEval_EvalFrame /cpython/./Include/internal/pycore_ceval.h:122:16 

SUMMARY: ThreadSanitizer: data race /cpython/./Modules/_weakref.c:78:36 in _weakref_getweakrefs
==================

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

[kumaraditya303]

I think this is the same issue as #142534.

The LOAD_ATTR specializations for modules and builtins use relaxed order to load the value whereas it should use atleast consume memory order so that TSAN doesn't flag the write of type pointer while allocating the object as a race.

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 993d2317514..a718f0be046 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2349,7 +2349,7 @@ dummy_func(
             assert(keys->dk_kind == DICT_KEYS_UNICODE);
             PyDictUnicodeEntry *entries = DK_UNICODE_ENTRIES(keys);
             assert(index < DK_SIZE(keys));
-            PyObject *res_o = FT_ATOMIC_LOAD_PTR_RELAXED(entries[index].me_value);
+            PyObject *res_o = FT_ATOMIC_LOAD_PTR_ACQUIRE(entries[index].me_value);
             DEOPT_IF(res_o == NULL);
             #if Py_GIL_DISABLED
             int increfed = _Py_TryIncrefCompareStackRef(&entries[index].me_value, res_o, &res);
@@ -2368,7 +2368,7 @@ dummy_func(
             DEOPT_IF(FT_ATOMIC_LOAD_UINT32_RELAXED(keys->dk_version) != version);
             assert(keys->dk_kind == DICT_KEYS_UNICODE);
             PyDictUnicodeEntry *entries = DK_UNICODE_ENTRIES(keys);
-            PyObject *res_o = FT_ATOMIC_LOAD_PTR_RELAXED(entries[index].me_value);
+            PyObject *res_o = FT_ATOMIC_LOAD_PTR_ACQUIRE(entries[index].me_value);
             DEOPT_IF(res_o == NULL);
             #if Py_GIL_DISABLED
             int increfed = _Py_TryIncrefCompareStackRef(&entries[index].me_value, res_o, &res);
@@ -2958,7 +2958,7 @@ dummy_func(
             assert(keys->dk_kind == DICT_KEYS_UNICODE);
             assert(index < FT_ATOMIC_LOAD_SSIZE_RELAXED(keys->dk_nentries));
             PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(keys) + index;
-            PyObject *attr_o = FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_value);
+            PyObject *attr_o = FT_ATOMIC_LOAD_PTR_ACQUIRE(ep->me_value);
             EXIT_IF(attr_o == NULL);
             #ifdef Py_GIL_DISABLED
             int increfed = _Py_TryIncrefCompareStackRef(&ep->me_value, attr_o, &attr);

With the above change I don't see data races with the reproducer.