gh-129824: Fix some data races with types in subinterpreters

[colesbury]

The test_running and test_is_running test cases are skipped for now because they have file descriptor races.

• Issue: Various data races in subinterpreter tests under TSAN #129824

[ZeroIntensity]

I'm seeing two other races with TSan:

• NEXT_GLOBAL_VERSION_TAG should use an atomic store/load, not rely on the per-interpreter type lock.
• Same thing for the tp_versions_used field in set_version_unlocked, because that will be shared for static types.

[ZeroIntensity]

I'm worried that these assertions might be racy. If two interpreters are trying to initialize the type at the same time, TSan might start complaining, because the store is non-atomic.

[colesbury]

There's no race because tp_flags is only set in the initial interpreter (when initial=1), and all static types are initialized at interpreter startup.

[ZeroIntensity]

I might not understand the type lifecycle correctly. I'm imagining a case where:

• Two subinterpreters attempt to import a module that has not been initialized by any interpreter, and that module contains static types.
• Interpreter A will get initial atomically, and then interpreter B falls back to the assertion.
• If interpreter A sets tp_flags at the same time as interpreter B calls PyType_HasFeature, wouldn't that race?

Is there some extra layer of synchronization?

[colesbury]

I think that modules are always imported in the main interpreter first, but @ericsnowcurrently would know better:

cpython/Python/import.c

Lines 1973 to 1979 in 8d9d3e4

	* To avoid all of that, we make sure the module's init function
	* is always run first with the main interpreter active. If it was
	* already the main interpreter then we can continue loading the
	* module like normal. Otherwise, right after the init function,
	* we take care of some import state bookkeeping, switch back
	* to the subinterpreter, check for single-phase init,
	* and then continue loading like normal. */

[ZeroIntensity]

Hmm. I think that just means it will have a thread state under the main interpreter, which should synchronize it on a GILicious build, but there would be no synchronization under FT. It's probably worth investigating that once we get the rest of the type races fixed. I'll yield to you on whether to keep the assertions in this PR.

[colesbury]

Yeah, this is more complicated than I originally thought. I'm going to close this for now.

[colesbury]

@ZeroIntensity - there are a number of additional races not fixed by this PR (see the linked issue). I think the two you mentioned are already listed there, but if you find additional ones, please edit the issue and add them.