Skip to content

Commit 98e55d7

Browse files
authored
gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python (gh-143441)
The PyObject header reference count fields must be initialized using atomic operations because they may be concurrently read by another thread (e.g., from `_Py_TryIncref`).
1 parent df35534 commit 98e55d7

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

Objects/obmalloc.c

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,45 @@ _PyObject_MiRealloc(void *ctx, void *ptr, size_t nbytes)
307307
{
308308
#ifdef Py_GIL_DISABLED
309309
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
310+
// Implement our own realloc logic so that we can copy PyObject header
311+
// in a thread-safe way.
312+
size_t size = mi_usable_size(ptr);
313+
if (nbytes <= size && nbytes >= (size / 2) && nbytes > 0) {
314+
return ptr;
315+
}
316+
310317
mi_heap_t *heap = tstate->mimalloc.current_object_heap;
311-
return mi_heap_realloc(heap, ptr, nbytes);
318+
void* newp = mi_heap_malloc(heap, nbytes);
319+
if (newp == NULL) {
320+
return NULL;
321+
}
322+
323+
// Free threaded Python allows access from other threads to the PyObject reference count
324+
// fields for a period of time after the object is freed (see InternalDocs/qsbr.md).
325+
// These fields are typically initialized by PyObject_Init() using relaxed
326+
// atomic stores. We need to copy these fields in a thread-safe way here.
327+
// We use the "debug_offset" to determine how many bytes to copy -- it
328+
// includes the PyObject header and plus any extra pre-headers.
329+
size_t offset = heap->debug_offset;
330+
assert(offset % sizeof(void*) == 0);
331+
332+
size_t copy_size = (size < nbytes ? size : nbytes);
333+
if (copy_size >= offset) {
334+
for (size_t i = 0; i != offset; i += sizeof(void*)) {
335+
// Use memcpy to avoid strict-aliasing issues. However, we probably
336+
// still have unavoidable strict-aliasing issues with
337+
// _Py_atomic_store_ptr_relaxed here.
338+
void *word;
339+
memcpy(&word, (char*)ptr + i, sizeof(void*));
340+
_Py_atomic_store_ptr_relaxed((void**)((char*)newp + i), word);
341+
}
342+
_mi_memcpy((char*)newp + offset, (char*)ptr + offset, copy_size - offset);
343+
}
344+
else {
345+
_mi_memcpy(newp, ptr, copy_size);
346+
}
347+
mi_free(ptr);
348+
return newp;
312349
#else
313350
return mi_realloc(ptr, nbytes);
314351
#endif

Tools/tsan/suppressions_free_threading.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,3 @@ race_top:_PyObject_TryGetInstanceAttribute
1616

1717
# https://gist.github.com/mpage/6962e8870606cfc960e159b407a0cb40
1818
thread:pthread_create
19-
20-
# PyObject_Realloc internally does memcpy which isn't atomic so can race
21-
# with non-locking reads. See #132070
22-
race:PyObject_Realloc

0 commit comments

Comments
 (0)