python / python/cpython

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

Đang mở
#149,142 4 bình luận 1 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

docs extension-modules topic-free-threading type-bug
Ngôn ngữ chính
Python
Star
77.2k
Fork
35.9k
Chỉ số merge pull request
Chỉ số pull request đang chờ

Mô tả

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:

Line Code Reached from Python by
616 ctx->status |= status; (in dec_addstatus) any arithmetic on the context
617, 625, 629 reads of ctx->traps (in dec_addstatus trap path) same
715 SdFlags(self) & flag (in signaldict_getitem) ctx.flags[X], ctx.traps[X]
744 SdFlags(self) |= flag; (in signaldict_setitem) ctx.flags[X] = True, ctx.traps[X] = True
747 SdFlags(self) &= ~flag; (in signaldict_setitem) ctx.flags[X] = False, ctx.traps[X] = False
1407 CTX(self)->traps = 0; (in _decimal_Context_clear_traps_impl) ctx.clear_traps()
1421 CTX(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.py
import threading
N_THREADS = 8
ITERATIONS = 100_000
def run_concurrently(workers):
    barrier = threading.Barrier(len(workers))
    threads = [threading.Thread(target=w, args=(barrier,)) for w in workers]
    for t in threads: t.start()
    for t in threads: t.join()
1. dec_addstatus (:616)
# repro_status_or.py
import decimal
from common import N_THREADS, ITERATIONS, run_concurrently
SHARED = decimal.Context(prec=4)  # prec=4 makes "1.23456" Inexact|Rounded
def worker(barrier):
    barrier.wait()
    for _ in range(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.py
import decimal
from common import ITERATIONS, run_concurrently
SHARED = decimal.Context(prec=4)
def producer(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        SHARED.create_decimal("1.23456")
def clearer(barrier):
    barrier.wait()
    for _ in range(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.py
import decimal
from common import ITERATIONS, run_concurrently
SHARED = decimal.Context(prec=4, traps=[decimal.Inexact])
def producer(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        try:
            SHARED.create_decimal("1.23456")  # reads ctx->traps
        except decimal.Inexact:
            pass
def clearer(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        SHARED.clear_traps()             # ctx->traps = 0;
run_concurrently([producer]*4 + [clearer]*4)
4. signaldict_setitem self-race (:744 / :747)
# repro_signaldict_set.py
import decimal
from common import ITERATIONS, run_concurrently
SHARED = decimal.Context(prec=28)
A, B = decimal.Inexact, decimal.Rounded  # different bits, same word
def setter_a(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        SHARED.flags[A] = True     # |= bit_a
        SHARED.flags[A] = False    # &= ~bit_a
def setter_b(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        SHARED.flags[B] = True
        SHARED.flags[B] = False
run_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.py
import decimal
from common import ITERATIONS, run_concurrently
SHARED = decimal.Context(prec=4)
INEXACT = decimal.Inexact
def producer(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        SHARED.create_decimal("1.23456")
def reader(barrier):
    barrier.wait()
    for _ in range(ITERATIONS):
        _ = SHARED.flags[INEXACT]   # reads ctx->status non-atomically
run_concurrently([producer]*4 + [reader]*4)

Suggested fix

The cleanest free-threading-safe option is a per-Context PyMutex 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

  • #141148 / #146482 — fixes the implicit getcontext()/current_context() inheritance path. After that PR lands, the primitive sites above are still racy whenever a Context is shared explicitly (e.g. Decimal(value, context=shared_ctx) or shared_ctx.create_decimal(s)); see the issue's own MRE for an explicit-share case that #146482 doesn't cover.

Drafted by Claude Code, reviewed by a human.

CPython versions tested on:

CPython main branch, 3.15

Operating systems tested on:

Linux

Linked PRs
  • gh-150598

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu trong Modules/_decimal/_decimal.c tại dec_addstatus, signaldict_getitem, signaldict_setitem và các triển khai của clear_flags/clear_traps. Chạy các mẫu tái hiện được liệt kê trong bản build debug free-threaded với ThreadSanitizer, sau đó xác minh rằng mọi truy cập vào status và traps đều được đồng bộ hóa và các race được báo cáo không còn xảy ra.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
c, python
Lĩnh vực
backend
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Đình trệ
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
30/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.