data race when resizing unicode objects

[kumaraditya303]

When running ctypes tests in parallel, the following data race is detected:

env TSAN_OPTIONS="suppressions=Tools/tsan/suppressions_free_threading.txt halt_on_error=1" ./python -m test test_ctypes --parallel-threads 4

==================
WARNING: ThreadSanitizer: data race (pid=19492)
  Atomic read of size 8 at 0x7f1b0a230680 by thread T1323:
    #0 _Py_atomic_load_uintptr_relaxed /home/realkumaraditya/cpython/./Include/cpython/pyatomic_gcc.h:375:10 (python+0x271055) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #1 _Py_IsOwnedByCurrentThread /home/realkumaraditya/cpython/./Include/object.h:254:12 (python+0x271055)
    #2 _Py_TryIncrefFast /home/realkumaraditya/cpython/./Include/internal/pycore_object.h:558:9 (python+0x271055)
    #3 _Py_TryIncrefCompare /home/realkumaraditya/cpython/./Include/internal/pycore_object.h:597:9 (python+0x271055)
    #4 compare_unicode_unicode_threadsafe /home/realkumaraditya/cpython/Objects/dictobject.c:1432:18 (python+0x271055)
    #5 do_lookup /home/realkumaraditya/cpython/Objects/dictobject.c:1013:23 (python+0x271055)
    #6 unicodekeys_lookup_unicode_threadsafe /home/realkumaraditya/cpython/Objects/dictobject.c:1449:12 (python+0x271055)
    #7 _Py_dict_lookup_threadsafe /home/realkumaraditya/cpython/Objects/dictobject.c:1504:18 (python+0x270757) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #8 _PyDict_GetItemRef_KnownHash /home/realkumaraditya/cpython/Objects/dictobject.c:2353:21 (python+0x272c0e) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #9 PyDict_GetItemRef /home/realkumaraditya/cpython/Objects/dictobject.c:2389:12 (python+0x272c0e)
    #10 cache_struct_converter /home/realkumaraditya/cpython/./Modules/_struct.c:2524:9 (_struct.cpython-314t-x86_64-linux-gnu.so+0x5338) (BuildId: cd844f32564aba7a2b18bdb1e258a78c397c8ebc)
    #11 calcsize /home/realkumaraditya/cpython/./Modules/clinic/_struct.c.h:266:10 (_struct.cpython-314t-x86_64-linux-gnu.so+0x4ac7) (BuildId: cd844f32564aba7a2b18bdb1e258a78c397c8ebc)
    #12 _PyEval_EvalFrameDefault /home/realkumaraditya/cpython/Python/generated_cases.c.h:2225:35 (python+0x3fef50) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #13 _PyEval_EvalFrame /home/realkumaraditya/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x3f7150) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #14 _PyEval_Vector /home/realkumaraditya/cpython/Python/ceval.c:1908:12 (python+0x3f7150)
    #15 _PyFunction_Vectorcall /home/realkumaraditya/cpython/Objects/call.c (python+0x1f1cdf) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #16 _PyObject_VectorcallTstate /home/realkumaraditya/cpython/./Include/internal/pycore_call.h:169:11 (python+0x1f66b0) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #17 method_vectorcall /home/realkumaraditya/cpython/Objects/classobject.c:94:18 (python+0x1f66b0)
    #18 _PyVectorcall_Call /home/realkumaraditya/cpython/Objects/call.c:273:16 (python+0x1f196f) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #19 _PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:348:16 (python+0x1f196f)
    #20 PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:373:12 (python+0x1f19d5) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #21 _PyEval_EvalFrameDefault /home/realkumaraditya/cpython/Python/generated_cases.c.h:2411:32 (python+0x3ff399) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #22 _PyEval_EvalFrame /home/realkumaraditya/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x3f7150) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #23 _PyEval_Vector /home/realkumaraditya/cpython/Python/ceval.c:1908:12 (python+0x3f7150)
    #24 _PyFunction_Vectorcall /home/realkumaraditya/cpython/Objects/call.c (python+0x1f1cdf) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #25 _PyObject_VectorcallTstate /home/realkumaraditya/cpython/./Include/internal/pycore_call.h:169:11 (python+0x1f65ff) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #26 method_vectorcall /home/realkumaraditya/cpython/Objects/classobject.c:72:20 (python+0x1f65ff)
    #27 _PyVectorcall_Call /home/realkumaraditya/cpython/Objects/call.c:273:16 (python+0x1f196f) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #28 _PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:348:16 (python+0x1f196f)
    #29 PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:373:12 (python+0x1f19d5) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #30 thread_run /home/realkumaraditya/cpython/./Modules/_threadmodule.c:353:21 (python+0x5998d2) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #31 pythread_wrapper /home/realkumaraditya/cpython/Python/thread_pthread.h:242:5 (python+0x4f4157) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)

  Previous write of size 8 at 0x7f1b0a230680 by thread T1321:
    #0 __tsan_memcpy <null> (python+0xdd10f) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #1 _mi_memcpy /home/realkumaraditya/cpython/./Include/internal/mimalloc/mimalloc/internal.h:929:3 (python+0x2af9bf) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #2 _mi_heap_realloc_zero /home/realkumaraditya/cpython/Objects/mimalloc/alloc.c:745:7 (python+0x2af9bf)
    #3 mi_heap_realloc /home/realkumaraditya/cpython/Objects/mimalloc/alloc.c:753:10 (python+0x2cce30) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #4 _PyObject_MiRealloc /home/realkumaraditya/cpython/Objects/obmalloc.c:276:12 (python+0x2cce30)
    #5 PyObject_Realloc /home/realkumaraditya/cpython/Objects/obmalloc.c:1416:12 (python+0x2cfc12) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #6 resize_compact /home/realkumaraditya/cpython/Objects/unicodeobject.c:1178:31 (python+0x359915) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #7 _PyUnicodeWriter_Finish /home/realkumaraditya/cpython/Objects/unicodeobject.c:14157:16 (python+0x33162a) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #8 PyUnicode_Format /home/realkumaraditya/cpython/Objects/unicodeobject.c:15361:12 (python+0x35e728) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #9 unicode_mod /home/realkumaraditya/cpython/Objects/unicodeobject.c:14332:12 (python+0x36df71) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #10 binary_op1 /home/realkumaraditya/cpython/Objects/abstract.c:964:13 (python+0x1bdb83) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #11 binary_op /home/realkumaraditya/cpython/Objects/abstract.c:1003:24 (python+0x1be2a8) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #12 PyNumber_Remainder /home/realkumaraditya/cpython/Objects/abstract.c:1187:1 (python+0x1be2a8)
    #13 _PyEval_EvalFrameDefault /home/realkumaraditya/cpython/Python/generated_cases.c.h:62:35 (python+0x3f7ae5) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #14 _PyEval_EvalFrame /home/realkumaraditya/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x3f7150) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #15 _PyEval_Vector /home/realkumaraditya/cpython/Python/ceval.c:1908:12 (python+0x3f7150)
    #16 _PyFunction_Vectorcall /home/realkumaraditya/cpython/Objects/call.c (python+0x1f1cdf) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #17 _PyObject_VectorcallTstate /home/realkumaraditya/cpython/./Include/internal/pycore_call.h:169:11 (python+0x1f66b0) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #18 method_vectorcall /home/realkumaraditya/cpython/Objects/classobject.c:94:18 (python+0x1f66b0)
    #19 _PyVectorcall_Call /home/realkumaraditya/cpython/Objects/call.c:273:16 (python+0x1f196f) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #20 _PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:348:16 (python+0x1f196f)
    #21 PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:373:12 (python+0x1f19d5) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #22 _PyEval_EvalFrameDefault /home/realkumaraditya/cpython/Python/generated_cases.c.h:2411:32 (python+0x3ff399) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #23 _PyEval_EvalFrame /home/realkumaraditya/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x3f7150) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #24 _PyEval_Vector /home/realkumaraditya/cpython/Python/ceval.c:1908:12 (python+0x3f7150)
    #25 _PyFunction_Vectorcall /home/realkumaraditya/cpython/Objects/call.c (python+0x1f1cdf) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #26 _PyObject_VectorcallTstate /home/realkumaraditya/cpython/./Include/internal/pycore_call.h:169:11 (python+0x1f65ff) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #27 method_vectorcall /home/realkumaraditya/cpython/Objects/classobject.c:72:20 (python+0x1f65ff)
    #28 _PyVectorcall_Call /home/realkumaraditya/cpython/Objects/call.c:273:16 (python+0x1f196f) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #29 _PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:348:16 (python+0x1f196f)
    #30 PyObject_Call /home/realkumaraditya/cpython/Objects/call.c:373:12 (python+0x1f19d5) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #31 thread_run /home/realkumaraditya/cpython/./Modules/_threadmodule.c:353:21 (python+0x5998d2) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)
    #32 pythread_wrapper /home/realkumaraditya/cpython/Python/thread_pthread.h:242:5 (python+0x4f4157) (BuildId: 27b79e16bbd11b196a3f4a8232b4b09ecf4e18ee)

