Skip to content

_decimal: mpd_context_t::status/traps mutated non-atomically leading to data race #149142

Description

@devdanzin

Bug report

Bug description:

Summary

Modules/_decimal/_decimal.c performs unsynchronized read-modify-write and plain stores on the status and traps fields of mpd_context_t embedded in a Python decimal.Context. Whenever a Context instance is reachable from more than one free-threaded thread — explicitly via context= arguments, Context.<method>(…), ctx.flags[…] = …, etc. — those accesses race.

#141148 + #146482 fix the implicit sharing case (inherited context via contextvars). This issue is for the underlying primitive, which is independent of how the Context ended up shared and is still racy after #146482.

Affected sites

All in Modules/_decimal/_decimal.c:

LineCodeReached from Python by
616ctx->status |= status; (in dec_addstatus)any arithmetic on the context
617, 625, 629reads of ctx->traps (in dec_addstatus trap path)same
715SdFlags(self) & flag (in signaldict_getitem)ctx.flags[X], ctx.traps[X]
744SdFlags(self) |= flag; (in signaldict_setitem)ctx.flags[X] = True, ctx.traps[X] = True
747SdFlags(self) &= ~flag; (in signaldict_setitem)ctx.flags[X] = False, ctx.traps[X] = False
1407CTX(self)->traps = 0; (in _decimal_Context_clear_traps_impl)ctx.clear_traps()
1421CTX(self)->status = 0; (in _decimal_Context_clear_flags_impl)ctx.clear_flags()

SdFlags(v) is *v->flags where v->flags is bound to either &CTX(ctx)->status or &CTX(ctx)->traps (_decimal.c:1474–1475), so the signaldict paths are the same memory as the context-level paths via a different surface.

Triggering pattern

Any pure-Python code that shares one Context instance across free-threaded threads. Five minimal reproducers, one per site, are below. They use a barrier so the racing windows align on the first iteration; under TSan on a free-threaded debug build (./configure --disable-gil --with-thread-sanitizer) each one should reliably produce a data race report attributable to the matching site.

# common.pyimportthreadingN_THREADS=8ITERATIONS=100_000defrun_concurrently(workers):
barrier=threading.Barrier(len(workers))
threads= [threading.Thread(target=w, args=(barrier,)) forwinworkers]
fortinthreads: t.start()
fortinthreads: t.join()

1. dec_addstatus (:616)

# repro_status_or.pyimportdecimalfromcommonimportN_THREADS, ITERATIONS, run_concurrentlySHARED=decimal.Context(prec=4) # prec=4 makes "1.23456" Inexact|Roundeddefworker(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.create_decimal("1.23456") # -> dec_addstatus(SHARED, ...)run_concurrently([worker] *N_THREADS)

2. clear_flags race vs. dec_addstatus (:1421:616)

# repro_clear_flags.pyimportdecimalfromcommonimportITERATIONS, run_concurrentlySHARED=decimal.Context(prec=4)
defproducer(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.create_decimal("1.23456")
defclearer(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.clear_flags() # ctx->status = 0;run_concurrently([producer]*4+ [clearer]*4)

3. clear_traps race vs. trap-detection read (:1407:617)

# repro_clear_traps.pyimportdecimalfromcommonimportITERATIONS, run_concurrentlySHARED=decimal.Context(prec=4, traps=[decimal.Inexact])
defproducer(barrier):
barrier.wait()
for_inrange(ITERATIONS):
try:
SHARED.create_decimal("1.23456") # reads ctx->trapsexceptdecimal.Inexact:
passdefclearer(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.clear_traps() # ctx->traps = 0;run_concurrently([producer]*4+ [clearer]*4)

4. signaldict_setitem self-race (:744 / :747)

# repro_signaldict_set.pyimportdecimalfromcommonimportITERATIONS, run_concurrentlySHARED=decimal.Context(prec=28)
A, B=decimal.Inexact, decimal.Rounded# different bits, same worddefsetter_a(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.flags[A] =True# |= bit_aSHARED.flags[A] =False# &= ~bit_adefsetter_b(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.flags[B] =TrueSHARED.flags[B] =Falserun_concurrently([setter_a]*4+ [setter_b]*4)
print("final flags:", dict(SHARED.flags)) # observable lost-update on FT

(Substituting SHARED.traps for SHARED.flags produces the same race on ctx->traps.)

5. signaldict_getitem read vs. dec_addstatus write (:715:616)

# repro_signaldict_get.pyimportdecimalfromcommonimportITERATIONS, run_concurrentlySHARED=decimal.Context(prec=4)
INEXACT=decimal.Inexactdefproducer(barrier):
barrier.wait()
for_inrange(ITERATIONS):
SHARED.create_decimal("1.23456")
defreader(barrier):
barrier.wait()
for_inrange(ITERATIONS):
_=SHARED.flags[INEXACT] # reads ctx->status non-atomicallyrun_concurrently([producer]*4+ [reader]*4)

Suggested fix

The cleanest free-threading-safe option is a per-ContextPyMutex covering all reads and writes of status and traps. The fields are tiny (one uint32_t each) and accessed at very high frequency, so an alternative is to switch to _Py_atomic_or_uint32 / _Py_atomic_and_uint32 / _Py_atomic_load_uint32 / _Py_atomic_store_uint32 directly on the fields. Given that the trap-detection path needs to read traps and status together, atomics-only is a little awkward (the OR-then-test sequence wants both observations to be from the same logical state), so PyMutex is probably the better fit; the lock-free path can be reserved for the hot read in signaldict_getitem if profiling shows the mutex matters.

Either way, the fix should also cover the four CTX(...)->status = 0; resets at :1825, :1901, :1924, :1985 (in current_context_from_dict, PyDec_SetCurrentContext, init_current_context, and PyDec_SetCurrentContext for the contextvar variant) — those are stores into a Context that has just been created or just been swapped in, so they're not strictly racy in the current code, but if any future change exposes them earlier the same atomicity argument applies.

Related

Drafted by Claude Code, reviewed by a human.

CPython versions tested on:

CPython main branch, 3.15

Operating systems tested on:

Linux

Linked PRs

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions