diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index b15c48092ce57..4427d70f51afb 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -1190,21 +1190,22 @@ def generate_getter(cl: ClassIR, attr: str, rtype: RType, emitter: Emitter) -> N attr_expr = f"self->{attr_field}" if IS_FREE_THREADED and is_simple_refcounted_pointer(rtype): - # In free-threaded builds, load the attribute and take a new reference - # atomically to avoid a use-after-free race with a concurrent setter. + # In free-threaded builds, load the attribute and take a new reference with + # an optimistic validated incref to avoid racing with a concurrent setter. # CPy_GetAttrRef returns NULL if the attribute is undefined (NULL field), # which is exactly the error/undefined value for a 'PyObject *' field. # # Final attributes are never rebound (no setter), so there is no concurrent # writer to race with: a plain load + incref is safe. Use the cheaper - # CPy_GetAttrRefFinal, which skips the try-incref and _Py_NewRefWithLock - # slow path entirely (an unconditional Py_INCREF needs no maybe-weakref). + # CPy_GetAttrRefFinal, which skips the try-incref and the locked + # _Py_XNewRefWithLock fallback entirely (an unconditional Py_INCREF needs no + # maybe-weakref). # This getter is generated per defining class, so a direct membership test # matches the read-only getset table above (no need to walk the MRO). if attr in cl.final_attributes: getattr_ref = f"CPy_GetAttrRefFinal((PyObject **)&{attr_expr})" else: - getattr_ref = f"CPy_GetAttrRef((PyObject **)&{attr_expr})" + getattr_ref = f"CPy_GetAttrRef((PyObject *)self, (PyObject **)&{attr_expr})" emitter.emit_line(f"PyObject *retval = {getattr_ref};") emitter.emit_line("if (unlikely(retval == NULL)) {") emitter.emit_line("PyErr_SetString(PyExc_AttributeError,") @@ -1253,11 +1254,12 @@ def generate_setter(cl: ClassIR, attr: str, rtype: RType, emitter: Emitter) -> N emitter.emit_line("}") if IS_FREE_THREADED and is_simple_refcounted_pointer(rtype): - # In free-threaded builds, publish the new value atomically via - # CPy_SetAttrRef so a concurrent reader (see CPy_GetAttrRef) never sees a - # torn pointer or a freed old value. CPy_SetAttrRef steals its value and - # reclaims the old one, so we cast/type-check the incoming value, take a - # new reference (the setter only borrows 'value'), then hand it over. + # In free-threaded builds, publish the new value via CPy_SetAttrRef, which + # takes the owner's critical section so a concurrent reader (see + # CPy_GetAttrRef) can always secure a reference to the value it observes, + # even though the old value is decrefed right away. CPy_SetAttrRef steals its + # value and reclaims the old one, so we cast/type-check the incoming value, + # take a new reference (the setter only borrows 'value'), then hand it over. # A NULL value deletes the attribute (reclaims the old value, stores NULL). if deletable: emitter.emit_line("if (value != NULL) {") @@ -1267,10 +1269,14 @@ def generate_setter(cl: ClassIR, attr: str, rtype: RType, emitter: Emitter) -> N emitter.emit_cast("value", "tmp", rtype, declare_dest=True) emitter.emit_lines("if (!tmp)", " return -1;") emitter.emit_inc_ref("tmp", rtype) - emitter.emit_line(f"CPy_SetAttrRef((PyObject **)&self->{attr_field}, tmp);") + emitter.emit_line( + f"CPy_SetAttrRef((PyObject *)self, (PyObject **)&self->{attr_field}, tmp);" + ) if deletable: emitter.emit_line("} else {") - emitter.emit_line(f"CPy_SetAttrRef((PyObject **)&self->{attr_field}, NULL);") + emitter.emit_line( + f"CPy_SetAttrRef((PyObject *)self, (PyObject **)&self->{attr_field}, NULL);" + ) emitter.emit_line("}") emitter.emit_line("return 0;") emitter.emit_line("}") diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 78746e1434331..28e6110361e9d 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -407,10 +407,12 @@ def emit_load_attr_take_ref( """Emit the load of a native attribute into 'dest', taking a new reference. On free-threaded builds, reading a single reference-counted 'PyObject *' field - and taking a new reference must be done atomically to avoid a use-after-free - race with a concurrent setter. CPy_GetAttrRef performs the load and incref - atomically and returns a new reference (or NULL if undefined), so callers must - NOT emit a separate inc_ref. Return True in that case so the caller can skip it. + and taking a new reference must be synchronized to avoid a use-after-free race + with a concurrent setter. CPy_GetAttrRef uses an optimistic validated incref + with a locked fallback and returns a new reference (or NULL if undefined, which + on a free-threaded build includes an attribute deleted by another thread while + the read was in flight), so callers must NOT emit a separate inc_ref. Return + True in that case so the caller can skip it. Final attributes are never rebound (no setter), so there is no concurrent writer and no use-after-free window; an owned read uses the cheaper CPy_GetAttrRefFinal @@ -425,7 +427,9 @@ def emit_load_attr_take_ref( if use_get_attr_ref and cl.is_final_attr(op.attr): self.emitter.emit_line(f"{dest} = CPy_GetAttrRefFinal((PyObject **)&{attr_expr});") elif use_get_attr_ref: - self.emitter.emit_line(f"{dest} = CPy_GetAttrRef((PyObject **)&{attr_expr});") + self.emitter.emit_line( + f"{dest} = CPy_GetAttrRef((PyObject *){obj}, (PyObject **)&{attr_expr});" + ) else: self.emitter.emit_line(f"{dest} = {attr_expr};") return use_get_attr_ref @@ -580,19 +584,23 @@ def visit_set_attr(self, op: SetAttr) -> None: self.emitter.emit_error_check(tmp, ret_type, f"{dest} = 0;") elif IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype): # In free-threaded builds, publishing a single reference-counted - # 'PyObject *' field must be atomic so a concurrent reader (see - # CPy_GetAttrRef) never observes a torn pointer or a freed value. - # Both helpers steal the reference to src. + # 'PyObject *' field must be synchronized with concurrent readers (see + # CPy_GetAttrRef): the store is atomic, and CPy_SetAttrRef additionally + # takes the owner's critical section, which is what lets it decref the + # old value right away. Both helpers steal the reference to src. attr_expr = self.get_attr_expr(obj, op, decl_cl) if op.is_init: - # The attribute is known to be previously undefined (NULL), so - # there is no old value to reclaim; a relaxed store suffices - # (self's later publication provides the release barrier -- see - # CPy_InitAttrRef). + # The attribute is known to be previously undefined (NULL) and self + # can't have leaked yet, so there is no old value to reclaim and no + # competing writer; a relaxed store suffices (self's later publication + # provides the release barrier -- see CPy_InitAttrRef). self.emitter.emit_line(f"CPy_InitAttrRef((PyObject **)&{attr_expr}, {src});") else: - # Atomically swap in the new value and reclaim the old one. - self.emitter.emit_line(f"CPy_SetAttrRef((PyObject **)&{attr_expr}, {src});") + # Replace the value under the owner's critical section and reclaim + # the old one with a normal decref. + self.emitter.emit_line( + f"CPy_SetAttrRef((PyObject *){obj}, (PyObject **)&{attr_expr}, {src});" + ) if op.error_kind == ERR_FALSE: self.emitter.emit_line(f"{dest} = 1;") else: diff --git a/mypyc/lib-rt/mypyc_util.h b/mypyc/lib-rt/mypyc_util.h index d7a3eb3214bed..4048fe93aa92c 100644 --- a/mypyc/lib-rt/mypyc_util.h +++ b/mypyc/lib-rt/mypyc_util.h @@ -17,10 +17,13 @@ #if defined(__clang__) || defined(__GNUC__) #define CPy_NOINLINE __attribute__((noinline)) +#define CPy_COLD __attribute__((cold)) #elif defined(_MSC_VER) #define CPy_NOINLINE __declspec(noinline) +#define CPy_COLD #else #define CPy_NOINLINE +#define CPy_COLD #endif #ifndef Py_GIL_DISABLED diff --git a/mypyc/lib-rt/pythonsupport.c b/mypyc/lib-rt/pythonsupport.c index a8f5a4f4ad4ea..d06d734ff1f4d 100644 --- a/mypyc/lib-rt/pythonsupport.c +++ b/mypyc/lib-rt/pythonsupport.c @@ -6,23 +6,33 @@ #include "pythonsupport.h" #ifdef Py_GIL_DISABLED -// Cold slow path of CPy_GetAttrRef (declared in pythonsupport.h). Reached only -// when the inline fast-path try-incref fails: the value is owned by another -// thread, so taking a reference requires an atomic shared-refcount operation. -// Kept out-of-line so the inline fast path stays small. +// Cold slow path of CPy_GetAttrRef (declared in pythonsupport.h). First try to +// acquire a shared reference without locking, then validate that the field still +// contains v. If validation fails, drop the provisional reference. If the shared +// incref or validation fails, take the owner's critical section, reload the field, +// and take a reference while the value cannot be replaced. _Py_XNewRefWithLock +// also sets maybe-weakref lazily, so later cross-thread reads generally use the +// lock-free shared-refcount path. Returns NULL if the field is NULL on reload. // -// First try the lock-free shared-refcount CAS. If the value has not had -// maybe-weakref set yet (for example, it was published by CPy_InitAttrRef), force -// a cross-thread reference via _Py_NewRefWithLock, which cannot fail and sets -// maybe-weakref so subsequent reads take the fast path. The value was already -// observed in the field by CPy_GetAttrRef; CPy_SetAttrRef's QSBR-delayed decref -// keeps any replaced value alive long enough for this reader. +// The reload is deliberately relaxed, where CPython 3.14 uses a consume load in the +// equivalent locked read (_PyObject_TryGetInstanceAttribute): acquiring the owner's +// critical section already pairs with CPy_SetAttrRef's release store, and a value +// published by CPy_InitAttrRef is ordered by the publication of the owner itself +// (see CPy_InitAttrRef). CPy_NOINLINE -PyObject *CPy_GetAttrRefSlow(PyObject *v) { +CPy_COLD +PyObject *CPy_GetAttrRefSlow(PyObject *v, PyObject *owner, PyObject **field) { if (_Py_TryIncRefShared(v)) { - return v; + if (v == (PyObject *)_Py_atomic_load_ptr(field)) { + return v; + } + Py_DECREF(v); } - return _Py_NewRefWithLock(v); // sets maybe-weakref; cannot fail + PyObject *result; + Py_BEGIN_CRITICAL_SECTION(owner); + result = _Py_XNewRefWithLock((PyObject *)_Py_atomic_load_ptr_relaxed(field)); + Py_END_CRITICAL_SECTION(); + return result; } #endif diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index 33c5a596d1747..b97d19e14dc9d 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -44,30 +44,59 @@ extern "C" { // // On free-threaded builds a plain load followed by an incref races with a // concurrent setter that may decref the old value to zero and free it before the -// incref runs (use-after-free). CPy_SetAttrRef avoids that by reclaiming old -// values through QSBR-delayed decref, so a value observed in the field remains -// safe to touch while this reader is running. +// incref runs (use-after-free). We avoid that with an optimistic try-incref, +// validating the field when taking a shared reference. If no reference can be +// secured that way, the fallback takes the same per-object lock as CPy_SetAttrRef +// and reloads the field before taking a reference. This mirrors CPython's own +// instance attribute read (in CPython 3.14: _Py_TryIncrefCompare, then a locked +// reload -- see _PyObject_TryGetInstanceAttribute in Objects/dictobject.c). // -// Only the hot case is inlined here: an incref of a value owned by this thread or -// immortal, via '_Py_TryIncrefFast' (no CAS, no loop). Everything colder -- the -// cross-thread shared-refcount CAS and the _Py_NewRefWithLock fallback -- lives -// out-of-line in 'CPy_GetAttrRefSlow'. Splitting it this way keeps each call -// site's fast path small enough to inline, which is measurably faster than -// letting the compiler auto-out-line the whole helper (that merges every read -// site's branch history into one shared copy and mispredicts). It is only used in -// free-threaded builds; the default (GIL) build keeps the plain load + incref -// generated inline by mypyc. -PyObject *CPy_GetAttrRefSlow(PyObject *v); - -static inline PyObject *CPy_GetAttrRef(PyObject **field) { +// The load and _Py_TryIncrefFast can therefore touch a 'v' that a concurrent +// CPy_SetAttrRef has already decrefed to zero and freed, since that decref is a +// plain Py_XDECREF. Two invariants make that safe, and they are what let +// CPy_SetAttrRef free the old value immediately instead of deferring it. Both were +// verified against CPython 3.14 and depend on interpreter internals, so they must +// be rechecked when adding support for a new Python version: +// - Reading the header of a freed object cannot fault. All three object heaps +// set 'page_use_qsbr' (see Python/pystate.c), so a freed block's page is not +// unmapped or handed to another size class while any thread is attached. +// - _Py_TryIncrefFast cannot succeed on stale memory. The current thread does +// not allocate between the field load and the try-incref, so the block cannot +// have been reused for an object owned by this thread; a freed block reads +// ob_tid as 0 or as mimalloc's free-list pointer (which overwrites only the +// first word, i.e. ob_tid) and ob_ref_local as 0, so neither the +// owned-by-this-thread test nor the immortal test can fire. +// If the block was reused for a live object at the same address, that object is +// either the field's current value (so returning it is correct) or the field +// validation fails and the provisional reference is dropped again, leaving its +// refcount unchanged. ob_ref_shared of a freed block is 0 or _Py_REF_MERGED, so +// _Py_TryIncRefShared fails on it and the reader falls into the locked path. +// +// The hot path is lock-free and intentionally small: _Py_TryIncrefFast handles +// values owned by this thread and immortal values. Everything colder -- the +// shared-refcount CAS, field validation, and per-object lock fallback -- is in +// CPy_GetAttrRefSlow. The cold annotation also prevents those arguments and +// branches from bloating the caller's hot path. An unflagged cross-thread value +// takes the locked slow path once; _Py_XNewRefWithLock sets maybe-weakref lazily, +// so subsequent reads generally succeed through the lock-free shared-refcount +// path. The default (GIL) build keeps the plain load + incref generated inline by +// mypyc. +// +// Note that observing a non-NULL 'v' does not guarantee a non-NULL result: 'v' is +// only a provisional read, and if the attribute is deleted before a reference to it +// can be secured, this returns NULL and the caller raises AttributeError. That is a +// legal outcome for a racing read, since it is ordered after the delete. +CPy_COLD PyObject *CPy_GetAttrRefSlow(PyObject *v, PyObject *owner, PyObject **field); + +static inline PyObject *CPy_GetAttrRef(PyObject *owner, PyObject **field) { PyObject *v = (PyObject *)_Py_atomic_load_ptr_acquire(field); if (v == NULL) { return NULL; } - if (_Py_TryIncrefFast(v)) { + if (likely(_Py_TryIncrefFast(v))) { return v; } - return CPy_GetAttrRefSlow(v); + return CPy_GetAttrRefSlow(v, owner, field); } // Read a native attribute that is a single reference-counted 'PyObject *' field @@ -78,10 +107,10 @@ static inline PyObject *CPy_GetAttrRef(PyObject **field) { // use-after-free race that CPy_GetAttrRef guards against cannot happen: the field // holds a strong reference for the object's whole lifetime, and any thread reading // it necessarily holds 'self', which keeps the value alive. So the try-incref + -// _Py_NewRefWithLock fallback are unnecessary here -- a plain load + Py_INCREF is -// safe. A cross-thread Py_INCREF is an unconditional -// atomic add on ob_ref_shared, so (unlike CPy_GetAttrRef's try-incref) it needs no -// maybe-weakref and has no slow path. The load is relaxed rather than acquire: the +// _Py_XNewRefWithLock fallback are unnecessary here -- a plain load + Py_INCREF is +// safe. A cross-thread Py_INCREF is an unconditional atomic add on ob_ref_shared +// (CPython 3.14), so (unlike CPy_GetAttrRef's try-incref) it needs no maybe-weakref +// and has no slow path. The load is relaxed rather than acquire: the // reader reached 'self' through a synchronization edge (self's own publication) // that already ordered the construction stores before it, exactly as with // CPy_InitAttrRef's relaxed store. Relaxed keeps it TSan-clean at zero cost (plain @@ -94,57 +123,39 @@ static inline PyObject *CPy_GetAttrRefFinal(PyObject **field) { return v; } -// Reclaim the previous value of a native attribute after it has been replaced. -// -// CPy_GetAttrRef reads the field optimistically without holding any lock the -// writer also takes, so a reader can load the old pointer and then try to take a -// reference after this store. The old value must therefore stay alive until every -// thread has passed a quiescent point, which is exactly what a QSBR-deferred -// decref guarantees. So all mortal old values are -// reclaimed via _PyObject_XDecRefDelayed, matching CPython's own replace-a-slot -// paths (e.g. _PyObject_SetDict / _PyObject_SetManagedDict). -// -// We deliberately do NOT take a "local refcount > 1, owned by this thread" fast -// path: dropping the field's reference is not the only decref of the object, so a -// non-freeing local decrement here does not prevent an unrelated reference holder -// from driving the object to zero (a plain, non-deferred Py_DECREF -> _Py_Dealloc) -// while an in-flight reader still holds the stale pointer -- a use-after-free that -// only QSBR deferral closes. Immortal objects are never freed, so skipping their -// decref entirely is safe and avoids queuing a no-op onto the delayed-free list. -static inline void CPy_DecRefAttrOld(PyObject *op) { - if (op == NULL) { - return; - } - if (_Py_IsImmortal(op)) { - return; - } - _PyObject_XDecRefDelayed(op); -} - // Set a native attribute that is a single reference-counted 'PyObject *' field, // stealing the reference to 'value' (which may be NULL to delete the attribute) // and safely reclaiming the previous value. // -// Memory safety does NOT depend on SetMaybeWeakref here, so (unlike an earlier -// version) we do not call it. Two things keep this safe: -// - The old value is reclaimed via CPy_DecRefAttrOld, a QSBR-deferred decref, so -// it cannot be freed while an in-flight CPy_GetAttrRef still holds the stale -// pointer. -// - A concurrent cross-thread reader whose inline fast-path try-incref fails on -// an unflagged 'value' still cannot fail and never blocks on this writer: -// CPy_GetAttrRef falls into CPy_GetAttrRefSlow, which is fully lock-free. It -// retries with a lock-free shared-refcount CAS (_Py_TryIncRefShared) and, only -// if that also fails, forces a reference via _Py_NewRefWithLock, which cannot -// fail and lazily sets maybe-weakref so later cross-thread reads take the CAS. -// This mirrors CPy_InitAttrRef, which already omits SetMaybeWeakref for the same -// reason. Setting the flag here would only be a possible performance tuning knob -// (it would let that first cross-thread reader succeed on the cheaper -// _Py_TryIncRefShared CAS instead of falling through to _Py_NewRefWithLock); it is -// not needed for correctness. The atomic exchange publishes the new pointer and -// hands back the old one without a writer/writer race. -static inline void CPy_SetAttrRef(PyObject **field, PyObject *value) { - PyObject *old = (PyObject *)_Py_atomic_exchange_ptr(field, value); - CPy_DecRefAttrOld(old); +// The owner's critical section serializes writers and synchronizes them with +// CPy_GetAttrRef's fallback path. A lock-free reader either secures a local or +// immortal reference directly, or secures a shared reference and validates the +// field. Otherwise it reloads the field under this lock. Thus once the replacement +// is published, the old field reference can be decrefed immediately, even if a +// reader is still dereferencing the old value (see CPy_GetAttrRef for why touching +// a freed value there is safe). Decref outside the critical section so an +// arbitrary destructor does not run while the owner is locked -- the equivalent in +// CPython 3.14 (store_instance_attr_lock_held) decrefs with the lock still held. +// +// Caveat (based on CPython 3.14 internals): this takes 'owner->ob_mutex', which for +// a native class with a built-in base is the same mutex CPython uses for that +// container's own critical sections, and CPython runs arbitrary Python code under it +// (e.g. _PyDict_SetItem_LockHeld calls __hash__/__eq__ while holding CS(dict)). +// Re-entering a critical section on the same object is only free when the held +// section is the top-most one; with an unrelated section still active in between, +// the thread parks, detaches, has this mutex released by +// _PyCriticalSection_SuspendAll, then re-locked by _PyCriticalSection_Resume, and +// parks again. Reaching that needs a callback under +// two nested critical sections to assign an attribute of the outer object; the +// helpers here can't cause it on their own, since they only ever hold this one +// mutex (so there is also no lock-ordering deadlock). +static inline void CPy_SetAttrRef(PyObject *owner, PyObject **field, PyObject *value) { + PyObject *old; + Py_BEGIN_CRITICAL_SECTION(owner); + old = (PyObject *)_Py_atomic_load_ptr_relaxed(field); + _Py_atomic_store_ptr_release(field, value); + Py_END_CRITICAL_SECTION(); + Py_XDECREF(old); } // Initialize a native attribute that is known to be previously undefined (NULL), @@ -158,10 +169,10 @@ static inline void CPy_SetAttrRef(PyObject **field, PyObject *value) { // returned), and that publication carries the release barrier making all the // construction stores visible. A relaxed store therefore suffices. // -// Unlike CPy_SetAttrRef, this deliberately does NOT call SetMaybeWeakref (its CAS -// is pure overhead here, ~+2.6ns per fresh store, and construction-heavy code -// pays it on every attribute of every new object). The cost is moved off this hot -// path onto CPy_GetAttrRef's cold slow path, which sets maybe-weakref lazily on +// This deliberately does NOT call SetMaybeWeakref (its CAS is pure overhead here, +// ~+2.6ns per fresh store, and construction-heavy code pays it on every attribute +// of every new object). CPy_SetAttrRef likewise leaves the flag unset. The cost is +// moved onto CPy_GetAttrRef's cold slow path, which sets maybe-weakref lazily on // the first cross-thread read that needs it. static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { _Py_atomic_store_ptr_relaxed(field, value); diff --git a/mypyc/test-data/run-threading.test b/mypyc/test-data/run-threading.test index 8af8755b76c44..b169a6d133ec5 100644 --- a/mypyc/test-data/run-threading.test +++ b/mypyc/test-data/run-threading.test @@ -1,5 +1,135 @@ # Test cases for librt.threading (compile and run) +[case testConcurrentNativeAttributeGetSet] +from typing import Any +import threading +import sys + +class Item: + def __init__(self, value: int) -> None: + self.value = value + +class Box: + def __init__(self) -> None: + self.item = Item(0) + +def native_writer(box: Box, count: int) -> None: + for i in range(count): + box.item = Item(i) + +def native_reader(box: Box, count: int) -> None: + for _ in range(count): + item = box.item + assert item.value >= 0 + +def dynamic_writer(box: Any, count: int) -> None: + for i in range(count): + box.item = Item(i) + +def dynamic_reader(box: Any, count: int) -> None: + for _ in range(count): + item = box.item + assert item.value >= 0 + +def test_attribute_set_decrefs_old_value_immediately() -> None: + box = Box() + dynamic_sys: Any = sys + old = box.item + before = dynamic_sys.getrefcount(old) + box.item = Item(1) + assert dynamic_sys.getrefcount(old) == before - 1 + + dynamic: Any = box + old = box.item + before = dynamic_sys.getrefcount(old) + dynamic.item = Item(2) + assert dynamic_sys.getrefcount(old) == before - 1 + +def test_concurrent_native_attribute_get_set() -> None: + # Native access exercises the direct generated calls. Access through Any uses + # the generated getset descriptor. On a free-threaded build all four workers + # race, repeatedly dropping the field's last reference to the previous Item. + box = Box() + count = 10_000 + threads = [ + threading.Thread(target=native_writer, args=(box, count)), + threading.Thread(target=native_reader, args=(box, count)), + threading.Thread(target=dynamic_writer, args=(box, count)), + threading.Thread(target=dynamic_reader, args=(box, count)), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert box.item.value >= 0 + +[case testConcurrentNativeAttributeDelete] +from typing import Any +import threading + +class Item: + def __init__(self, value: int) -> None: + self.value = value + +class DelBox: + __deletable__ = ["item"] + + def __init__(self) -> None: + self.item = Item(0) + +def native_setter(box: DelBox, count: int) -> None: + for i in range(count): + box.item = Item(i) + +def native_deleter(box: DelBox, count: int) -> None: + for _ in range(count): + try: + del box.item + except AttributeError: + pass + +def native_reader(box: DelBox, count: int) -> None: + for _ in range(count): + try: + item = box.item + except AttributeError: + continue + assert item.value >= 0 + +def dynamic_deleter(box: Any, count: int) -> None: + for _ in range(count): + try: + del box.item + except AttributeError: + pass + +def dynamic_reader(box: Any, count: int) -> None: + for _ in range(count): + try: + item = box.item + except AttributeError: + continue + assert item.value >= 0 + +def test_concurrent_native_attribute_delete() -> None: + # A deleted attribute is the only way a read that already observed a live value + # can still fail: the optimistic incref can miss and the locked reload then sees + # NULL, so the read raises AttributeError (see CPy_GetAttrRefSlow). Readers here + # must therefore tolerate AttributeError, but never see a freed or torn value. + box = DelBox() + count = 10_000 + threads = [ + threading.Thread(target=native_setter, args=(box, count)), + threading.Thread(target=native_deleter, args=(box, count)), + threading.Thread(target=native_reader, args=(box, count)), + threading.Thread(target=dynamic_deleter, args=(box, count)), + threading.Thread(target=dynamic_reader, args=(box, count)), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + [case testLockBasics_librt] from typing import Any from testutil import assertRaises diff --git a/mypyc/test/test_emitclass.py b/mypyc/test/test_emitclass.py index 9c3cd02d1100c..4e34f35ef464e 100644 --- a/mypyc/test/test_emitclass.py +++ b/mypyc/test/test_emitclass.py @@ -3,9 +3,17 @@ import unittest from mypyc.analysis.attrdefined import detect_undefined_bitmap -from mypyc.codegen.emitclass import getter_name, setter_name, slot_key +from mypyc.codegen.emit import Emitter, EmitterContext +from mypyc.codegen.emitclass import ( + generate_getter, + generate_setter, + getter_name, + setter_name, + slot_key, +) +from mypyc.common import IS_FREE_THREADED from mypyc.ir.class_ir import ClassIR -from mypyc.ir.rtypes import int32_rprimitive +from mypyc.ir.rtypes import int32_rprimitive, object_rprimitive from mypyc.namegen import NameGenerator @@ -36,6 +44,24 @@ def test_getter_name(self) -> None: assert getter_name(cls, "down", generator) == "testing___SomeClass_get_down" + @unittest.skipUnless(IS_FREE_THREADED, "requires a free threaded build") + def test_free_threaded_ref_attribute_getter_and_setter_use_owner(self) -> None: + # Note: We can't monkey patch IS_FREE_THREADED to test this on a build with + # the GIL enabled, since monkey patching doesn't work if mypyc is compiled. + cl = ClassIR("A", "mod") + cl.attributes = {"o": object_rprimitive} + cl.deletable = ["o"] + cl.mro = cl.base_mro = [cl] + emitter = Emitter(EmitterContext(NameGenerator([["mod"]]), True)) + + generate_getter(cl, "o", object_rprimitive, emitter) + generate_setter(cl, "o", object_rprimitive, emitter) + + generated = "".join(emitter.fragments) + assert "CPy_GetAttrRef((PyObject *)self, (PyObject **)&self->_o)" in generated + assert "CPy_SetAttrRef((PyObject *)self, (PyObject **)&self->_o, tmp)" in generated + assert "CPy_SetAttrRef((PyObject *)self, (PyObject **)&self->_o, NULL)" in generated + def test_bitmap_attrs_stable_across_repeat_analysis(self) -> None: # Regression: detect_undefined_bitmap used to mutate cl.bitmap_attrs # in place, so under separate=True (one SCC per group) a shared base diff --git a/mypyc/test/test_emitfunc.py b/mypyc/test/test_emitfunc.py index 00cbe603a9b91..adaafbf7af9cc 100644 --- a/mypyc/test/test_emitfunc.py +++ b/mypyc/test/test_emitfunc.py @@ -5,7 +5,7 @@ from mypy.test.helpers import assert_string_arrays_equal from mypyc.codegen.emit import Emitter, EmitterContext from mypyc.codegen.emitfunc import FunctionEmitterVisitor, generate_native_function -from mypyc.common import HAVE_IMMORTAL, PLATFORM_SIZE +from mypyc.common import HAVE_IMMORTAL, IS_FREE_THREADED, PLATFORM_SIZE from mypyc.ir.class_ir import ClassIR from mypyc.ir.func_ir import FuncDecl, FuncIR, FuncSignature, RuntimeArg from mypyc.ir.ops import ( @@ -121,6 +121,7 @@ def add_local(name: str, rtype: RType) -> Register: ir.attributes = { "x": bool_rprimitive, "y": int_rprimitive, + "o": object_rprimitive, "i1": int64_rprimitive, "i2": int32_rprimitive, "t": RTuple([object_rprimitive, object_rprimitive]), @@ -487,6 +488,20 @@ def test_get_attr_non_refcounted(self) -> None: """, ) + # Note: We can't monkey patch IS_FREE_THREADED to test this on a build with the + # GIL enabled, since monkey patching doesn't work if mypyc is compiled. + @unittest.skipUnless(IS_FREE_THREADED, "requires a free threaded build") + def test_get_attr_ref_free_threaded(self) -> None: + self.assert_emit( + GetAttr(self.r, "o", 1), + """\ + cpy_r_r0 = CPy_GetAttrRef((PyObject *)cpy_r_r, (PyObject **)&((mod___AObject *)cpy_r_r)->_o); + if (unlikely(cpy_r_r0 == NULL)) { + PyErr_SetString(PyExc_AttributeError, "attribute 'o' of 'A' undefined"); + } + """, + ) + def test_get_attr_merged(self) -> None: op = GetAttr(self.r, "y", 1) branch = Branch(op, BasicBlock(8), BasicBlock(9), Branch.IS_ERROR) @@ -538,6 +553,16 @@ def test_set_attr(self) -> None: """, ) + @unittest.skipUnless(IS_FREE_THREADED, "requires a free threaded build") + def test_set_attr_ref_free_threaded(self) -> None: + self.assert_emit( + SetAttr(self.r, "o", self.o, 1), + """\ + CPy_SetAttrRef((PyObject *)cpy_r_r, (PyObject **)&((mod___AObject *)cpy_r_r)->_o, cpy_r_o); + cpy_r_r0 = 1; + """, + ) + def test_set_attr_non_refcounted(self) -> None: self.assert_emit( SetAttr(self.r, "x", self.b, 1),