Linked PRs

• gh-132070: use _PyObject_IsUniquelyReferenced in unicodeobject #132091
• gh-132070: add PyObject_Realloc suppression in free-threading #132468
• gh-132070: Use _PyObject_IsUniquelyReferenced in unicodeobject #133039

[kumaraditya303]

I think in free-threading Py_REFCNT(op)==1 should be replaced by PyObject_IsUniquelyReferenced in such functions in unicodeobject.c:

cpython/Objects/unicodeobject.c

Lines 1197 to 1202 in a126cef

	static int
	resize_inplace(PyObject *unicode, Py_ssize_t length)
	{
	assert(!PyUnicode_IS_COMPACT(unicode));
	assert(Py_REFCNT(unicode) == 1);

[colesbury]

We should leave the assertions as is. I don't think that's related to this.

The race is because PyObject_Realloc() calls memcpy to fill in the new allocation, if the existing allocation can't be reused. The memcpy doesn't use relaxed atomic writes, so it can race with an one of the optimistic, non-locking reads.

For now, we should just add a suppression.

[colesbury]

We should change Py_REFCNT(op)==1 to PyObject_IsUniquelyReferenced for places where it controls behaviors, such as in if-statements, but the uses in assertions don't really matter.

[godlygeek]

Would it make sense to change the behavior of Py_REFCNT in free-threading so that Py_REFCNT(op)==1 remains a valid check? I think that due to TOCTOU the only thing an extension module can usefully do with Py_REFCNT even in GIL builds is check whether the recount of an object is or isn't 1. If you're no longer able to use Py_REFCNT to check that in FT builds, it seems like Py_REFCNT becomes totally useless, and it would be much more useful to do

#define Py_REFCNT(x) (PyObject_IsUniquelyReferenced((x)) ? 1 : INT_MAX)

or some other arbitrary large number.

[serhiy-storchaka]

There are some places where Py_REFCNT is compared with 2 or 0.

[godlygeek]

Sure, but those don't work in free threaded builds today, right? So my question is whether we can make it so that Py_REFCNT in an FT build returns something wrong but usually useful instead of something wrong and never useful.