Thanks for doing this

My pleasure!

I'm not sure we're able to skip the subinterpreter test yet, because there are still races when doing non-atomic stores of the version on static types. I've looked into fixing those myself, and it'ssignficantly more complex.

Hmm, yes. I had tested running it with thread sanitizer enabled, with-and-without the GIL, and didn't trigger any data race reports. However, adding a new test specifically to try and exercise static type initialization indeed shows abunch of problems remain. Not only with version, but various other accesses (most oftentp_flags).

I suppose those problems may also be hit with the existing test, depending on timing. I'll drop re-enabling that test for now. I might come back and try and fix some of these issues separately, if I have time and no-one else gets there first.

For reference, the test I used:

@requires_subinterpreters@threading_helper.requires_working_threading()deftest_static_type_initialization(self):# This is specifically testing static type initialization thread safety# and is intended to be run with the thread-santizer enableddefinit(barrier):interpid=_interpreters.create()barrier.wait()try:_interpreters.run_string(interpid,"import datetime")finally:_interpreters.destroy(interpid)N=3ITERATIONS=10barrier=threading.Barrier(N)for_inrange(ITERATIONS):threads= [threading.Thread(target=init,args=(barrier,))for_inrange(N)]withthreading_helper.start_threads(threads):pass

I'm sure using subinterpreters is not strictly required, but they seemed like an easy way to reliably run static type initialisation. Note that if assertions are enabled this test immediately fails:

Objects/typeobject.c:208: managed_static_type_state_init: Assertion `!managed_static_type_index_is_set(self)' failed.

It definitely looks like this is all very racy.