LIST_APPEND and SET_ADD opcodes safety in Free Threading
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 35.9k
- PR merge metrics
- PR metrics pending
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing SET_ADD and LIST_APPEND through _PySet_AddTakeRef() and set_add_entry_takeref(), using the provided threaded reproducer as the behavioral check. Run it on a free-threaded build, including the thread-sanitized build, and determine whether omitting the critical section is safe. Done means the safety rationale is established and any necessary explanation or correction is covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100