Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cb6f75a

Browse filesBrowse files
mpagempage
authored
gh-117657: Fix data races when writing / reading ob_gc_bits (#118292)
Use relaxed atomics when reading / writing to the field. There are still a few places in the GC where we do not use atomics. Those should be safe as the world is stopped.
1 parent 8d84120 commit cb6f75a
Copy full SHA for cb6f75a

File tree

4 files changed

+45
-16
lines changed
Filter options

4 files changed

+45
-16
lines changed

‎Include/internal/pycore_gc.h

Copy file name to clipboardExpand all lines: Include/internal/pycore_gc.h
+41-9Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,15 @@ static inline PyObject* _Py_FROM_GC(PyGC_Head *gc) {
3737
}
3838

3939

40-
/* Bit flags for ob_gc_bits (in Py_GIL_DISABLED builds) */
40+
/* Bit flags for ob_gc_bits (in Py_GIL_DISABLED builds)
41+
*
42+
* Setting the bits requires a relaxed store. The per-object lock must also be
43+
* held, except when the object is only visible to a single thread (e.g. during
44+
* object initialization or destruction).
45+
*
46+
* Reading the bits requires using a relaxed load, but does not require holding
47+
* the per-object lock.
48+
*/
4149
#ifdef Py_GIL_DISABLED
4250
# define _PyGC_BITS_TRACKED (1) // Tracked by the GC
4351
# define _PyGC_BITS_FINALIZED (2) // tp_finalize was called
@@ -48,10 +56,34 @@ static inline PyObject* _Py_FROM_GC(PyGC_Head *gc) {
4856
# define _PyGC_BITS_DEFERRED (64) // Use deferred reference counting
4957
#endif
5058

59+
#ifdef Py_GIL_DISABLED
60+
61+
static inline void
62+
_PyObject_SET_GC_BITS(PyObject *op, uint8_t new_bits)
63+
{
64+
uint8_t bits = _Py_atomic_load_uint8_relaxed(&op->ob_gc_bits);
65+
_Py_atomic_store_uint8_relaxed(&op->ob_gc_bits, bits | new_bits);
66+
}
67+
68+
static inline int
69+
_PyObject_HAS_GC_BITS(PyObject *op, uint8_t bits)
70+
{
71+
return (_Py_atomic_load_uint8_relaxed(&op->ob_gc_bits) & bits) != 0;
72+
}
73+
74+
static inline void
75+
_PyObject_CLEAR_GC_BITS(PyObject *op, uint8_t bits_to_clear)
76+
{
77+
uint8_t bits = _Py_atomic_load_uint8_relaxed(&op->ob_gc_bits);
78+
_Py_atomic_store_uint8_relaxed(&op->ob_gc_bits, bits & ~bits_to_clear);
79+
}
80+
81+
#endif
82+
5183
/* True if the object is currently tracked by the GC. */
5284
static inline int _PyObject_GC_IS_TRACKED(PyObject *op) {
5385
#ifdef Py_GIL_DISABLED
54-
return (op->ob_gc_bits & _PyGC_BITS_TRACKED) != 0;
86+
return _PyObject_HAS_GC_BITS(op, _PyGC_BITS_TRACKED);
5587
#else
5688
PyGC_Head *gc = _Py_AS_GC(op);
5789
return (gc->_gc_next != 0);
@@ -80,12 +112,12 @@ static inline int _PyObject_GC_MAY_BE_TRACKED(PyObject *obj) {
80112
* for calling _PyMem_FreeDelayed on the referenced
81113
* memory. */
82114
static inline int _PyObject_GC_IS_SHARED(PyObject *op) {
83-
return (op->ob_gc_bits & _PyGC_BITS_SHARED) != 0;
115+
return _PyObject_HAS_GC_BITS(op, _PyGC_BITS_SHARED);
84116
}
85117
#define _PyObject_GC_IS_SHARED(op) _PyObject_GC_IS_SHARED(_Py_CAST(PyObject*, op))
86118

87119
static inline void _PyObject_GC_SET_SHARED(PyObject *op) {
88-
op->ob_gc_bits |= _PyGC_BITS_SHARED;
120+
_PyObject_SET_GC_BITS(op, _PyGC_BITS_SHARED);
89121
}
90122
#define _PyObject_GC_SET_SHARED(op) _PyObject_GC_SET_SHARED(_Py_CAST(PyObject*, op))
91123

@@ -95,13 +127,13 @@ static inline void _PyObject_GC_SET_SHARED(PyObject *op) {
95127
* Objects with this bit that are GC objects will automatically
96128
* delay-freed by PyObject_GC_Del. */
97129
static inline int _PyObject_GC_IS_SHARED_INLINE(PyObject *op) {
98-
return (op->ob_gc_bits & _PyGC_BITS_SHARED_INLINE) != 0;
130+
return _PyObject_HAS_GC_BITS(op, _PyGC_BITS_SHARED_INLINE);
99131
}
100132
#define _PyObject_GC_IS_SHARED_INLINE(op) \
101133
_PyObject_GC_IS_SHARED_INLINE(_Py_CAST(PyObject*, op))
102134

103135
static inline void _PyObject_GC_SET_SHARED_INLINE(PyObject *op) {
104-
op->ob_gc_bits |= _PyGC_BITS_SHARED_INLINE;
136+
_PyObject_SET_GC_BITS(op, _PyGC_BITS_SHARED_INLINE);
105137
}
106138
#define _PyObject_GC_SET_SHARED_INLINE(op) \
107139
_PyObject_GC_SET_SHARED_INLINE(_Py_CAST(PyObject*, op))
@@ -178,23 +210,23 @@ static inline void _PyGCHead_SET_PREV(PyGC_Head *gc, PyGC_Head *prev) {
178210

179211
static inline int _PyGC_FINALIZED(PyObject *op) {
180212
#ifdef Py_GIL_DISABLED
181-
return (op->ob_gc_bits & _PyGC_BITS_FINALIZED) != 0;
213+
return _PyObject_HAS_GC_BITS(op, _PyGC_BITS_FINALIZED);
182214
#else
183215
PyGC_Head *gc = _Py_AS_GC(op);
184216
return ((gc->_gc_prev & _PyGC_PREV_MASK_FINALIZED) != 0);
185217
#endif
186218
}
187219
static inline void _PyGC_SET_FINALIZED(PyObject *op) {
188220
#ifdef Py_GIL_DISABLED
189-
op->ob_gc_bits |= _PyGC_BITS_FINALIZED;
221+
_PyObject_SET_GC_BITS(op, _PyGC_BITS_FINALIZED);
190222
#else
191223
PyGC_Head *gc = _Py_AS_GC(op);
192224
gc->_gc_prev |= _PyGC_PREV_MASK_FINALIZED;
193225
#endif
194226
}
195227
static inline void _PyGC_CLEAR_FINALIZED(PyObject *op) {
196228
#ifdef Py_GIL_DISABLED
197-
op->ob_gc_bits &= ~_PyGC_BITS_FINALIZED;
229+
_PyObject_CLEAR_GC_BITS(op, _PyGC_BITS_FINALIZED);
198230
#else
199231
PyGC_Head *gc = _Py_AS_GC(op);
200232
gc->_gc_prev &= ~_PyGC_PREV_MASK_FINALIZED;

‎Include/internal/pycore_object.h

Copy file name to clipboardExpand all lines: Include/internal/pycore_object.h
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ static inline int
168168
_PyObject_HasDeferredRefcount(PyObject *op)
169169
{
170170
#ifdef Py_GIL_DISABLED
171-
return (op->ob_gc_bits & _PyGC_BITS_DEFERRED) != 0;
171+
return _PyObject_HAS_GC_BITS(op, _PyGC_BITS_DEFERRED);
172172
#else
173173
return 0;
174174
#endif
@@ -320,7 +320,7 @@ static inline void _PyObject_GC_TRACK(
320320
"object already tracked by the garbage collector",
321321
filename, lineno, __func__);
322322
#ifdef Py_GIL_DISABLED
323-
op->ob_gc_bits |= _PyGC_BITS_TRACKED;
323+
_PyObject_SET_GC_BITS(op, _PyGC_BITS_TRACKED);
324324
#else
325325
PyGC_Head *gc = _Py_AS_GC(op);
326326
_PyObject_ASSERT_FROM(op,
@@ -361,7 +361,7 @@ static inline void _PyObject_GC_UNTRACK(
361361
filename, lineno, __func__);
362362

363363
#ifdef Py_GIL_DISABLED
364-
op->ob_gc_bits &= ~_PyGC_BITS_TRACKED;
364+
_PyObject_CLEAR_GC_BITS(op, _PyGC_BITS_TRACKED);
365365
#else
366366
PyGC_Head *gc = _Py_AS_GC(op);
367367
PyGC_Head *prev = _PyGCHead_PREV(gc);

‎Objects/object.c

Copy file name to clipboardExpand all lines: Objects/object.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2427,14 +2427,14 @@ _PyObject_SetDeferredRefcount(PyObject *op)
24272427
assert(PyType_IS_GC(Py_TYPE(op)));
24282428
assert(_Py_IsOwnedByCurrentThread(op));
24292429
assert(op->ob_ref_shared == 0);
2430+
_PyObject_SET_GC_BITS(op, _PyGC_BITS_DEFERRED);
24302431
PyInterpreterState *interp = _PyInterpreterState_GET();
24312432
if (interp->gc.immortalize.enabled) {
24322433
// gh-117696: immortalize objects instead of using deferred reference
24332434
// counting for now.
24342435
_Py_SetImmortal(op);
24352436
return;
24362437
}
2437-
op->ob_gc_bits |= _PyGC_BITS_DEFERRED;
24382438
op->ob_ref_local += 1;
24392439
op->ob_ref_shared = _Py_REF_QUEUED;
24402440
#endif

‎Tools/tsan/suppressions_free_threading.txt

Copy file name to clipboardExpand all lines: Tools/tsan/suppressions_free_threading.txt
-3Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ race:_PyImport_AcquireLock
1919
race:_PyImport_ReleaseLock
2020
race:_PyInterpreterState_SetNotRunningMain
2121
race:_PyInterpreterState_IsRunningMain
22-
race:_PyObject_GC_IS_SHARED
23-
race:_PyObject_GC_SET_SHARED
24-
race:_PyObject_GC_TRACK
2522
# https://gist.github.com/mpage/0a24eb2dd458441ededb498e9b0e5de8
2623
race:_PyParkingLot_Park
2724
race:_PyType_HasFeature

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.