Repository navigation
slot.__delete__() behaves as non-atomic in free-threading #146270
Description
Bug report
Bug description:
In #119368+#123211, member descriptors (related to user-defined slots) were made thread-safe. But not everywhere. The branch related to deletion remained unchanged:
Lines 174 to 187 in 83360b5
Since threads perform the check (addr == NULL) in a non-thread-safe manner, the operation may succeed even if it has already been executed by another thread. At the Python level, this means that an AttributeError may not be raised during two or more concurrent attempts to delete an attribute, and as a result, code that was supposed to execute only once (after a successful deletion) will be executed more than once.
The behavior matches what is expected on CPython with GIL and on PyPy (and possibly other interpreters), but not on CPython without GIL. Without slots (using __dict__), the behavior also matches what is expected even in free-threading. Meanwhile, user-defined slots already rely on critical sections, so the simplest solution would be to move the check for Py_T_OBJECT_EX under the critical section (into the case block).
Code to reproduce (del obj.b raises an AttributeError):
#!/usr/bin/env python3
import time
from concurrent.futures import ThreadPoolExecutor
class ObjectWithSlots:
__slots__ = (
"a",
"b",
)
def __init__(self):
self.a = None
self.b = None
def main():
def test():
try:
del obj.a
except AttributeError: # not the first thread
pass
else:
del obj.b
with ThreadPoolExecutor(2) as executor:
start = time.perf_counter()
while time.perf_counter() - start < 1: # one second
obj = ObjectWithSlots()
f1 = executor.submit(test)
f2 = executor.submit(test)
f1.result() # reraise
f2.result() # reraise
if __name__ == "__main__":
main()CPython versions tested on:
3.14
Operating systems tested on:
Linux
Activity
brijkapadia commentedon Mar 22, 2026
I am not able to reproduce the result (
del obj.bdoes not raise any exception) on main. Doesn't the first thread delete bothobj.aandobj.b?x42005e1f commentedon Mar 22, 2026
The result is reproduced in free-threading. Below is my output on CPython 3.14.3t (via mise):
If you cannot reproduce this, you are likely not running in free-threading mode (the successful deletion of both attributes is the expected behavior that should be achieved). Alternatively, other factors may be at play, but it takes me only a fraction of a second to reproduce the issue.
CPython 3.15.0a7+ free-threading build (heads/main:f0aeabc)
x42005e1f commentedon Mar 22, 2026
If you cannot reproduce the issue exactly in free-threading, then it may be due to a difference in the system scheduler. I write my tests specifically based on elapsed time rather than the number of iterations to minimize differences in hardware speed and interpreter implementation (if one machine runs the software twice as fast as another, but both use the same timeslices (e.g., the same 5 milliseconds with GIL), then the first machine’s thread-safety coverage will be twice as bad as the second’s for the same number of iterations; therefore, it seems to me that elapsed time is a much more suitable parameter for reproducibility here, if we ignore the impact of foreign processes). I have a custom scheduler and a laptop from the early 2010s, which may affect reproducibility, since a different scheduler might schedule threads in such a way that two short-term scheduled calls would never actually enter a race condition.
If that is the case, please let me know so I can try to write a test that takes into account the specifics of your scheduler.
I just tested it on modern hardware and the default scheduler. Nope, the issue reproduces there too.
brijkapadia commentedon Mar 22, 2026
When I run it on main with this config:
./configure --with-pydebug --with-thread-sanitizer --disable-gil,I do not getting any error. However, when I downloaded 3.14.3t, I did get an error so there is a chance that this issue has already been fixed.
x42005e1f commentedon Mar 23, 2026
When you call
sys._is_gil_enabled(), do you getFalse?My comment also includes an output for main, commit f0aeabc, where the issue reproduces. The issue has definitely not been fixed, since I can still see it in the source code and can personally reproduce it on main. (The latter is included here purely for illustrative purposes, since thread-safety issues are not always easy to reproduce.)
x42005e1f commentedon Mar 23, 2026
Behavior that occurs in free-threading:
PyMember_SetOne().addr == NULL, getsfalse, but sleeps beforeFT_ATOMIC_STORE_PTR_RELEASE().addr == NULLand also getsfalse(since Thread A did not have time to change the value ofaddr).AttributeErroratdel obj.a, and as a result, both threads will attempt to dodel obj.b(and either enter a race condition again, or one of them will raise anAttributeError).A classic scenario, nothing new. Incorrect sequences such as
if not loop.is_closed(): loop.call_soon_threadsafe(...)also have this nature (unfortunately, I sometimes see this in various projects; here it would be better to use a try-except onRuntimeErrorinstead, since otherwise the event loop may close before the call but after the check), and everything else that for some reason tries to use simple checks in a multithreaded environment.x42005e1f commentedon Mar 23, 2026
Okay, I built main with your configuration via Docker (reusing the environment from
python:latest), and I understood why you could not reproduce the issue (apologies for my tone; I have a specific background in working with people). By process of elimination, I concluded that TSan (--with-thread-sanitizer) is to blame, as it affects the test's behavior so significantly that... Let me put it this way: I ran 10 tests with a timeout of 60 (one minute, see the while loop condition), and only 2 of them were able to reproduce the issue (with the same output). Moreover, TSan printed a warning in only one of the remaining 8 runs.output-1&2.txt (reproduced)
output-3.txt (not reproduced)
I also tried running the test for 10 minutes:
No errors, no warnings. All subsequent tries (11) after that were successful: 52.82±37.68 secs. In terms of the number of iterations required for reproduction, TSan increases them by about 20 times (~2K → ~40K).
colesbury commentedon Mar 23, 2026
Yeah, this looks like a bug to me. We should handle
Py_T_OBJECT_EXin the delete code path the same way we handle it further on inPyMember_SetOne.diegorusso commentedon Mar 25, 2026
I guess this is something that @dpdani could take
dpdani commentedon Mar 25, 2026
Oops, it seems I forgot looking into that code path back then. It is definitely a bug, thank you for reporting that 👍
x42005e1f commentedon Mar 25, 2026
I should note that it would be possible to handle both
__set__()and__delete__()without per-object locks, simply by using something like_Py_atomic_exchange_ptr()frompyatomic.h(and raising anAttributeErrorwhen both the new and old values areNULL). There is no wrapper for it inpycore_pyatomic_ft_wrappers.h, but as I saw in one PR, it can be used with#ifdef Py_GIL_DISABLED.Oh, wait, I saw the comments about this in the discussion of the first PR mentioned. Never mind.
Hypothetically, it could be used reliably in
PyMember_SetOne()if it were also used inPyMember_GetOne(): the getter would exchange the value with some special value, thereby preventing the setter from performing a decref. It would then attempt a compare-and-exchange back, and perform an incref only if successful. That way, only the getters would have to be synchronized. But this is a dubious benefit, and support for the special value would also have to be handled by C API users (or not? if they always use per-object locks or if some special value is safe, then the new behavior would not make any difference except for the possibility of concurrent writes while holding the lock, but I know too little about C API use cases). So yes, it seems that there is not much that can be done here.On the other hand, the technique I described optimizes my use case (
slot.__delete__()as an atomic Python-level test-and-set). The optimistic path ofPyMember_GetOne()is not going anywhere, so in the best-case scenario (read-only or write-only), all operations will be performed without per-object locks. But there is the problem with the special value...gh-146270: Fix `PyMember_SetOne(..., NULL)` not being atomic (gh-148800)
[3.14] gh-146270: Fix `PyMember_SetOne(..., NULL)` not being atomic (g…
colesbury commentedon May 6, 2026
Thanks for the bug report @x42005e1f and @dpdani for fixing this