Skip to content

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Apr 22, 2024

These should be the last instances of TSAN warning of data races when accessing tstate->state non-atomically.

TSAN reports a race (example) between accessing tstate->state in _PyThreadState_Suspend():

assert(tstate->state == _Py_THREAD_ATTACHED);

and the compare/exchange in park_detached_threads():

cpython/Python/pystate.c

Lines 2172 to 2173 in fc21c7f

if (_Py_atomic_compare_exchange_int(&t->state,
&state, _Py_THREAD_SUSPENDED)) {

This is a false positive due to TSAN modeling failed compare/exchange operations as writes.

TSAN also reports a race (example) between accessing tstate->state in _PySemaphore_Wait():

if (tstate && tstate->state == _Py_THREAD_ATTACHED) {

and the compare/exchange in park_detached_threads() above. This is the same issue that we saw in gh-117830.

…tate`

These should be the last instances of TSAN warning of data races when accessing
`tstate->state` non-atomically.

TSAN reports a race between accessing `tstate->state` in `_PyThreadState_Suspend()`
and the compare/exhange in `park_detached_threads`. This is a false positive due
to TSAN modeling failed compare/exchange operations as writes.

TSAN reports a race between accessing `tstate->state` in `_PySemaphore_Wait()`
and the compare/exchange in `park_detached_threads`. This is the same issue that
we saw in pythongh-117830.
@mpage mpage changed the title Quiet TSAN warnings about remaining non-atomic accesses of tstate->state gh-117657: Quiet TSAN warnings about remaining non-atomic accesses of tstate->state Apr 23, 2024
@mpage mpage requested review from DinoV and colesbury April 23, 2024 17:02
@mpage mpage marked this pull request as ready for review April 23, 2024 17:02
Copy link
Contributor

@DinoV DinoV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@DinoV DinoV merged commit 2e7771a into python:main Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants