Skip to content

Commit a4c76a6

Browse files
committed
gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python
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 4f9a8d0 commit a4c76a6

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

Objects/obmalloc.c

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,42 @@ _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 safe access 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+
void *word;
336+
memcpy(&word, (char*)ptr + i, sizeof(void*));
337+
_Py_atomic_store_ptr_relaxed((void**)((char*)newp + i), word);
338+
}
339+
_mi_memcpy((char*)newp + offset, (char*)ptr + offset, copy_size - offset);
340+
}
341+
else {
342+
_mi_memcpy(newp, ptr, copy_size);
343+
}
344+
mi_free(ptr);
345+
return newp;
312346
#else
313347
return mi_realloc(ptr, nbytes);
314348
#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)