Repository navigation
LIST_APPEND and SET_ADD opcodes safety in Free Threading #152288
Description
Hi,
While reviewing PR gh-152273, I moved _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); check to set_add_entry_takeref() to check for bugs. I discovered that SET_ADD opcode calls _PySet_AddTakeRef() without getting the set critical section. That's surprising and IMO it would deserve adding a comment to explain why it's safe to omit locking here.
LIST_APPEND has a similar design (also omit locking).
The following code uses the evil gc.get_objects() function to call set.add() in a different thread while the main thread is building a set:
import dis
import gc
import threading
GLOBAL_SET = None
def worker():
EVENT.wait()
obj = GLOBAL_SET
for i in range(100):
# Call set.add() which uses the critical section
obj.add(i)
MARKER = b"MARKER".decode()
EVENT = threading.Event()
class EvilHash:
def __init__(self, hash_value):
self.hash_value = hash_value
def __hash__(self):
global GLOBAL_SET, EVENT
if GLOBAL_SET is None:
# Invoke the evil gc.get_objects()!
for obj in gc.get_objects():
if isinstance(obj, set) and MARKER in obj:
GLOBAL_SET = obj
EVENT.set()
return self.hash_value
def __repr__(self):
return f"EvilHash({self.hash_value})"
def func():
build_set = {
# Use *list so following items are added by SET_ADD opcode
*[MARKER],
# Added by SET_ADD which calls _PySet_AddTakeRef()
# without the critical section
EvilHash(0), EvilHash(1), EvilHash(2), EvilHash(3), EvilHash(4),
EvilHash(5)}
print(build_set)
print("Length:", len(build_set))
if len(build_set) != 107:
raise Exception("race condition!")
thread = threading.Thread(target=worker)
thread.start()
func()
thread.join()
#dis.dis(func)I expected the code to fail randomly, but so far I failed to trigger a race condition on Free Threading. Running the code on Python built with --with-thread-sanitizer doesn't show any warning.
Activity
corona10 commented on Jun 26, 2026
cc @colesbury
vstinner commented on Jun 26, 2026
I created this issue while working on issues about immutable types (frozendict and frozenset) being exposed by the GC while building new collections:
vstinner commented on Jun 26, 2026
If we care about fixing the evil case using gc.get_objects() to be 100% safe, the following bytecode:
should use new (hypothetical) UNTRACKED and GC_TRACK opcodes. Something like:
kumaraditya303 commented on Jul 6, 2026
FWIW I disagree that this needs to be fixed at all. get_objects is a problematic API and should be used for debugging only in specific cases. get_objects exposes all GC objects and it breaks code which relies on check if object has 1 refcnt and mutate it which is a well known optimization technique used in C code.
I'd like to know the use-case of why fixing this is important otherwise I am strong -1 on this and similar changes.
vstinner commented on Jul 6, 2026
Well, at the beginning, I just asked for adding a comment on LIST_APPEND and SET_ADD opcodes to explain why it's safe to omit locking here :-) I'm fine with not fixing the corner case of gc.get_objects().
gc.get_referrers() documentation has this warning: