Race in type_get_annotations under free-threading

[hawkinsp]

Bug report

Bug description:

I don't have a succinct reproducer for this, but TSAN reported the following race under CPython 3.13 with free-threading.

(forked from #128714 (comment) )

WARNING: ThreadSanitizer: data race (pid=355399)
  Read of size 4 at 0x7e07ad0c2190 by thread T65 (mutexes: read M0):
    #0 PyType_Modified /__w/jax/jax/cpython/Objects/typeobject.c:1059:15 (python3.13+0x305823) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #1 type_get_annotations /__w/jax/jax/cpython/Objects/typeobject.c:1790:17 (python3.13+0x305823)
    #2 getset_get /__w/jax/jax/cpython/Objects/descrobject.c:193:16 (python3.13+0x1ff5e8) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #3 _Py_type_getattro_impl /__w/jax/jax/cpython/Objects/typeobject.c:5411:19 (python3.13+0x2e92cd) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #4 PyObject_GetOptionalAttr /__w/jax/jax/cpython/Objects/object.c:1307:19 (python3.13+0x292eb1) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #5 builtin_getattr /__w/jax/jax/cpython/Python/bltinmodule.c:1200:13 (python3.13+0x3d8d79) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #6 cfunction_vectorcall_FASTCALL /__w/jax/jax/cpython/Objects/methodobject.c:425:24 (python3.13+0x289b7b) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #7 _PyObject_VectorcallTstate /__w/jax/jax/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1ead4a) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)

...

Previous atomic write of size 4 at 0x7e07ad0c2190 by thread T67 (mutexes: read M0):
    #0 _Py_atomic_store_uint32_relaxed /__w/jax/jax/cpython/./Include/cpython/pyatomic_gcc.h:461:3 (python3.13+0x2e5090) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #1 set_version_unlocked /__w/jax/jax/cpython/Objects/typeobject.c:978:5 (python3.13+0x2e5090)
    #2 type_modified_unlocked /__w/jax/jax/cpython/Objects/typeobject.c:1047:5 (python3.13+0x2e5090)
    #3 PyType_Modified /__w/jax/jax/cpython/Objects/typeobject.c:1064:5 (python3.13+0x3058d4) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #4 type_get_annotations /__w/jax/jax/cpython/Objects/typeobject.c:1790:17 (python3.13+0x3058d4)
    #5 getset_get /__w/jax/jax/cpython/Objects/descrobject.c:193:16 (python3.13+0x1ff5e8) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #6 _Py_type_getattro_impl /__w/jax/jax/cpython/Objects/typeobject.c:5411:19 (python3.13+0x2e92cd) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
    #7 PyObject_GetOptionalAttr /__w/jax/jax/cpython/Objects/object.c:1307:19 (python3.13+0x292eb1) (BuildId: c9937216e103905f871b62bf50b66fc5a8e96f80)
 
...

Reading the code, it does not appear there is any locking protecting the initialization of the annotations dictionary of a type.

CPython versions tested on:

3.13.2

Operating systems tested on:

Linux

[picnixz]

cc @JelleZijlstra

[colesbury]

There are two separate things here:

First, there's a data race on type->tp_version_tag because it's read without atomics and outside the lock. This was reported previously in the following issue, but I think we modified the wrong access in the PR. We should have changed PyType_Modified, not type_modified_unlocked.

• Race on tp_version_tag under free threading #128759

Second, type_get_annotations doesn't do any locking, as you point out, other than the internal locking performed by dict. That's not showing up in the TSan race. From reading the code, we might lazily initialize __annotations__ twice, but I'm not sure that it is a problem. It'd probably be cleaner (and simpler to reason about) if we only initialize it once.

[corona10]

I will take a look at this issue.

[corona10]

@hawkinsp
Is it still reproducible at the latest build? I think that most of the things that had been pointed out by Sam were fixed.

[hawkinsp]

I don't know. Let me try removing the tsan suppression from our TSAN CI and see if I see it again!