8000 gh-117657: Fix race data race in `_Py_IsOwnedByCurrentThread()` by mpage · Pull Request #118258 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix race data race in _Py_IsOwnedByCurrentThread()
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
  • Loading branch information
mpage committed Apr 24, 2024
commit 758178407de4d6f66ddfd1a3a580f8c480b27554
2 changes: 1 addition & 1 deletion Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ _Py_ThreadId(void)
static inline Py_ALWAYS_INLINE int
_Py_IsOwnedByCurrentThread(PyObject *ob)
{
return ob->ob_tid == _Py_ThreadId();
return _Py_atomic_load_uintptr_relaxed(&ob->ob_tid) == _Py_ThreadId();
}
#endif

Expand Down
1 change: 0 additions & 1 deletion Tools/tsan/suppressions_free_threading.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ race:_add_to_weak_set
race:_in_weak_set
race:_mi_heap_delayed_free_partial
race:_Py_IsImmortal
race:_Py_IsOwnedByCurrentThread
race:_PyEval_EvalFrameDefault
race:_PyFunction_SetVersion
race:_PyImport_AcquireLock
Expand Down
0