gh-154044: Fix data race on descriptor __qualname__ in FT build

[deadlovelll]

Fix data race on descriptor __qualname__ in FT build

For more details see gh-154044

[brijkapadia]

Not needed

Suggested change

#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_ACQUIRE()

[deadlovelll]

I think it's implicit, the rest of the codebase includes it explicitly when using FT_ATOMIC_*, so Its better leave it

[brijkapadia]

I think you can get away without using a critical section. Something like this:

static PyObject *
descr_get_qualname(PyObject *self, void *Py_UNUSED(ignored))
{
    PyDescrObject *descr = (PyDescrObject *)self;
    PyObject *qualname = FT_ATOMIC_LOAD_PTR_ACQUIRE(descr->d_qualname);
    if (qualname == NULL) {
        qualname = calculate_qualname(descr);
#ifdef Py_GIL_DISABLED
        PyObject *expected = NULL;
        if (!_Py_atomic_compare_exchange_ptr(&descr->d_qualname, &expected, qualname)) {
            Py_DECREF(qualname);
            qualname = expected;
        }
#else
        descr->d_qualname = qualname;
#endif
    }
    return Py_NewRef(qualname);
}

[deadlovelll]

CAS is a valid solution, but id prefer CS to avoid #ifdef and keep a single compute.

But your snippet contains UB, because if calculate_qualname fails it will return NULL and code will try to:

1 Py_DECREF the NULL after CAS
2 Py_NewRef on NULL

[deadlovelll]

BTW thanks for the review! Means a lot

[brijkapadia]

Do wee need to test all of these types?

[deadlovelll]

Do wee need to test all of these types?

Yes. These mirror the builtin types from the issue's reproducer, so the test reproduces the exact reported case