gh-117657: Quiet more TSAN warnings due to incorrect modeling of compare/exchange

[mpage]

TSAN reports (example) data races between the non-atomic loads of tstate->state in detach_thread() and tstate_set_detached():

cpython/Python/pystate.c

Lines 2067 to 2080 in eca5362

	static void
	detach_thread(PyThreadState *tstate, int detached_state)
	{
	// XXX assert(tstate_is_alive(tstate) && tstate_is_bound(tstate));
	assert(tstate->state == _Py_THREAD_ATTACHED);
	assert(tstate == current_fast_get());
	if (tstate->critical_section != 0) {
	_PyCriticalSection_SuspendAll(tstate);
	}
	#ifdef Py_GIL_DISABLED
	_Py_qsbr_detach(((_PyThreadStateImpl *)tstate)->qsbr);
	#endif
	tstate_deactivate(tstate);
	tstate_set_detached(tstate, detached_state);

cpython/Python/pystate.c

Lines 2003 to 2008 in eca5362

	static void
	tstate_set_detached(PyThreadState *tstate, int detached_state)
	{
	assert(tstate->state == _Py_THREAD_ATTACHED);
	#ifdef Py_GIL_DISABLED
	_Py_atomic_store_int(&tstate->state, detached_state);

and the atomic compare/exchange of tstate->state in park_detached_threads():

cpython/Python/pystate.c

Lines 2159 to 2170 in eca5362

	static bool
	park_detached_threads(struct _stoptheworld_state *stw)
	{
	int num_parked = 0;
	PyInterpreterState *i;
	PyThreadState *t;
	_Py_FOR_EACH_THREAD(stw, i, t) {
	int state = _Py_atomic_load_int_relaxed(&t->state);
	if (state == _Py_THREAD_DETACHED) {
	// Atomically transition to "suspended" if in "detached" state.
	if (_Py_atomic_compare_exchange_int(&t->state,
	&state, _Py_THREAD_SUSPENDED)) {

I believe this is due to a bug with how TSAN models compare/exchange. Note that this appears to be a different issue than what we're seeing in #117828; the compare/exhange is succeeding in this case.

According to the standard,

If one evaluation modifies a memory location, and the other reads or modifies the same memory location, and if at least one of the evaluations is not an atomic operation, the behavior of the program is undefined (the program has a data race) unless there exists a happens-before relationship between these two evaluations.

However, according to the standard, A strongly happens-before B if any of the following are true:

1. A is sequenced-before B.
2. A synchronizes-with B.
3. A strongly happens-before X, and X strongly happens-before B.

Using this we can establish a happens-before relationship between each non-atomic load and the compare/exchange:

1. Each non-atomic load is sequenced-before the seq-cst store to tstate->state that happens in tstate_set_detached(). Therefore, each non-atomic load happens-before the seq-cst store.
2. The seq-cst store to tstate->state in tstate_set_detached() synchronizes-with the compare/exchange to tstate->state in park_detached_threads(). Thus the seq-cst store happens-before the compare/exchange.
3. (1) and (2) establish the happens-before relationship between each non-atomic load and the compare/exchange on tstate->state, therefore they should not be flagged as a data races.

• Issue: Make TSAN tests pass with the GIL disabled in free-threaded builds #117657