Behavior change in assertValid in free threaded python

[astrelsky]

Small discussion: 7c31d02#r149329647

Py_REFCNT expands to the following in free-threaded Python, builds correctly and works as expected. If it didn't build, then I suspect there was something else wrong.

static inline Py_ssize_t Py_REFCNT(PyObject *ob) {
#if !defined(Py_GIL_DISABLED)
    return ob->ob_refcnt;
#else
    uint32_t local = _Py_atomic_load_uint32_relaxed(&ob->ob_ref_local);
    if (local == _Py_IMMORTAL_REFCNT_LOCAL) {
        return _Py_IMMORTAL_REFCNT;
    }
    Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&ob->ob_ref_shared);
    return _Py_STATIC_CAST(Py_ssize_t, local) +
           Py_ARITHMETIC_RIGHT_SHIFT(Py_ssize_t, shared, _Py_REF_SHARED_SHIFT);
#endif
}
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
#  define Py_REFCNT(ob) Py_REFCNT(_PyObject_CAST(ob))
#endif

While I've only encountered it during development, it is protecting against a use-after-free. In the linked discussion I meantioned that I have encountered the error before, but I fixed the cause of that yesterday. Still, I think the check should be enabled in both normal and free-threaded.

[marscher]

I agree, that this should be treated in a better way. Is your snipped from refcount.h?

https://github.com/python/cpython/blob/b83be9c9718aac42d0d8fc689a829d6594192afa/Include/refcount.h#L93

Can you isolate a piece of code which triggers this use-after free behavior, so we can come up with a regression test?

[astrelsky]

I agree, that this should be treated in a better way. Is your snipped from refcount.h?

https://github.com/python/cpython/blob/b83be9c9718aac42d0d8fc689a829d6594192afa/Include/refcount.h#L93

Can you isolate a piece of code which triggers this use-after free behavior, so we can come up with a regression test?

The times I had actually encountered it was during development and I have since isolated and fixed the problem. I don't currently know of a way to hit that code path. However, it is better to fail and raise the exception then continue to use an object that has been collected, or soon will be collected, by the garbage collector.

Yes, the code snippet was from refcount.h