gh-129069: Fix listobject.c data races due to memmove#142957
gh-129069: Fix listobject.c data races due to memmove#142957colesbury merged 4 commits intopython:mainfrom
Conversation
The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing. Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
colesbury
added
the
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 97e8d13 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142957%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
Objects/listobject.c
Outdated
There was a problem hiding this comment.
Why can't this just call list_atomic_memmove?
There was a problem hiding this comment.
Yeah, that will simplify this a bit and avoid the special case here (since list_atomic_memmove has it internally).
Objects/listobject.c
Outdated
There was a problem hiding this comment.
This is so much nicer, I wish we'd refactored this the first time round :P
Objects/listobject.c
Outdated
There was a problem hiding this comment.
Not a fan of the name (it's not the memmove as a whole that's atomic, just the individual writes) but I can't think of a better name and I think the comment explains it well enough.
There was a problem hiding this comment.
Yeah, it's a bit misleading. I can name it ptr_wise_atomic_memmove (as in Byte-wise atomic memcpy)
…-142957) The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing. Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
3 participants
The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing.
Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
I've also updated
list_resizeso it definitively doesn't fail when shrinking a list. This simplifies some of the call sites.