FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python by colesbury · Pull Request #143441 · python/cpython · GitHub

/ cpython Public

gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python #143441

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.

Sign up for GitHub

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
colesbury merged 3 commits into from
Jan 6, 2026
+38 −5
Conversation 6 Commits 3 Checks 46 Files changed 2
Merged

gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python #143441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4c76a6
gh-132070: Fix PyObject_Realloc thread-safety in free threaded Python
colesbury Jan 2, 2026
da8b157
Update Objects/obmalloc.c
colesbury Jan 5, 2026
3387cd2
Add comment about memcpy
colesbury Jan 6, 2026
File filter

Filter by extension

Filter by extension .c  (1) .txt  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
39 changes: 38 additions & 1 deletion Objects/obmalloc.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,45 @@ _PyObject_MiRealloc(void *ctx, void *ptr, size_t nbytes)
{
#ifdef Py_GIL_DISABLED
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
// Implement our own realloc logic so that we can copy PyObject header
// in a thread-safe way.
size_t size = mi_usable_size(ptr);
if (nbytes <= size && nbytes >= (size / 2) && nbytes > 0) {
return ptr;
}

mi_heap_t *heap = tstate->mimalloc.current_object_heap;
return mi_heap_realloc(heap, ptr, nbytes);
void* newp = mi_heap_malloc(heap, nbytes);
if (newp == NULL) {
return NULL;
}

// Free threaded Python allows access from other threads to the PyObject reference count
// fields for a period of time after the object is freed (see InternalDocs/qsbr.md).
// These fields are typically initialized by PyObject_Init() using relaxed
// atomic stores. We need to copy these fields in a thread-safe way here.
// We use the "debug_offset" to determine how many bytes to copy -- it
// includes the PyObject header and plus any extra pre-headers.
size_t offset = heap->debug_offset;
assert(offset % sizeof(void*) == 0);

size_t copy_size = (size < nbytes ? size : nbytes);
if (copy_size >= offset) {
for (size_t i = 0; i != offset; i += sizeof(void*)) {
// Use memcpy to avoid strict-aliasing issues. However, we probably
// still have unavoidable strict-aliasing issues with
// _Py_atomic_store_ptr_relaxed here.
void *word;
memcpy(&word, (char*)ptr + i, sizeof(void*));
_Py_atomic_store_ptr_relaxed((void**)((char*)newp + i), word);

Yhg1s Jan 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is there a reason for the intermediate word? And why the memcpy instead of just a cast + deref + assignment? Can't this just be:

Suggested change
_Py_atomic_store_ptr_relaxed((void**)((char*)newp + i), word);
_Py_atomic_store_ptr_relaxed((void**)((char*)newp + i), (void *)((char *)ptr + 1));

(If it can't be, it probably deserves a comment.)

colesbury Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

My thinking was that the memcpy avoids accessing data the pointers of an incorrect size or strict aliasing issues. For example, we read ob_flags, ob_mutex, ob_gc_bits, and ob_ref_local together as a 64-bit load (on 64-bit systems).

I guess there's still the same issue when we write the data with _Py_atomic_store_ptr_relaxed, so maybe it doesn't matter.

From what I can tell, UBSan and ASan don't check for this kind of thing.

colesbury Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I added a comment
}
_mi_memcpy((char*)newp + offset, (char*)ptr + offset, copy_size - offset);
}
else {
_mi_memcpy(newp, ptr, copy_size);
}
mi_free(ptr);
return newp;
#else
return mi_realloc(ptr, nbytes);
#endif
Expand Down
4 changes: 0 additions & 4 deletions Tools/tsan/suppressions_free_threading.txt
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,3 @@ race_top:_PyObject_TryGetInstanceAttribute

# https://gist.github.com/mpage/6962e8870606cfc960e159b407a0cb40
thread:pthread_create

# PyObject_Realloc internally does memcpy which isn't atomic so can race
# with non-locking reads. See #132070
race:PyObject_Realloc
Loading
Back | FazBrowse Home | New Git URL