Skip to content

gh-117657: Fix race data race in _Py_IsOwnedByCurrentThread() - #118258

Merged
colesbury merged 3 commits into
python:mainfrom
mpage:gh-117657-tsan-isownedby
Apr 26, 2024
Merged

gh-117657: Fix race data race in _Py_IsOwnedByCurrentThread()#118258
colesbury merged 3 commits into
python:mainfrom
mpage:gh-117657-tsan-isownedby

Conversation

@mpage

@mpagempage commented Apr 24, 2024

Copy link
Copy Markdown
Contributor

We need to load ob_tid atomically; it may be set to zero in another thread without any intervening operations that establish a happens-before relationship.

Using a relaxed load should be sufficient; we do not depend on the store publishing any other information.

Examples:

The load in _Py_IsOwnedByCurrentThread():

returnob->ob_tid==_Py_ThreadId();

races with the store in _Py_ExplicitMergeRefcount() (example):

_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);

and the store in _Py_MergeZeroLocalRefcount() (example):

_Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);

We need to load `ob_tid` atomically; it may be set to zero in another
thread without any intervening operations that establish a happens-before
relationship.
Using a relaxed load should be sufficient; we do not depend on the store
publishing any other information.
Examples:
The load in `_Py_IsOwnedByCurrentThread()`:
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Include/object.h#L306
races with the store in `_Py_ExplicitMergeRefcount()` ([example](https://gist.github.com/mpage/0342ebce4b5b446689c41cdd72f16e61)):
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Objects/object.c#L421
and the store in `_Py_MergeZeroLocalRefcount()` ([example](https://gist.github.com/mpage/eaec11eda6e2d988ed0dd2bcfd6d606b)):
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Objects/object.c#L378
Comment threadInclude/object.h
@mpage
mpage requested a review from colesburyApril 25, 2024 21:52
Sign up for freeto 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

@mpage@colesbury