Data race in `_PyType_AllocNoTrack` in free threaded build

[colesbury]

Bug report

Seen in https://github.com/python/cpython/actions/runs/13272883082/job/37056243083?pr=130015

WARNING: ThreadSanitizer: data race (pid=17248)
  Write of size 8 at 0x7fbca00601d0 by thread T2652:
    #0 __tsan_memset <null> (python+0xdc7a8) (BuildId: cba02c45a3623f17982fb0d328c59833[13](https://github.com/python/cpython/actions/runs/13272883082/job/37056243083?pr=130015#step:14:14)92b589)
    #1 _PyType_AllocNoTrack /home/runner/work/cpython/cpython/Objects/typeobject.c:2251:5 (python+0x37dd92) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #2 PyType_GenericAlloc /home/runner/work/cpython/cpython/Objects/typeobject.c:2268:21 (python+0x37daf0) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #3 PyType_GenericNew /home/runner/work/cpython/cpython/Objects/typeobject.c:2282:12 (python+0x37e1a8) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #4 type_call /home/runner/work/cpython/cpython/Objects/typeobject.c:2183:11 (python+0x386be7) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #5 _PyObject_MakeTpCall /home/runner/work/cpython/cpython/Objects/call.c:242:18 (python+0x24ee43) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #6 _PyObject_VectorcallTstate /home/runner/work/cpython/cpython/./Include/internal/pycore_call.h:[16](https://github.com/python/cpython/actions/runs/13272883082/job/37056243083?pr=130015#step:14:17)5:16 (python+0x24e70b) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
...

Previous atomic read of size 8 at 0x7fbca00601d0 by thread T2651:
    #0 _Py_atomic_load_uintptr_relaxed /home/runner/work/cpython/cpython/./Include/cpython/pyatomic_gcc.h:375:10 (python+0x382bea) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
    #1 _Py_IsOwnedByCurrentThread /home/runner/work/cpython/cpython/./Include/object.h:252:12 (python+0x382bea)
    #2 _Py_TryIncrefFast /home/runner/work/cpython/cpython/./Include/internal/pycore_object.h:560:9 (python+0x382bea)
    #3 _Py_TryIncref /home/runner/work/cpython/cpython/./Include/internal/pycore_object.h:764:12 (python+0x382bea)
    #4 _PyType_LookupRefAndVersion /home/runner/work/cpython/cpython/Objects/typeobject.c:5567:34 (python+0x382bea)
    #5 _PyType_LookupRef /home/runner/work/cpython/cpython/Objects/typeobject.c:5659:12 (python+0x37e522) (BuildId: cba02c[45](https://github.com/python/cpython/actions/runs/13272883082/job/37056243083?pr=130015#step:14:46)a3623f17982fb0d328c598331392b589)
    #6 _PyObject_GenericSetAttrWithDict /home/runner/work/cpython/cpython/Objects/object.c:1805:13 (python+0x3257d9) (BuildId: cba02c45a3623f17982fb0d328c598331392b589)
...

_PyType_AllocNoTrack() zeroes out the the allocation, including reference count fields. The memset is not atomic and so can race with _Py_TryIncref or similar function.

I think we should memset() on the data after the PyObject header. The ob_type and reference count fields are immediately initialized after the memset anyways by _PyObject_Init or _PyObject_InitVar.

cpython/Objects/typeobject.c

Lines 2248 to 2262 in ed816f1

	if (PyType_IS_GC(type)) {
	_PyObject_GC_Link(obj);
	}
	memset(obj, '\0', size);
	if (type->tp_itemsize == 0) {
	_PyObject_Init(obj, type);
	}
	else {
	_PyObject_InitVar((PyVarObject *)obj, type, nitems);
	}
	if (type->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
	_PyObject_InitInlineValues(obj, type);
	}
	return obj;

Linked PRs

• gh-130019: Fix data race in _PyType_AllocNoTrack #130058

[colesbury]

Also probably PyUnstable_Object_GC_NewWithExtraData:

cpython/Python/gc_free_threading.c

Lines 2592 to 2603 in ed816f1

	PyObject *
	PyUnstable_Object_GC_NewWithExtraData(PyTypeObject *tp, size_t extra_size)
	{
	size_t presize = _PyType_PreHeaderSize(tp);
	PyObject *op = gc_alloc(tp, _PyObject_SIZE(tp) + extra_size, presize);
	if (op == NULL) {
	return NULL;
	}
	memset(op, 0, _PyObject_SIZE(tp) + extra_size);
	_PyObject_Init(op, tp);
	return op;
	}

[tom-pytel]

If I understand correctly is just false positive from TSAN? Can shut this up by adding race:_PyType_AllocNoTrack to Tools/tsan/suppressions_free_threading.txt. Or if you worried about potentially suppressing too much then minimally invasive code that fixes would be something like:

#ifdef _Py_THREAD_SANITIZER
    memset((void *)((char *)obj + sizeof(PyObject)), '\0', size - sizeof(PyObject));
#else
    memset(obj, '\0', size);
#endif

[colesbury]

I'll have a PR for this soon. I think we should adjust the memset unconditionally. It's not really a false positive.

[tom-pytel]

It's not really a false positive.

Meaning the data race condition is real (as detected by tsan, according to tsan's rules), but harmless? new_reference() happens in same thread after the memset so consistency should be assured?

[colesbury]

memset may reasonably zero the data byte-by-byte. That, for example, could make a different thread id appear to match some other thread's id. Not very likely to happen, but that's the sort of thing I mean by "not really a false positive".