GeezerVison is showing github.com — Data race: `TextIOWrapper.detach()` stores `self->buffer = NULL` non-atomically · Issue #154523 · python/cpython
Data race: TextIOWrapper.detach() stores self->buffer = NULL non-atomically #154523DescriptionBug report
Bug description:
In the free-threaded build, TextIOWrapper.detach() clears the self->buffer slot with a plain store, while reading tw.buffer goes through a LOAD_ATTR_SLOT specialization that reads the same slot with FT_ATOMIC_LOAD_PTR.
Writer — _io_TextIOWrapper_detach_impl:
|
_io_TextIOWrapper_detach_impl(textio *self) |
|
/*[clinic end generated code: output=7ba3715cd032d5f2 input=c908a3b4ef203b0f]*/ |
|
{ |
|
PyObject *buffer; |
|
if (_PyFile_Flush((PyObject *)self) < 0) { |
|
return NULL; |
|
} |
|
/* _PyFile_Flush could detach before returning; raise an exception. */ |
|
buffer = buffer_access_safe(self); |
|
if (buffer == NULL) { |
|
return NULL; |
|
} |
|
self->buffer = NULL; |
|
self->detached = 1; |
|
return buffer; |
|
} |
Reader — LOAD_ATTR_SLOT reads the slot atomically:
|
// _LOAD_ATTR_SLOT |
|
{ |
|
uint16_t index = read_u16(&this_instr[4].cache); |
|
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); |
|
PyObject **addr = (PyObject **)((char *)owner_o + index); |
|
PyObject *attr_o = FT_ATOMIC_LOAD_PTR(*addr); |
|
if (attr_o == NULL) { |
|
UPDATE_MISS_STATS(LOAD_ATTR); |
|
assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR)); |
|
JUMP_TO_PREDICTED(LOAD_ATTR); |
|
} |
So tw.buffer racing tw.detach() is a plain-store-vs-atomic-load data race on the buffer slot. detach() should store the slot atomically.
Reproducer:
import io
from threading import Thread
def make():
return io.TextIOWrapper(io.BytesIO(b'x' * 16))
slot = [make()]
def reader():
for _ in range(20000):
try:
_ = slot[0].buffer
except Exception:
pass
def detacher():
for _ in range(20000):
try:
slot[0].detach()
except Exception:
pass
slot[0] = make()
threads = [Thread(target=reader) for _ in range(6)]
threads += [Thread(target=detacher) for _ in range(2)]
for t in threads: t.start()
for t in threads: t.join()TSAN Report:
WARNING: ThreadSanitizer: data race (pid=1522499)
Write of size 8 at 0x7fffd0100110 by thread T7:
#0 _io_TextIOWrapper_detach_impl /cpython/./Modules/_io/textio.c:1642:18
#1 _io_TextIOWrapper_detach /cpython/./Modules/_io/clinic/textio.c.h:793:20
#2 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:4330:35
Previous atomic read of size 8 at 0x7fffd0100110 by thread T6:
#0 _Py_atomic_load_ptr /cpython/./Include/cpython/pyatomic_gcc.h:300:18
#1 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:9588:36
#2 _PyEval_EvalFrame /cpython/./Include/internal/pycore_ceval.h:122:16
SUMMARY: ThreadSanitizer: data race /cpython/./Modules/_io/textio.c:1642:18 in _io_TextIOWrapper_detach_impl
==================
CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS, Linux
Linked PRs
Activitydevdanzin commented on Jul 23, 2026 Agree this is a genuine C-level data race worth the "store the slot atomically" fix, but as a severity note: it looks value-benign on aligned hardware — I couldn't turn it into a crash.
Tried on a free-threaded debug build and a free-threaded ASan build of main: the reader/detacher race (amplified — readers also touch the returned buffer), and separately concurrent detach() on a shared wrapper (barrier-synchronized, to try for a double-Py_CLEAR/double-DECREF), both run clean across many attempts.
That matches the code: _io_TextIOWrapper_detach_impl is @critical_section (so concurrent detach() can't double-free — the second one gets NULL from buffer_access_safe and raises), and it transfers ownership of the buffer to the caller via return buffer rather than Py_CLEAR-ing it — so the LOAD_ATTR_SLOT reader's atomic-load-plus-try-incref never increfs a freed object. What remains is exactly a plain pointer store (self->buffer = NULL) vs an atomic pointer load on the aligned slot: real UB that TSan correctly flags, but the reader only ever observes the old valid pointer or NULL. So the atomic-store fix is right as UB hygiene; there doesn't appear to be a memory-safety issue behind it.
Checked and drafted with AI assistance (Claude Code), verified by hand on free-threaded debug/ASan builds of main. kumaraditya303 commented on Jul 30, 2026 The fix should change buffer to be a getset descriptor and use critical section to access it.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|