From 1cd520448a6c1ac6b3097e7a0c06beae0d63f255 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 09:42:37 +0100 Subject: [PATCH 01/10] [mypyc] Allow one execution of a generator at a time in FT build This only applies to coroutines. This allows using unsynchronized attribute access, which is a bug win on free-threaded builds. This matches Python semantics. We should perhaps do this in non-FT builds as well, but this is left as a follow-up task, as there it won't directly help with performance. --- mypyc/codegen/emitclass.py | 8 +++- mypyc/codegen/emitfunc.py | 83 +++++++++++++++++++++++++++++++++--- mypyc/codegen/emitmodule.py | 20 ++++++++- mypyc/common.py | 5 +++ mypyc/ir/class_ir.py | 29 +++++++++++++ mypyc/irbuild/generator.py | 15 ++++++- mypyc/lib-rt/CPy.h | 3 ++ mypyc/lib-rt/misc_ops.c | 15 +++++++ mypyc/lib-rt/pythonsupport.h | 30 +++++++++++++ 9 files changed, 199 insertions(+), 9 deletions(-) diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index f8d3d2696b166..da199d60826b6 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -29,6 +29,7 @@ BITMAP_BITS, BITMAP_TYPE, CPYFUNCTION_NAME, + EXCLUSIVE_RESUME_FIELD, IS_FREE_THREADED, MYPYC_DEFAULTS_SETUP, NATIVE_PREFIX, @@ -264,7 +265,7 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None: fields: dict[str, str] = {"tp_name": f'"{name}"'} generate_full = not cl.is_trait and not cl.builtin_base - needs_getseters = cl.needs_getseters or not cl.is_generated or cl.has_dict + needs_getseters = cl.needs_getseters_table if not cl.builtin_base: fields["tp_new"] = new_name @@ -484,6 +485,11 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None: lines += ["typedef struct {", "PyObject_HEAD", "CPyVTableItem *vtable;"] if cl.has_method("__call__"): lines.append("vectorcallfunc vectorcall;") + if cl.uses_exclusive_resume(): + # Exclusive-resume token. Not an IR attribute: it is invisible to the GC, + # to tp_clear and to attribute definedness analysis, and it is never read + # outside the generated helper method's entry and exits. + lines.append(f"uint32_t {EXCLUSIVE_RESUME_FIELD};") bitmap_attrs = [] for base in reversed(cl.base_mro): if not base.is_trait: diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 78746e1434331..2c035c4dd81af 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -13,6 +13,7 @@ c_array_initializer, ) from mypyc.common import ( + EXCLUSIVE_RESUME_FIELD, GENERATOR_ATTRIBUTE_PREFIX, HAVE_IMMORTAL, IS_FREE_THREADED, @@ -129,12 +130,24 @@ def native_function_header(fn: FuncDecl, emitter: Emitter) -> str: def generate_native_function( - fn: FuncIR, emitter: Emitter, source_path: str, module_name: str + fn: FuncIR, + emitter: Emitter, + source_path: str, + module_name: str, + exclusive_resume_class: ClassIR | None = None, ) -> None: + """Emit the C body of a native function. + + If 'exclusive_resume_class' is set, this is the generator helper method of that + class, and the body is wrapped in the class's exclusive-resume token (see + ClassIR.uses_exclusive_resume). + """ declarations = Emitter(emitter.context) names = generate_names_for_ir(fn.arg_regs, fn.blocks) body = Emitter(emitter.context, names) - visitor = FunctionEmitterVisitor(body, declarations, source_path, module_name) + visitor = FunctionEmitterVisitor( + body, declarations, source_path, module_name, exclusive_resume_class + ) declarations.emit_line(f"{native_function_header(fn.decl, emitter)} {{") body.indent() @@ -183,6 +196,11 @@ def generate_native_function( if not is_next_block or is_problematic_op: fn.blocks[target.label].referenced = True + if exclusive_resume_class is not None: + # Emitted before the first label, so resumes enter here but internal jumps + # back to the first block (if any) don't. + visitor.emit_exclusive_resume_enter(fn) + common = frequently_executed_blocks(fn.blocks[0]) for i in range(len(blocks)): @@ -209,13 +227,23 @@ def generate_native_function( class FunctionEmitterVisitor(OpVisitor[None]): def __init__( - self, emitter: Emitter, declarations: Emitter, source_path: str, module_name: str + self, + emitter: Emitter, + declarations: Emitter, + source_path: str, + module_name: str, + exclusive_resume_class: ClassIR | None = None, ) -> None: self.emitter = emitter self.names = emitter.names self.declarations = declarations self.source_path = source_path self.module_name = module_name + # Set if we are emitting the generator helper method of this class, which + # must hold the class's exclusive-resume token while it runs. + self.exclusive_resume_class = exclusive_resume_class + # C expression for the address of that token, set up on function entry + self.exclusive_resume_token: str | None = None self.literals = emitter.context.literals self.rare = False # Next basic block to be processed after the current one (if any), set by caller @@ -291,8 +319,38 @@ def visit_branch(self, op: Branch) -> None: self.emit_lines("} else", " goto %s;" % self.label(false)) + def emit_exclusive_resume_enter(self, fn: FuncIR) -> None: + """Claim the receiver's exclusive-resume token, or fail without running the body. + + Rejecting a concurrent (or reentrant) resume is required for correctness, not + just fidelity to CPython: the body's attribute accesses are only safe on + free-threaded builds because at most one thread is inside it at a time. + """ + cl = self.exclusive_resume_class + assert cl is not None + struct = cl.struct_name(self.names) + self_str = self.reg(fn.arg_regs[0]) + self.exclusive_resume_token = f"&(({struct} *){self_str})->{EXCLUSIVE_RESUME_FIELD}" + token = self.exclusive_resume_token + is_coroutine = 1 if cl.has_method("__await__") else 0 + self.emit_line(f"if (unlikely(!CPyGen_TryEnter({token}))) {{") + self.emit_line(f"return CPyGen_AlreadyExecutingError({is_coroutine});") + self.emit_line("}") + + def emit_exclusive_resume_exit(self) -> None: + """Drop the exclusive-resume token before leaving the generator body. + + Every exit goes through a Return op (error exits included), so this covers + suspension, completion and exceptions alike. The release also publishes + everything the body stored to the next thread that resumes the generator. + """ + assert self.exclusive_resume_token is not None + self.emit_line(f"CPyGen_Exit({self.exclusive_resume_token});") + def visit_return(self, op: Return) -> None: value_str = self.reg(op.value) + if self.exclusive_resume_class is not None: + self.emit_exclusive_resume_exit() self.emit_line("return %s;" % value_str) def visit_tuple_set(self, op: TupleSet) -> None: @@ -418,9 +476,16 @@ def emit_load_attr_take_ref( for attributes safe to borrow on free-threaded builds (Final and vec attrs -- see transform_member_expr in irbuild), whose values live as long as their container. The default (GIL) build always takes the plain-load path and increfs separately. + + Attributes of a generator class with exclusive resume are likewise read with a + plain load: no concurrent writer can exist, since only the thread holding the + token runs the generator body (see ClassIR.uses_exclusive_resume). """ use_get_attr_ref = ( - IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype) and not op.is_borrowed + IS_FREE_THREADED + and is_simple_refcounted_pointer(attr_rtype) + and not op.is_borrowed + and not cl.uses_exclusive_resume() ) if use_get_attr_ref and cl.is_final_attr(op.attr): self.emitter.emit_line(f"{dest} = CPy_GetAttrRefFinal((PyObject **)&{attr_expr});") @@ -578,10 +643,18 @@ def visit_set_attr(self, op: SetAttr) -> None: ) self.emit_line(f"{dest} = 1;") self.emitter.emit_error_check(tmp, ret_type, f"{dest} = 0;") - elif IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype): + elif ( + IS_FREE_THREADED + and is_simple_refcounted_pointer(attr_rtype) + and not cl.uses_exclusive_resume() + ): # 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. + # Generator classes with exclusive resume are exempt: no other thread + # can read or write the field while the token holder runs, so a plain + # store and an immediate decref of the old value are safe (and let the + # old value be freed inline rather than through the QSBR queue). # Both helpers steal the reference to src. attr_expr = self.get_attr_expr(obj, op, decl_cl) if op.is_init: diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 9130007e3f6ef..6ae6a6cc2f284 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -75,7 +75,7 @@ from mypyc.ir.rtypes import RType from mypyc.irbuild.main import build_ir from mypyc.irbuild.mapper import Mapper -from mypyc.irbuild.prepare import load_type_map +from mypyc.irbuild.prepare import GENERATOR_HELPER_NAME, load_type_map from mypyc.namegen import NameGenerator, exported_name from mypyc.options import CompilerOptions from mypyc.transform.copy_propagation import do_copy_propagation @@ -708,12 +708,28 @@ def generate_c_for_modules(self) -> list[tuple[str, str]]: if cl.is_ext_class: generate_class(cl, module_name, emitter) + # Generator classes whose helper method must claim an exclusive-resume + # token. Collected after generating the classes, since that's where the + # token field is added to the object struct. + exclusive_resume_classes = { + cl.name: cl for cl in module.classes if cl.uses_exclusive_resume() + } + # Generate Python extension module definitions and module initialization functions. self.generate_module_def(emitter, module_name, module) for fn in module.functions: emitter.emit_line() - generate_native_function(fn, emitter, self.source_paths[module_name], module_name) + exclusive_resume_class = None + if fn.decl.name == GENERATOR_HELPER_NAME and fn.class_name is not None: + exclusive_resume_class = exclusive_resume_classes.get(fn.class_name) + generate_native_function( + fn, + emitter, + self.source_paths[module_name], + module_name, + exclusive_resume_class, + ) if fn.name != TOP_LEVEL_NAME and not fn.internal: emitter.emit_line() if is_fastcall_supported(fn, emitter.capi_version): diff --git a/mypyc/common.py b/mypyc/common.py index fa34647c5c729..e11157d04af7b 100644 --- a/mypyc/common.py +++ b/mypyc/common.py @@ -28,6 +28,11 @@ GENERATOR_ATTRIBUTE_PREFIX: Final = "__mypyc_generator_attribute__" CPYFUNCTION_NAME = "__cpyfunction__" +# C struct field of a generator object that holds the exclusive-resume token (see +# ClassIR.uses_exclusive_resume). This is not an IR-level attribute, so it can't +# collide with one: those are all prefixed with ATTR_PREFIX. +EXCLUSIVE_RESUME_FIELD: Final = "mypyc_running" + # Max short int we accept as a literal is based on 32-bit platforms, # so that we can just always emit the same code. diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index 3b54331cb2f07..4c694ad460371 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,6 +250,16 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None + # If True, this is a generated generator/coroutine class whose instances may + # only ever be executed by one thread at a time, and this is enforced at + # runtime by an exclusive-resume token in the object (see + # uses_exclusive_resume() and mypyc/codegen/emitfunc.py). All the attributes + # are then private to whoever holds the token, which lets attribute access + # in the generator body skip the concurrency-safe (atomic) attribute + # operations that free-threaded builds otherwise need. Only ever set on + # free-threaded builds, since there is nothing to gain under the GIL. + self.exclusive_resume = False + def __repr__(self) -> str: return ( "ClassIR(" @@ -302,6 +312,23 @@ def is_final_attr(self, name: str) -> bool: return False return False + @property + def needs_getseters_table(self) -> bool: + """Do we generate a tp_getset table exposing the attributes to Python?""" + return self.needs_getseters or not self.is_generated or self.has_dict + + def uses_exclusive_resume(self) -> bool: + """Is execution of this generator class serialized by an exclusive-resume token? + + If so, the generated helper method claims the token on entry and drops it at + every exit, so while the body runs no other thread can touch the instance's + attributes. Attribute access in the body can then use plain (non-atomic) + loads and stores even on free-threaded builds. That reasoning breaks if + anything outside the helper can read or write the attributes concurrently, + which is why getseters must not be generated for the class. + """ + return self.exclusive_resume and not self.needs_getseters_table + def method_decl(self, name: str) -> FuncDecl: for ir in self.mro: if name in ir.method_decls: @@ -470,6 +497,7 @@ def serialize(self) -> JsonDict: "init_self_leak": self.init_self_leak, "env_user_function": self.env_user_function.id if self.env_user_function else None, "reuse_freed_instance": self.reuse_freed_instance, + "exclusive_resume": self.exclusive_resume, "is_acyclic": self.is_acyclic, "is_enum": self.is_enum, "is_coroutine": self.coroutine_name, @@ -533,6 +561,7 @@ def deserialize(cls, data: JsonDict, ctx: DeserMaps) -> ClassIR: ctx.functions[data["env_user_function"]] if data["env_user_function"] else None ) ir.reuse_freed_instance = data["reuse_freed_instance"] + ir.exclusive_resume = data["exclusive_resume"] ir.is_acyclic = data.get("is_acyclic", False) ir.is_enum = data["is_enum"] ir.coroutine_name = data["is_coroutine"] diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index 555baaada0e81..4e666db96d925 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -13,7 +13,12 @@ from collections.abc import Callable from mypy.nodes import ARG_OPT, FuncDef, Var -from mypyc.common import ENV_ATTR_NAME, GENERATOR_ATTRIBUTE_PREFIX, NEXT_LABEL_ATTR_NAME +from mypyc.common import ( + ENV_ATTR_NAME, + GENERATOR_ATTRIBUTE_PREFIX, + IS_FREE_THREADED, + NEXT_LABEL_ATTR_NAME, +) from mypyc.ir.class_ir import ClassIR from mypyc.ir.func_ir import FuncDecl, FuncIR from mypyc.ir.ops import ( @@ -172,6 +177,14 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: generator_class_ir = mapper.fdef_to_generator[builder.fn_info.fitem] if builder.fn_info.can_merge_generator_and_env_classes(): builder.fn_info.env_class = generator_class_ir + # The locals live directly in the generator object, and since the environment + # wasn't split out, no nested function can capture them: every attribute is + # private to the code running the state machine. On free-threaded builds we + # can therefore serialize execution with a single exclusive-resume token and + # use plain attribute access in the body (see ClassIR.uses_exclusive_resume). + # This is pointless under the GIL, where attribute access is already plain. + if IS_FREE_THREADED: + generator_class_ir.exclusive_resume = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) if not builder.fn_info.fitem.is_coroutine: diff --git a/mypyc/lib-rt/CPy.h b/mypyc/lib-rt/CPy.h index e2b4ff0e8c750..a9b2a2e915c58 100644 --- a/mypyc/lib-rt/CPy.h +++ b/mypyc/lib-rt/CPy.h @@ -990,6 +990,9 @@ static inline PyObject *CPy_TYPE(PyObject *obj) { PyObject *CPy_CalculateMetaclass(PyObject *type, PyObject *o); PyObject *CPy_GetCoro(PyObject *obj); +#ifdef Py_GIL_DISABLED +PyObject *CPyGen_AlreadyExecutingError(int is_coroutine); +#endif PyObject *CPyIter_Send(PyObject *iter, PyObject *val); int CPy_YieldFromErrorHandle(PyObject *iter, PyObject **outp); PyObject *CPy_FetchStopIterationValue(void); diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index 392dba0deca4c..9675b82037e75 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -22,6 +22,21 @@ PyObject *CPy_GetCoro(PyObject *obj) } } +#ifdef Py_GIL_DISABLED +// Report a failed attempt to claim a generator's exclusive-resume token (see +// CPyGen_TryEnter): some other thread, or this one reentrantly, is already +// running the body. Always returns NULL, so a helper method can tail-return it. +// Out of line since every generated helper method has one of these, and the +// messages match what CPython's own generators and coroutines raise. +PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) +{ + PyErr_SetString(PyExc_ValueError, + is_coroutine ? "coroutine already executing" + : "generator already executing"); + return NULL; +} +#endif + PyObject *CPyIter_Send(PyObject *iter, PyObject *val) { // Do a send, or a next if second arg is None. diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index 33c5a596d1747..808a7160730cb 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -166,6 +166,36 @@ static inline void CPy_SetAttrRef(PyObject **field, PyObject *value) { static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { _Py_atomic_store_ptr_relaxed(field, value); } + +// Exclusive-resume token of a generated generator/coroutine object. +// +// A generator has exactly one legitimate driver at a time: resuming one that is +// already executing is an error in every Python implementation. Generated helper +// methods claim the token on entry and drop it at every exit, which buys two +// things on free-threaded builds: +// +// - Mutual exclusion. While the body runs, no other thread can be inside it, +// so the object's private attributes are thread-confined and their loads and +// stores need not be atomic (mypyc emits the same plain accesses as under the +// GIL). One RMW per resume replaces one atomic operation per attribute access. +// - Publication. The release on suspension pairs with the acquire on the next +// resume, so a generator that migrates between threads (the normal case for +// an asyncio task on a thread pool) sees everything the previous thread +// stored, including values written with plain stores. +// +// The token must be a real atomic read-modify-write, not a load followed by a +// store: two threads calling send() concurrently would both pass a non-atomic +// test and then race on non-atomic fields, which is a memory-safety bug rather +// than merely a confused generator. The exchange is sequentially consistent, +// which implies the acquire we need. Its cost is one uncontended local RMW on a +// line the resume is about to write anyway. +static inline int CPyGen_TryEnter(uint32_t *running) { + return _Py_atomic_exchange_uint32(running, 1) == 0; +} + +static inline void CPyGen_Exit(uint32_t *running) { + _Py_atomic_store_uint32_release(running, 0); +} #endif PyObject* update_bases(PyObject *bases); From 3cb3a2dd1cd47d26f3b4638de18ccfb4ccd60001 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 15:26:23 +0100 Subject: [PATCH 02/10] Update comments --- mypyc/ir/class_ir.py | 29 +++++++++++++---------------- mypyc/lib-rt/misc_ops.c | 5 +---- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index 4c694ad460371..5e246cc31dd95 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,14 +250,10 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None - # If True, this is a generated generator/coroutine class whose instances may - # only ever be executed by one thread at a time, and this is enforced at - # runtime by an exclusive-resume token in the object (see - # uses_exclusive_resume() and mypyc/codegen/emitfunc.py). All the attributes - # are then private to whoever holds the token, which lets attribute access - # in the generator body skip the concurrency-safe (atomic) attribute - # operations that free-threaded builds otherwise need. Only ever set on - # free-threaded builds, since there is nothing to gain under the GIL. + # If True, this is a generated generator/coroutine class, and resuming an + # instance that is already executing is rejected via a token in the object + # (see uses_exclusive_resume()). Only set on free-threaded builds, where + # this also makes attributes private to the token holder. self.exclusive_resume = False def __repr__(self) -> str: @@ -318,14 +314,15 @@ def needs_getseters_table(self) -> bool: return self.needs_getseters or not self.is_generated or self.has_dict def uses_exclusive_resume(self) -> bool: - """Is execution of this generator class serialized by an exclusive-resume token? - - If so, the generated helper method claims the token on entry and drops it at - every exit, so while the body runs no other thread can touch the instance's - attributes. Attribute access in the body can then use plain (non-atomic) - loads and stores even on free-threaded builds. That reasoning breaks if - anything outside the helper can read or write the attributes concurrently, - which is why getseters must not be generated for the class. + """Is execution of this generator class guarded by an exclusive-resume token? + + The generated helper method claims the token on entry and drops it at every + exit, so a resume of an already-executing generator fails instead of running + the body again -- whether the resume is reentrant (a generator resuming itself, + as in CPython) or from another thread. Since only the token holder is inside + the body, the instance attributes are private to it and can use plain + (non-atomic) loads and stores even on free-threaded builds. That only holds if + nothing outside the helper can touch the attributes, hence the getseters check. """ return self.exclusive_resume and not self.needs_getseters_table diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index 9675b82037e75..3f01194b8dbc9 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -24,10 +24,7 @@ PyObject *CPy_GetCoro(PyObject *obj) #ifdef Py_GIL_DISABLED // Report a failed attempt to claim a generator's exclusive-resume token (see -// CPyGen_TryEnter): some other thread, or this one reentrantly, is already -// running the body. Always returns NULL, so a helper method can tail-return it. -// Out of line since every generated helper method has one of these, and the -// messages match what CPython's own generators and coroutines raise. +// CPyGen_TryEnter). PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) { PyErr_SetString(PyExc_ValueError, From 25e82172f932befe19973770a73a5bef09b85165 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 15:42:59 +0100 Subject: [PATCH 03/10] Also enable on GIL builds --- mypyc/codegen/emitfunc.py | 12 ++++---- mypyc/ir/class_ir.py | 14 ++++++---- mypyc/irbuild/generator.py | 13 ++++----- mypyc/lib-rt/CPy.h | 2 -- mypyc/lib-rt/misc_ops.c | 5 ++-- mypyc/lib-rt/pythonsupport.h | 32 ++++++++++++++++++--- mypyc/test-data/run-generators.test | 43 +++++++++++++++++++++++++++++ 7 files changed, 94 insertions(+), 27 deletions(-) diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 2c035c4dd81af..9771e3bfa68b5 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -322,9 +322,10 @@ def visit_branch(self, op: Branch) -> None: def emit_exclusive_resume_enter(self, fn: FuncIR) -> None: """Claim the receiver's exclusive-resume token, or fail without running the body. - Rejecting a concurrent (or reentrant) resume is required for correctness, not - just fidelity to CPython: the body's attribute accesses are only safe on - free-threaded builds because at most one thread is inside it at a time. + Rejecting a reentrant (or concurrent) resume matches CPython, which raises + ValueError for an already-executing generator. On free-threaded builds it is + also required for correctness: the body's attribute accesses are only safe + there because at most one thread is inside the body at a time. """ cl = self.exclusive_resume_class assert cl is not None @@ -341,8 +342,9 @@ def emit_exclusive_resume_exit(self) -> None: """Drop the exclusive-resume token before leaving the generator body. Every exit goes through a Return op (error exits included), so this covers - suspension, completion and exceptions alike. The release also publishes - everything the body stored to the next thread that resumes the generator. + suspension, completion and exceptions alike. On free-threaded builds the + release also publishes everything the body stored to the next thread that + resumes the generator. """ assert self.exclusive_resume_token is not None self.emit_line(f"CPyGen_Exit({self.exclusive_resume_token});") diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index 5e246cc31dd95..b34e99bfa5ed6 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -252,8 +252,8 @@ def __init__( # If True, this is a generated generator/coroutine class, and resuming an # instance that is already executing is rejected via a token in the object - # (see uses_exclusive_resume()). Only set on free-threaded builds, where - # this also makes attributes private to the token holder. + # (see uses_exclusive_resume()). On free-threaded builds this additionally + # makes the attributes private to the token holder. self.exclusive_resume = False def __repr__(self) -> str: @@ -319,10 +319,12 @@ def uses_exclusive_resume(self) -> bool: The generated helper method claims the token on entry and drops it at every exit, so a resume of an already-executing generator fails instead of running the body again -- whether the resume is reentrant (a generator resuming itself, - as in CPython) or from another thread. Since only the token holder is inside - the body, the instance attributes are private to it and can use plain - (non-atomic) loads and stores even on free-threaded builds. That only holds if - nothing outside the helper can touch the attributes, hence the getseters check. + as CPython also rejects) or from another thread. On free-threaded builds this + additionally means the instance attributes are private to the token holder, so + the body can access them with plain (non-atomic) loads and stores. That only + holds if nothing outside the helper can touch the attributes, hence the + getseters check. The check is only needed for that reasoning, but it's applied + in GIL builds as well, so that both builds agree on which classes carry a token. """ return self.exclusive_resume and not self.needs_getseters_table diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index 4e666db96d925..c0ca9ddabf688 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -16,7 +16,6 @@ from mypyc.common import ( ENV_ATTR_NAME, GENERATOR_ATTRIBUTE_PREFIX, - IS_FREE_THREADED, NEXT_LABEL_ATTR_NAME, ) from mypyc.ir.class_ir import ClassIR @@ -179,12 +178,12 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: builder.fn_info.env_class = generator_class_ir # The locals live directly in the generator object, and since the environment # wasn't split out, no nested function can capture them: every attribute is - # private to the code running the state machine. On free-threaded builds we - # can therefore serialize execution with a single exclusive-resume token and - # use plain attribute access in the body (see ClassIR.uses_exclusive_resume). - # This is pointless under the GIL, where attribute access is already plain. - if IS_FREE_THREADED: - generator_class_ir.exclusive_resume = True + # private to the code running the state machine. We can therefore guard + # execution with a single exclusive-resume token, which rejects resuming an + # already-executing generator (matching CPython) and, on free-threaded + # builds, also lets the body use plain attribute access (see + # ClassIR.uses_exclusive_resume). + generator_class_ir.exclusive_resume = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) if not builder.fn_info.fitem.is_coroutine: diff --git a/mypyc/lib-rt/CPy.h b/mypyc/lib-rt/CPy.h index a9b2a2e915c58..e3a8be30952a0 100644 --- a/mypyc/lib-rt/CPy.h +++ b/mypyc/lib-rt/CPy.h @@ -990,9 +990,7 @@ static inline PyObject *CPy_TYPE(PyObject *obj) { PyObject *CPy_CalculateMetaclass(PyObject *type, PyObject *o); PyObject *CPy_GetCoro(PyObject *obj); -#ifdef Py_GIL_DISABLED PyObject *CPyGen_AlreadyExecutingError(int is_coroutine); -#endif PyObject *CPyIter_Send(PyObject *iter, PyObject *val); int CPy_YieldFromErrorHandle(PyObject *iter, PyObject **outp); PyObject *CPy_FetchStopIterationValue(void); diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index 3f01194b8dbc9..19d5ed588dc82 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -22,9 +22,9 @@ PyObject *CPy_GetCoro(PyObject *obj) } } -#ifdef Py_GIL_DISABLED // Report a failed attempt to claim a generator's exclusive-resume token (see -// CPyGen_TryEnter). +// CPyGen_TryEnter). These are the messages CPython uses; there is no third case +// for async generators, since mypyc doesn't compile those. PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) { PyErr_SetString(PyExc_ValueError, @@ -32,7 +32,6 @@ PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) : "generator already executing"); return NULL; } -#endif PyObject *CPyIter_Send(PyObject *iter, PyObject *val) { diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index 808a7160730cb..e087e4fd93ae1 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -167,12 +167,18 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { _Py_atomic_store_ptr_relaxed(field, value); } +#endif + // Exclusive-resume token of a generated generator/coroutine object. // // A generator has exactly one legitimate driver at a time: resuming one that is // already executing is an error in every Python implementation. Generated helper -// methods claim the token on entry and drop it at every exit, which buys two -// things on free-threaded builds: +// methods claim the token on entry and drop it at every exit, which rejects both +// a reentrant resume (the generator, directly or indirectly, resuming itself) and +// a concurrent one from another thread. This mirrors what CPython's own +// generators do with gi_frame_state == FRAME_EXECUTING. +// +// On free-threaded builds the token additionally buys: // // - Mutual exclusion. While the body runs, no other thread can be inside it, // so the object's private attributes are thread-confined and their loads and @@ -183,12 +189,18 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { // an asyncio task on a thread pool) sees everything the previous thread // stored, including values written with plain stores. // -// The token must be a real atomic read-modify-write, not a load followed by a -// store: two threads calling send() concurrently would both pass a non-atomic +// There the token must be a real atomic read-modify-write, not a load followed by +// a store: two threads calling send() concurrently would both pass a non-atomic // test and then race on non-atomic fields, which is a memory-safety bug rather // than merely a confused generator. The exchange is sequentially consistent, // which implies the acquire we need. Its cost is one uncontended local RMW on a // line the resume is about to write anyway. +// +// Under the GIL a plain load and store are enough, and no barrier is needed: the +// GIL is held for the whole check (this is straight-line C with no eval-breaker +// point in it), so another thread can't slip in between the test and the set, and +// it also orders everything the previous holder of the token stored. +#ifdef Py_GIL_DISABLED static inline int CPyGen_TryEnter(uint32_t *running) { return _Py_atomic_exchange_uint32(running, 1) == 0; } @@ -196,6 +208,18 @@ static inline int CPyGen_TryEnter(uint32_t *running) { static inline void CPyGen_Exit(uint32_t *running) { _Py_atomic_store_uint32_release(running, 0); } +#else +static inline int CPyGen_TryEnter(uint32_t *running) { + if (*running) { + return 0; + } + *running = 1; + return 1; +} + +static inline void CPyGen_Exit(uint32_t *running) { + *running = 0; +} #endif PyObject* update_bases(PyObject *bases); diff --git a/mypyc/test-data/run-generators.test b/mypyc/test-data/run-generators.test index f4cef93ea8f3e..34bc7ad0fda62 100644 --- a/mypyc/test-data/run-generators.test +++ b/mypyc/test-data/run-generators.test @@ -1002,3 +1002,46 @@ def test_borrow_across_yield_from() -> None: assert False [typing fixtures/typing-full.pyi] + +[case testReentrantResume] +from typing import Any, Iterator, Optional + +box: list[Optional[Iterator[int]]] = [None] + +def self_resuming() -> Iterator[int]: + g = box[0] + assert g is not None + yield next(g) + +def test_reentrant_next() -> None: + g = self_resuming() + box[0] = g + try: + next(g) + except ValueError as e: + assert str(e) == "generator already executing", str(e) + else: + assert False + # The generator was left in a completed state by the propagating exception. + try: + next(g) + except StopIteration: + pass + else: + assert False + +cbox: list[Any] = [None] + +async def self_sending() -> int: + cbox[0].send(None) + return 1 + +def test_reentrant_send_to_coroutine() -> None: + c: Any = self_sending() + cbox[0] = c + try: + c.send(None) + except ValueError as e: + assert str(e) == "coroutine already executing", str(e) + else: + assert False From d610a2e837be02a51714b71bc6367f376c1da239 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 15:47:12 +0100 Subject: [PATCH 04/10] Remove comment --- mypyc/codegen/emitfunc.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 9771e3bfa68b5..9c0494511a4cd 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -136,12 +136,6 @@ def generate_native_function( module_name: str, exclusive_resume_class: ClassIR | None = None, ) -> None: - """Emit the C body of a native function. - - If 'exclusive_resume_class' is set, this is the generator helper method of that - class, and the body is wrapped in the class's exclusive-resume token (see - ClassIR.uses_exclusive_resume). - """ declarations = Emitter(emitter.context) names = generate_names_for_ir(fn.arg_regs, fn.blocks) body = Emitter(emitter.context, names) From 29ad1676f53b8dc1f7f12761cbcdf27fdb378720 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 15:53:26 +0100 Subject: [PATCH 05/10] Refactor --- mypyc/codegen/emitclass.py | 8 ++--- mypyc/codegen/emitfunc.py | 62 ++++++++++++++++++------------------ mypyc/codegen/emitmodule.py | 20 ++++-------- mypyc/common.py | 6 ++-- mypyc/ir/class_ir.py | 32 +++++++++---------- mypyc/irbuild/generator.py | 15 +++------ mypyc/lib-rt/misc_ops.c | 2 +- mypyc/lib-rt/pythonsupport.h | 18 +++++------ 8 files changed, 76 insertions(+), 87 deletions(-) diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index da199d60826b6..7f1186973f0a0 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -29,12 +29,12 @@ BITMAP_BITS, BITMAP_TYPE, CPYFUNCTION_NAME, - EXCLUSIVE_RESUME_FIELD, IS_FREE_THREADED, MYPYC_DEFAULTS_SETUP, NATIVE_PREFIX, PREFIX, REG_PREFIX, + RUNNING_FIELD, short_id_from_name, ) from mypyc.ir.class_ir import ClassIR, VTableEntries @@ -485,11 +485,11 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None: lines += ["typedef struct {", "PyObject_HEAD", "CPyVTableItem *vtable;"] if cl.has_method("__call__"): lines.append("vectorcallfunc vectorcall;") - if cl.uses_exclusive_resume(): - # Exclusive-resume token. Not an IR attribute: it is invisible to the GC, + if cl.uses_running_flag(): + # Running flag. Not an IR attribute: it is invisible to the GC, # to tp_clear and to attribute definedness analysis, and it is never read # outside the generated helper method's entry and exits. - lines.append(f"uint32_t {EXCLUSIVE_RESUME_FIELD};") + lines.append(f"uint32_t {RUNNING_FIELD};") bitmap_attrs = [] for base in reversed(cl.base_mro): if not base.is_trait: diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 9c0494511a4cd..25221c2d60aaa 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -13,12 +13,12 @@ c_array_initializer, ) from mypyc.common import ( - EXCLUSIVE_RESUME_FIELD, GENERATOR_ATTRIBUTE_PREFIX, HAVE_IMMORTAL, IS_FREE_THREADED, NATIVE_PREFIX, REG_PREFIX, + RUNNING_FIELD, ) from mypyc.ir.class_ir import ClassIR from mypyc.ir.func_ir import FUNC_CLASSMETHOD, FUNC_STATICMETHOD, FuncDecl, FuncIR, all_values @@ -134,13 +134,13 @@ def generate_native_function( emitter: Emitter, source_path: str, module_name: str, - exclusive_resume_class: ClassIR | None = None, + running_flag_class: ClassIR | None = None, ) -> None: declarations = Emitter(emitter.context) names = generate_names_for_ir(fn.arg_regs, fn.blocks) body = Emitter(emitter.context, names) visitor = FunctionEmitterVisitor( - body, declarations, source_path, module_name, exclusive_resume_class + body, declarations, source_path, module_name, running_flag_class ) declarations.emit_line(f"{native_function_header(fn.decl, emitter)} {{") @@ -190,10 +190,10 @@ def generate_native_function( if not is_next_block or is_problematic_op: fn.blocks[target.label].referenced = True - if exclusive_resume_class is not None: + if running_flag_class is not None: # Emitted before the first label, so resumes enter here but internal jumps # back to the first block (if any) don't. - visitor.emit_exclusive_resume_enter(fn) + visitor.emit_claim_running_flag(fn) common = frequently_executed_blocks(fn.blocks[0]) @@ -226,7 +226,7 @@ def __init__( declarations: Emitter, source_path: str, module_name: str, - exclusive_resume_class: ClassIR | None = None, + running_flag_class: ClassIR | None = None, ) -> None: self.emitter = emitter self.names = emitter.names @@ -234,10 +234,10 @@ def __init__( self.source_path = source_path self.module_name = module_name # Set if we are emitting the generator helper method of this class, which - # must hold the class's exclusive-resume token while it runs. - self.exclusive_resume_class = exclusive_resume_class - # C expression for the address of that token, set up on function entry - self.exclusive_resume_token: str | None = None + # must hold the instance's running flag while it runs. + self.running_flag_class = running_flag_class + # C expression for the address of that flag, set up on function entry + self.running_flag_ptr: str | None = None self.literals = emitter.context.literals self.rare = False # Next basic block to be processed after the current one (if any), set by caller @@ -313,40 +313,40 @@ def visit_branch(self, op: Branch) -> None: self.emit_lines("} else", " goto %s;" % self.label(false)) - def emit_exclusive_resume_enter(self, fn: FuncIR) -> None: - """Claim the receiver's exclusive-resume token, or fail without running the body. + def emit_claim_running_flag(self, fn: FuncIR) -> None: + """Claim the receiver's running flag, or fail without running the body. - Rejecting a reentrant (or concurrent) resume matches CPython, which raises + Rejecting a reentrant (or concurrent) entry matches CPython, which raises ValueError for an already-executing generator. On free-threaded builds it is also required for correctness: the body's attribute accesses are only safe there because at most one thread is inside the body at a time. """ - cl = self.exclusive_resume_class + cl = self.running_flag_class assert cl is not None struct = cl.struct_name(self.names) self_str = self.reg(fn.arg_regs[0]) - self.exclusive_resume_token = f"&(({struct} *){self_str})->{EXCLUSIVE_RESUME_FIELD}" - token = self.exclusive_resume_token + self.running_flag_ptr = f"&(({struct} *){self_str})->{RUNNING_FIELD}" + flag = self.running_flag_ptr is_coroutine = 1 if cl.has_method("__await__") else 0 - self.emit_line(f"if (unlikely(!CPyGen_TryEnter({token}))) {{") + self.emit_line(f"if (unlikely(!CPyGen_TryEnter({flag}))) {{") self.emit_line(f"return CPyGen_AlreadyExecutingError({is_coroutine});") self.emit_line("}") - def emit_exclusive_resume_exit(self) -> None: - """Drop the exclusive-resume token before leaving the generator body. + def emit_release_running_flag(self) -> None: + """Clear the running flag before leaving the generator body. Every exit goes through a Return op (error exits included), so this covers - suspension, completion and exceptions alike. On free-threaded builds the - release also publishes everything the body stored to the next thread that + suspension, completion and exceptions alike. On free-threaded builds clearing + the flag also publishes everything the body stored to the next thread that resumes the generator. """ - assert self.exclusive_resume_token is not None - self.emit_line(f"CPyGen_Exit({self.exclusive_resume_token});") + assert self.running_flag_ptr is not None + self.emit_line(f"CPyGen_Exit({self.running_flag_ptr});") def visit_return(self, op: Return) -> None: value_str = self.reg(op.value) - if self.exclusive_resume_class is not None: - self.emit_exclusive_resume_exit() + if self.running_flag_class is not None: + self.emit_release_running_flag() self.emit_line("return %s;" % value_str) def visit_tuple_set(self, op: TupleSet) -> None: @@ -473,15 +473,15 @@ def emit_load_attr_take_ref( transform_member_expr in irbuild), whose values live as long as their container. The default (GIL) build always takes the plain-load path and increfs separately. - Attributes of a generator class with exclusive resume are likewise read with a + Attributes of a generator class with a running flag are likewise read with a plain load: no concurrent writer can exist, since only the thread holding the - token runs the generator body (see ClassIR.uses_exclusive_resume). + flag runs the generator body (see ClassIR.uses_running_flag). """ use_get_attr_ref = ( IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype) and not op.is_borrowed - and not cl.uses_exclusive_resume() + and not cl.uses_running_flag() ) if use_get_attr_ref and cl.is_final_attr(op.attr): self.emitter.emit_line(f"{dest} = CPy_GetAttrRefFinal((PyObject **)&{attr_expr});") @@ -642,13 +642,13 @@ def visit_set_attr(self, op: SetAttr) -> None: elif ( IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype) - and not cl.uses_exclusive_resume() + and not cl.uses_running_flag() ): # 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. - # Generator classes with exclusive resume are exempt: no other thread - # can read or write the field while the token holder runs, so a plain + # Generator classes with a running flag are exempt: no other thread + # can read or write the field while the flag holder runs, so a plain # store and an immediate decref of the old value are safe (and let the # old value be freed inline rather than through the QSBR queue). # Both helpers steal the reference to src. diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 6ae6a6cc2f284..ce269fdd0bb62 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -708,27 +708,21 @@ def generate_c_for_modules(self) -> list[tuple[str, str]]: if cl.is_ext_class: generate_class(cl, module_name, emitter) - # Generator classes whose helper method must claim an exclusive-resume - # token. Collected after generating the classes, since that's where the - # token field is added to the object struct. - exclusive_resume_classes = { - cl.name: cl for cl in module.classes if cl.uses_exclusive_resume() - } + # Generator classes whose helper method must claim the running flag. + # Collected after generating the classes, since that's where the flag + # field is added to the object struct. + running_flag_classes = {cl.name: cl for cl in module.classes if cl.uses_running_flag()} # Generate Python extension module definitions and module initialization functions. self.generate_module_def(emitter, module_name, module) for fn in module.functions: emitter.emit_line() - exclusive_resume_class = None + running_flag_class = None if fn.decl.name == GENERATOR_HELPER_NAME and fn.class_name is not None: - exclusive_resume_class = exclusive_resume_classes.get(fn.class_name) + running_flag_class = running_flag_classes.get(fn.class_name) generate_native_function( - fn, - emitter, - self.source_paths[module_name], - module_name, - exclusive_resume_class, + fn, emitter, self.source_paths[module_name], module_name, running_flag_class ) if fn.name != TOP_LEVEL_NAME and not fn.internal: emitter.emit_line() diff --git a/mypyc/common.py b/mypyc/common.py index e11157d04af7b..49ead1e32dbaa 100644 --- a/mypyc/common.py +++ b/mypyc/common.py @@ -28,10 +28,10 @@ GENERATOR_ATTRIBUTE_PREFIX: Final = "__mypyc_generator_attribute__" CPYFUNCTION_NAME = "__cpyfunction__" -# C struct field of a generator object that holds the exclusive-resume token (see -# ClassIR.uses_exclusive_resume). This is not an IR-level attribute, so it can't +# C struct field of a generator object that holds the running flag (see +# ClassIR.uses_running_flag). This is not an IR-level attribute, so it can't # collide with one: those are all prefixed with ATTR_PREFIX. -EXCLUSIVE_RESUME_FIELD: Final = "mypyc_running" +RUNNING_FIELD: Final = "mypyc_running" # Max short int we accept as a literal is based on 32-bit platforms, # so that we can just always emit the same code. diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index b34e99bfa5ed6..ade11d6ea3916 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,11 +250,11 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None - # If True, this is a generated generator/coroutine class, and resuming an - # instance that is already executing is rejected via a token in the object - # (see uses_exclusive_resume()). On free-threaded builds this additionally - # makes the attributes private to the token holder. - self.exclusive_resume = False + # If True, this is a generated generator/coroutine class, and entering an + # instance that is already executing is rejected using a running flag in + # the object (see uses_running_flag()). On free-threaded builds this + # additionally makes the attributes private to the flag holder. + self.has_running_flag = False def __repr__(self) -> str: return ( @@ -313,20 +313,20 @@ def needs_getseters_table(self) -> bool: """Do we generate a tp_getset table exposing the attributes to Python?""" return self.needs_getseters or not self.is_generated or self.has_dict - def uses_exclusive_resume(self) -> bool: - """Is execution of this generator class guarded by an exclusive-resume token? + def uses_running_flag(self) -> bool: + """Is execution of this generator class guarded by a running flag? - The generated helper method claims the token on entry and drops it at every - exit, so a resume of an already-executing generator fails instead of running - the body again -- whether the resume is reentrant (a generator resuming itself, - as CPython also rejects) or from another thread. On free-threaded builds this - additionally means the instance attributes are private to the token holder, so + The generated helper method claims the flag on entry and clears it at every + exit, so entering an already-executing generator fails instead of running the + body again -- whether the entry is reentrant (a generator resuming itself, as + CPython also rejects) or from another thread. On free-threaded builds this + additionally means the instance attributes are private to the flag holder, so the body can access them with plain (non-atomic) loads and stores. That only holds if nothing outside the helper can touch the attributes, hence the getseters check. The check is only needed for that reasoning, but it's applied - in GIL builds as well, so that both builds agree on which classes carry a token. + in GIL builds as well, so that both builds agree on which classes have a flag. """ - return self.exclusive_resume and not self.needs_getseters_table + return self.has_running_flag and not self.needs_getseters_table def method_decl(self, name: str) -> FuncDecl: for ir in self.mro: @@ -496,7 +496,7 @@ def serialize(self) -> JsonDict: "init_self_leak": self.init_self_leak, "env_user_function": self.env_user_function.id if self.env_user_function else None, "reuse_freed_instance": self.reuse_freed_instance, - "exclusive_resume": self.exclusive_resume, + "has_running_flag": self.has_running_flag, "is_acyclic": self.is_acyclic, "is_enum": self.is_enum, "is_coroutine": self.coroutine_name, @@ -560,7 +560,7 @@ def deserialize(cls, data: JsonDict, ctx: DeserMaps) -> ClassIR: ctx.functions[data["env_user_function"]] if data["env_user_function"] else None ) ir.reuse_freed_instance = data["reuse_freed_instance"] - ir.exclusive_resume = data["exclusive_resume"] + ir.has_running_flag = data["has_running_flag"] ir.is_acyclic = data.get("is_acyclic", False) ir.is_enum = data["is_enum"] ir.coroutine_name = data["is_coroutine"] diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index c0ca9ddabf688..14f7b2dfba594 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -13,11 +13,7 @@ from collections.abc import Callable from mypy.nodes import ARG_OPT, FuncDef, Var -from mypyc.common import ( - ENV_ATTR_NAME, - GENERATOR_ATTRIBUTE_PREFIX, - NEXT_LABEL_ATTR_NAME, -) +from mypyc.common import ENV_ATTR_NAME, GENERATOR_ATTRIBUTE_PREFIX, NEXT_LABEL_ATTR_NAME from mypyc.ir.class_ir import ClassIR from mypyc.ir.func_ir import FuncDecl, FuncIR from mypyc.ir.ops import ( @@ -179,11 +175,10 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: # The locals live directly in the generator object, and since the environment # wasn't split out, no nested function can capture them: every attribute is # private to the code running the state machine. We can therefore guard - # execution with a single exclusive-resume token, which rejects resuming an - # already-executing generator (matching CPython) and, on free-threaded - # builds, also lets the body use plain attribute access (see - # ClassIR.uses_exclusive_resume). - generator_class_ir.exclusive_resume = True + # execution with a running flag, which rejects entering an already-executing + # generator (matching CPython) and, on free-threaded builds, also lets the + # body use plain attribute access (see ClassIR.uses_running_flag). + generator_class_ir.has_running_flag = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) if not builder.fn_info.fitem.is_coroutine: diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index 19d5ed588dc82..e6ba02043fc2f 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -22,7 +22,7 @@ PyObject *CPy_GetCoro(PyObject *obj) } } -// Report a failed attempt to claim a generator's exclusive-resume token (see +// Report a failed attempt to claim a generator's running flag (see // CPyGen_TryEnter). These are the messages CPython uses; there is no third case // for async generators, since mypyc doesn't compile those. PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index e087e4fd93ae1..6d240dc1dbe55 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -169,16 +169,16 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { #endif -// Exclusive-resume token of a generated generator/coroutine object. +// Running flag of a generated generator/coroutine object. // // A generator has exactly one legitimate driver at a time: resuming one that is // already executing is an error in every Python implementation. Generated helper -// methods claim the token on entry and drop it at every exit, which rejects both -// a reentrant resume (the generator, directly or indirectly, resuming itself) and +// methods claim the flag on entry and clear it at every exit, which rejects both +// a reentrant entry (the generator, directly or indirectly, resuming itself) and // a concurrent one from another thread. This mirrors what CPython's own // generators do with gi_frame_state == FRAME_EXECUTING. // -// On free-threaded builds the token additionally buys: +// On free-threaded builds the flag additionally buys: // // - Mutual exclusion. While the body runs, no other thread can be inside it, // so the object's private attributes are thread-confined and their loads and @@ -189,17 +189,17 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { // an asyncio task on a thread pool) sees everything the previous thread // stored, including values written with plain stores. // -// There the token must be a real atomic read-modify-write, not a load followed by -// a store: two threads calling send() concurrently would both pass a non-atomic -// test and then race on non-atomic fields, which is a memory-safety bug rather -// than merely a confused generator. The exchange is sequentially consistent, +// There the flag must be claimed with a real atomic read-modify-write, not a load +// followed by a store: two threads calling send() concurrently would both pass a +// non-atomic test and then race on non-atomic fields, which is a memory-safety bug +// rather than merely a confused generator. The exchange is sequentially consistent, // which implies the acquire we need. Its cost is one uncontended local RMW on a // line the resume is about to write anyway. // // Under the GIL a plain load and store are enough, and no barrier is needed: the // GIL is held for the whole check (this is straight-line C with no eval-breaker // point in it), so another thread can't slip in between the test and the set, and -// it also orders everything the previous holder of the token stored. +// it also orders everything the previous holder of the flag stored. #ifdef Py_GIL_DISABLED static inline int CPyGen_TryEnter(uint32_t *running) { return _Py_atomic_exchange_uint32(running, 1) == 0; From 36c6f0054b0157e44f06b48821e361f24184c5a4 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Thu, 3 Sep 2026 17:01:27 +0100 Subject: [PATCH 06/10] Support nested cases --- mypyc/codegen/emitclass.py | 9 +++++- mypyc/codegen/emitfunc.py | 14 ++++----- mypyc/codegen/emitmodule.py | 2 +- mypyc/ir/class_ir.py | 45 ++++++++++++++++----------- mypyc/irbuild/generator.py | 11 ++++--- mypyc/test-data/run-generators.test | 47 +++++++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 31 deletions(-) diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index 7f1186973f0a0..4e818fae53241 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -485,7 +485,14 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None: lines += ["typedef struct {", "PyObject_HEAD", "CPyVTableItem *vtable;"] if cl.has_method("__call__"): lines.append("vectorcallfunc vectorcall;") - if cl.uses_running_flag(): + # A base and its subclasses must agree about the flag, or the structs stop being + # layout compatible and inherited methods read attributes at the wrong offsets. + # Generator classes only ever inherit from generator classes (see + # adjust_generator_classes_of_methods), and all of those have the flag. + assert all( + base.has_running_flag == cl.has_running_flag for base in cl.base_mro + ), f"{cl.name} disagrees with a base class about the running flag" + if cl.has_running_flag: # Running flag. Not an IR attribute: it is invisible to the GC, # to tp_clear and to attribute definedness analysis, and it is never read # outside the generated helper method's entry and exits. diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 25221c2d60aaa..1486db116d1d6 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -473,15 +473,15 @@ def emit_load_attr_take_ref( transform_member_expr in irbuild), whose values live as long as their container. The default (GIL) build always takes the plain-load path and increfs separately. - Attributes of a generator class with a running flag are likewise read with a - plain load: no concurrent writer can exist, since only the thread holding the - flag runs the generator body (see ClassIR.uses_running_flag). + Thread-confined attributes are likewise read with a plain load: no concurrent + writer can exist, since they are only reachable from a generator body that just + one thread at a time can run (see ClassIR.attrs_are_thread_confined). """ use_get_attr_ref = ( IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype) and not op.is_borrowed - and not cl.uses_running_flag() + and not cl.attrs_are_thread_confined() ) if use_get_attr_ref and cl.is_final_attr(op.attr): self.emitter.emit_line(f"{dest} = CPy_GetAttrRefFinal((PyObject **)&{attr_expr});") @@ -642,13 +642,13 @@ def visit_set_attr(self, op: SetAttr) -> None: elif ( IS_FREE_THREADED and is_simple_refcounted_pointer(attr_rtype) - and not cl.uses_running_flag() + and not cl.attrs_are_thread_confined() ): # 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. - # Generator classes with a running flag are exempt: no other thread - # can read or write the field while the flag holder runs, so a plain + # Thread-confined attributes are exempt: no other thread can read or + # write the field while the generator body runs, so a plain # store and an immediate decref of the old value are safe (and let the # old value be freed inline rather than through the QSBR queue). # Both helpers steal the reference to src. diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index ce269fdd0bb62..f6cbcc556b9ff 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -711,7 +711,7 @@ def generate_c_for_modules(self) -> list[tuple[str, str]]: # Generator classes whose helper method must claim the running flag. # Collected after generating the classes, since that's where the flag # field is added to the object struct. - running_flag_classes = {cl.name: cl for cl in module.classes if cl.uses_running_flag()} + running_flag_classes = {cl.name: cl for cl in module.classes if cl.has_running_flag} # Generate Python extension module definitions and module initialization functions. self.generate_module_def(emitter, module_name, module) diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index ade11d6ea3916..7a5f79e562b0c 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,12 +250,21 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None - # If True, this is a generated generator/coroutine class, and entering an - # instance that is already executing is rejected using a running flag in - # the object (see uses_running_flag()). On free-threaded builds this - # additionally makes the attributes private to the flag holder. + # If True, this is a generated generator/coroutine class whose instances have + # a running flag: the generator helper method claims it on entry and clears it + # at every exit, so entering an instance that is already executing fails + # instead of running the body again (matching CPython). Set for all generator + # classes, since reentrant and concurrent entry must be rejected either way. self.has_running_flag = False + # If True, this is a generated generator/coroutine class that holds the + # function's locals directly, instead of pointing to a separate environment + # class. Nothing outside the generator body can then reach those attributes: + # merging only happens when no nested function can capture a local. Combined + # with the running flag this makes them thread-confined -- see + # attrs_are_thread_confined(). + self.has_private_attrs = False + def __repr__(self) -> str: return ( "ClassIR(" @@ -313,20 +322,20 @@ def needs_getseters_table(self) -> bool: """Do we generate a tp_getset table exposing the attributes to Python?""" return self.needs_getseters or not self.is_generated or self.has_dict - def uses_running_flag(self) -> bool: - """Is execution of this generator class guarded by a running flag? - - The generated helper method claims the flag on entry and clears it at every - exit, so entering an already-executing generator fails instead of running the - body again -- whether the entry is reentrant (a generator resuming itself, as - CPython also rejects) or from another thread. On free-threaded builds this - additionally means the instance attributes are private to the flag holder, so - the body can access them with plain (non-atomic) loads and stores. That only - holds if nothing outside the helper can touch the attributes, hence the - getseters check. The check is only needed for that reasoning, but it's applied - in GIL builds as well, so that both builds agree on which classes have a flag. + def attrs_are_thread_confined(self) -> bool: + """Can only one thread at a time reach this class's attributes? + + True for a generator class that holds its locals directly (has_private_attrs) + and serializes execution with a running flag: the attributes are then reachable + only from the generator body, and only the flag holder runs the body. On + free-threaded builds such attributes can use plain (non-atomic) loads and + stores. Generator classes with a separate environment class don't qualify: + their locals live in an object that escaped nested functions can also touch, + even while the generator is suspended, so the running flag says nothing about + them. Getseters would expose the attributes to arbitrary code too, so they + must not be generated. """ - return self.has_running_flag and not self.needs_getseters_table + return self.has_private_attrs and self.has_running_flag and not self.needs_getseters_table def method_decl(self, name: str) -> FuncDecl: for ir in self.mro: @@ -497,6 +506,7 @@ def serialize(self) -> JsonDict: "env_user_function": self.env_user_function.id if self.env_user_function else None, "reuse_freed_instance": self.reuse_freed_instance, "has_running_flag": self.has_running_flag, + "has_private_attrs": self.has_private_attrs, "is_acyclic": self.is_acyclic, "is_enum": self.is_enum, "is_coroutine": self.coroutine_name, @@ -561,6 +571,7 @@ def deserialize(cls, data: JsonDict, ctx: DeserMaps) -> ClassIR: ) ir.reuse_freed_instance = data["reuse_freed_instance"] ir.has_running_flag = data["has_running_flag"] + ir.has_private_attrs = data["has_private_attrs"] ir.is_acyclic = data.get("is_acyclic", False) ir.is_enum = data["is_enum"] ir.coroutine_name = data["is_coroutine"] diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index 14f7b2dfba594..0b3f2e3631d38 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -170,15 +170,16 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: mapper = builder.mapper assert isinstance(builder.fn_info.fitem, FuncDef), builder.fn_info.fitem generator_class_ir = mapper.fdef_to_generator[builder.fn_info.fitem] + # Reject entering a generator that is already executing, like CPython does. + generator_class_ir.has_running_flag = True if builder.fn_info.can_merge_generator_and_env_classes(): builder.fn_info.env_class = generator_class_ir # The locals live directly in the generator object, and since the environment # wasn't split out, no nested function can capture them: every attribute is - # private to the code running the state machine. We can therefore guard - # execution with a running flag, which rejects entering an already-executing - # generator (matching CPython) and, on free-threaded builds, also lets the - # body use plain attribute access (see ClassIR.uses_running_flag). - generator_class_ir.has_running_flag = True + # only reachable from the generator body. Together with the running flag this + # lets free-threaded builds use plain attribute access in the body (see + # ClassIR.attrs_are_thread_confined). + generator_class_ir.has_private_attrs = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) if not builder.fn_info.fitem.is_coroutine: diff --git a/mypyc/test-data/run-generators.test b/mypyc/test-data/run-generators.test index 34bc7ad0fda62..4e19f286bd5ff 100644 --- a/mypyc/test-data/run-generators.test +++ b/mypyc/test-data/run-generators.test @@ -1045,3 +1045,50 @@ def test_reentrant_send_to_coroutine() -> None: assert str(e) == "coroutine already executing", str(e) else: assert False + +[case testReentrantResumeWithSeparateEnvironment] +from typing import Any, Iterator, Optional + +def outer() -> int: + box: list[Any] = [] + + def nested() -> Iterator[int]: + # Captures 'box', so the environment is a separate class and the locals + # are not private to the generator object. The running flag still applies. + yield next(box[0]) + + g = nested() + box.append(g) + return next(g) + +def test_reentrant_next_in_nested_generator() -> None: + try: + outer() + except ValueError as e: + assert str(e) == "generator already executing", str(e) + else: + assert False + +class Base: + def gen(self) -> Iterator[int]: + yield 1 + +class Derived(Base): + def gen(self) -> Iterator[int]: + g = box[0] + assert g is not None + yield next(g) + +box: list[Optional[Iterator[int]]] = [None] + +def test_generator_method_override_still_works() -> None: + assert list(Base().gen()) == [1] + b: Base = Derived() + g = b.gen() + box[0] = g + try: + next(g) + except ValueError as e: + assert str(e) == "generator already executing", str(e) + else: + assert False From db7314d2293bd02c39469ca70762b6a368cd337a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Sep 2026 11:24:12 +0100 Subject: [PATCH 07/10] Minor comment tweaks --- mypyc/codegen/emitclass.py | 14 +++++++------- mypyc/codegen/emitfunc.py | 31 +++++++++++++++--------------- mypyc/codegen/emitmodule.py | 4 ++-- mypyc/common.py | 4 ++-- mypyc/ir/class_ir.py | 37 ++++++++++++++++++------------------ mypyc/irbuild/generator.py | 2 +- mypyc/lib-rt/misc_ops.c | 6 +++--- mypyc/lib-rt/pythonsupport.h | 31 ++++++++++++++---------------- 8 files changed, 63 insertions(+), 66 deletions(-) diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index 4e818fae53241..7e705374362bb 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -485,17 +485,17 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None: lines += ["typedef struct {", "PyObject_HEAD", "CPyVTableItem *vtable;"] if cl.has_method("__call__"): lines.append("vectorcallfunc vectorcall;") - # A base and its subclasses must agree about the flag, or the structs stop being - # layout compatible and inherited methods read attributes at the wrong offsets. - # Generator classes only ever inherit from generator classes (see - # adjust_generator_classes_of_methods), and all of those have the flag. + # A class and its bases must agree about the flag, or the structs stop being layout + # compatible and inherited methods read attributes at the wrong offsets. Generator + # classes only inherit from generator classes (see + # adjust_generator_classes_of_methods), which all have the flag. assert all( base.has_running_flag == cl.has_running_flag for base in cl.base_mro ), f"{cl.name} disagrees with a base class about the running flag" if cl.has_running_flag: - # Running flag. Not an IR attribute: it is invisible to the GC, - # to tp_clear and to attribute definedness analysis, and it is never read - # outside the generated helper method's entry and exits. + # Running flag. Not an IR attribute, so it's invisible to the GC, to tp_clear + # and to definedness analysis, and only ever accessed on entry to and exit + # from the generated helper method. lines.append(f"uint32_t {RUNNING_FIELD};") bitmap_attrs = [] for base in reversed(cl.base_mro): diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 1486db116d1d6..cce2182b210b7 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -191,8 +191,8 @@ def generate_native_function( fn.blocks[target.label].referenced = True if running_flag_class is not None: - # Emitted before the first label, so resumes enter here but internal jumps - # back to the first block (if any) don't. + # Emitted before the first label, so every resume runs it, while internal + # jumps back to the first block don't. visitor.emit_claim_running_flag(fn) common = frequently_executed_blocks(fn.blocks[0]) @@ -233,8 +233,8 @@ def __init__( self.declarations = declarations self.source_path = source_path self.module_name = module_name - # Set if we are emitting the generator helper method of this class, which - # must hold the instance's running flag while it runs. + # Set if we're emitting the generator helper method of this class, which must + # hold the instance's running flag while it runs. self.running_flag_class = running_flag_class # C expression for the address of that flag, set up on function entry self.running_flag_ptr: str | None = None @@ -316,10 +316,10 @@ def visit_branch(self, op: Branch) -> None: def emit_claim_running_flag(self, fn: FuncIR) -> None: """Claim the receiver's running flag, or fail without running the body. - Rejecting a reentrant (or concurrent) entry matches CPython, which raises - ValueError for an already-executing generator. On free-threaded builds it is - also required for correctness: the body's attribute accesses are only safe - there because at most one thread is inside the body at a time. + Rejecting reentrant and concurrent entry matches CPython, which raises + ValueError for an already-executing generator. On free-threaded builds it's + also needed for correctness: the body's attribute accesses are only safe + because at most one thread is inside the body at a time. """ cl = self.running_flag_class assert cl is not None @@ -335,7 +335,7 @@ def emit_claim_running_flag(self, fn: FuncIR) -> None: def emit_release_running_flag(self) -> None: """Clear the running flag before leaving the generator body. - Every exit goes through a Return op (error exits included), so this covers + Every exit goes through a Return op, error exits included, so this covers suspension, completion and exceptions alike. On free-threaded builds clearing the flag also publishes everything the body stored to the next thread that resumes the generator. @@ -473,9 +473,9 @@ def emit_load_attr_take_ref( transform_member_expr in irbuild), whose values live as long as their container. The default (GIL) build always takes the plain-load path and increfs separately. - Thread-confined attributes are likewise read with a plain load: no concurrent - writer can exist, since they are only reachable from a generator body that just - one thread at a time can run (see ClassIR.attrs_are_thread_confined). + Thread-confined attributes also use a plain load: they are only reachable from + a generator body that just one thread at a time can run, so there can be no + concurrent writer (see ClassIR.attrs_are_thread_confined). """ use_get_attr_ref = ( IS_FREE_THREADED @@ -647,10 +647,9 @@ def visit_set_attr(self, op: SetAttr) -> None: # 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. - # Thread-confined attributes are exempt: no other thread can read or - # write the field while the generator body runs, so a plain - # store and an immediate decref of the old value are safe (and let the - # old value be freed inline rather than through the QSBR queue). + # Thread-confined attributes are exempt: no other thread can read or write + # the field while the generator body runs, so a plain store and an + # immediate decref of the old value are safe. # Both helpers steal the reference to src. attr_expr = self.get_attr_expr(obj, op, decl_cl) if op.is_init: diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index f6cbcc556b9ff..5e98617608b5a 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -709,8 +709,8 @@ def generate_c_for_modules(self) -> list[tuple[str, str]]: generate_class(cl, module_name, emitter) # Generator classes whose helper method must claim the running flag. - # Collected after generating the classes, since that's where the flag - # field is added to the object struct. + # Collected after generating the classes, which is where the flag field is + # added to the object struct. running_flag_classes = {cl.name: cl for cl in module.classes if cl.has_running_flag} # Generate Python extension module definitions and module initialization functions. diff --git a/mypyc/common.py b/mypyc/common.py index 49ead1e32dbaa..93665d291c369 100644 --- a/mypyc/common.py +++ b/mypyc/common.py @@ -29,8 +29,8 @@ CPYFUNCTION_NAME = "__cpyfunction__" # C struct field of a generator object that holds the running flag (see -# ClassIR.uses_running_flag). This is not an IR-level attribute, so it can't -# collide with one: those are all prefixed with ATTR_PREFIX. +# ClassIR.has_running_flag). This isn't an IR-level attribute, so it can't collide +# with one: those are all prefixed with ATTR_PREFIX. RUNNING_FIELD: Final = "mypyc_running" # Max short int we accept as a literal is based on 32-bit platforms, diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index 7a5f79e562b0c..f141d26f89892 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,19 +250,19 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None - # If True, this is a generated generator/coroutine class whose instances have - # a running flag: the generator helper method claims it on entry and clears it - # at every exit, so entering an instance that is already executing fails - # instead of running the body again (matching CPython). Set for all generator - # classes, since reentrant and concurrent entry must be rejected either way. + # If True, instances of this generated generator/coroutine class have a running + # flag: the generator helper method claims it on entry and clears it at every + # exit, so entering an instance that is already executing fails instead of + # running the body again, like in CPython. Set for all generator classes, since + # reentrant and concurrent entry must be rejected either way. self.has_running_flag = False - # If True, this is a generated generator/coroutine class that holds the - # function's locals directly, instead of pointing to a separate environment - # class. Nothing outside the generator body can then reach those attributes: - # merging only happens when no nested function can capture a local. Combined - # with the running flag this makes them thread-confined -- see - # attrs_are_thread_confined(). + # If True, this generated generator/coroutine class holds the function's locals + # directly instead of pointing to a separate environment class. Nothing outside + # the generator body can reach those attributes then, since the classes are only + # merged if no nested function can capture a local. Together with the running + # flag this makes the attributes thread-confined (see + # attrs_are_thread_confined). self.has_private_attrs = False def __repr__(self) -> str: @@ -326,14 +326,15 @@ def attrs_are_thread_confined(self) -> bool: """Can only one thread at a time reach this class's attributes? True for a generator class that holds its locals directly (has_private_attrs) - and serializes execution with a running flag: the attributes are then reachable - only from the generator body, and only the flag holder runs the body. On + and serializes execution with a running flag: the attributes are then only + reachable from the generator body, and only the flag holder runs the body. On free-threaded builds such attributes can use plain (non-atomic) loads and - stores. Generator classes with a separate environment class don't qualify: - their locals live in an object that escaped nested functions can also touch, - even while the generator is suspended, so the running flag says nothing about - them. Getseters would expose the attributes to arbitrary code too, so they - must not be generated. + stores. + + Generator classes with a separate environment class don't qualify, since their + locals live in an object that escaped nested functions can also touch, even + while the generator is suspended. Getseters must not be generated either, as + they would expose the attributes to arbitrary code. """ return self.has_private_attrs and self.has_running_flag and not self.needs_getseters_table diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index 0b3f2e3631d38..ce55295b69d8f 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -175,7 +175,7 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: if builder.fn_info.can_merge_generator_and_env_classes(): builder.fn_info.env_class = generator_class_ir # The locals live directly in the generator object, and since the environment - # wasn't split out, no nested function can capture them: every attribute is + # wasn't split out, no nested function can capture them -- every attribute is # only reachable from the generator body. Together with the running flag this # lets free-threaded builds use plain attribute access in the body (see # ClassIR.attrs_are_thread_confined). diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index e6ba02043fc2f..ed0762e98d2c0 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -22,9 +22,9 @@ PyObject *CPy_GetCoro(PyObject *obj) } } -// Report a failed attempt to claim a generator's running flag (see -// CPyGen_TryEnter). These are the messages CPython uses; there is no third case -// for async generators, since mypyc doesn't compile those. +// Report a failed attempt to claim a generator's running flag (see CPyGen_TryEnter). +// These are the messages CPython uses. There is no third case for async generators, +// since mypyc doesn't support those. PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) { PyErr_SetString(PyExc_ValueError, diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index 6d240dc1dbe55..4b1c8ee7dd065 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -173,33 +173,30 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { // // A generator has exactly one legitimate driver at a time: resuming one that is // already executing is an error in every Python implementation. Generated helper -// methods claim the flag on entry and clear it at every exit, which rejects both -// a reentrant entry (the generator, directly or indirectly, resuming itself) and -// a concurrent one from another thread. This mirrors what CPython's own -// generators do with gi_frame_state == FRAME_EXECUTING. +// methods claim the flag on entry and clear it at every exit, rejecting both a +// reentrant entry (the generator resuming itself, directly or indirectly) and a +// concurrent one from another thread. This mirrors what CPython's own generators do +// with gi_frame_state == FRAME_EXECUTING. // // On free-threaded builds the flag additionally buys: // -// - Mutual exclusion. While the body runs, no other thread can be inside it, -// so the object's private attributes are thread-confined and their loads and -// stores need not be atomic (mypyc emits the same plain accesses as under the -// GIL). One RMW per resume replaces one atomic operation per attribute access. +// - Mutual exclusion. No other thread can be inside the body while it runs, so the +// object's private attributes are thread-confined and their loads and stores need +// not be atomic (mypyc emits the same plain accesses as under the GIL). One RMW +// per resume replaces one atomic operation per attribute access. // - Publication. The release on suspension pairs with the acquire on the next -// resume, so a generator that migrates between threads (the normal case for -// an asyncio task on a thread pool) sees everything the previous thread -// stored, including values written with plain stores. +// resume, so a generator that migrates between threads (the normal case for an +// asyncio task on a thread pool) sees everything the previous thread stored, +// including values written with plain stores. // // There the flag must be claimed with a real atomic read-modify-write, not a load // followed by a store: two threads calling send() concurrently would both pass a // non-atomic test and then race on non-atomic fields, which is a memory-safety bug // rather than merely a confused generator. The exchange is sequentially consistent, -// which implies the acquire we need. Its cost is one uncontended local RMW on a -// line the resume is about to write anyway. +// which implies the acquire we need, and costs one uncontended local RMW on a line +// the resume is about to write anyway. // -// Under the GIL a plain load and store are enough, and no barrier is needed: the -// GIL is held for the whole check (this is straight-line C with no eval-breaker -// point in it), so another thread can't slip in between the test and the set, and -// it also orders everything the previous holder of the flag stored. +// Under the GIL a plain load and store suffice. #ifdef Py_GIL_DISABLED static inline int CPyGen_TryEnter(uint32_t *running) { return _Py_atomic_exchange_uint32(running, 1) == 0; From 42f63d572e1cf1d653e619342a3014eab2c7bb76 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Sep 2026 11:33:25 +0100 Subject: [PATCH 08/10] Simplify comments --- mypyc/codegen/emitclass.py | 9 ++------- mypyc/codegen/emitfunc.py | 31 ++++++------------------------- mypyc/codegen/emitmodule.py | 3 --- mypyc/common.py | 4 +--- mypyc/ir/class_ir.py | 31 ++++++++----------------------- mypyc/irbuild/generator.py | 7 +------ mypyc/lib-rt/misc_ops.c | 4 +--- mypyc/lib-rt/pythonsupport.h | 34 +++++++--------------------------- 8 files changed, 26 insertions(+), 97 deletions(-) diff --git a/mypyc/codegen/emitclass.py b/mypyc/codegen/emitclass.py index 7e705374362bb..9a1b2efca8156 100644 --- a/mypyc/codegen/emitclass.py +++ b/mypyc/codegen/emitclass.py @@ -485,17 +485,12 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None: lines += ["typedef struct {", "PyObject_HEAD", "CPyVTableItem *vtable;"] if cl.has_method("__call__"): lines.append("vectorcallfunc vectorcall;") - # A class and its bases must agree about the flag, or the structs stop being layout - # compatible and inherited methods read attributes at the wrong offsets. Generator - # classes only inherit from generator classes (see - # adjust_generator_classes_of_methods), which all have the flag. + # The flag affects attribute offsets, so concrete bases must agree on its presence. assert all( base.has_running_flag == cl.has_running_flag for base in cl.base_mro ), f"{cl.name} disagrees with a base class about the running flag" if cl.has_running_flag: - # Running flag. Not an IR attribute, so it's invisible to the GC, to tp_clear - # and to definedness analysis, and only ever accessed on entry to and exit - # from the generated helper method. + # This implementation field is intentionally absent from the IR attributes. lines.append(f"uint32_t {RUNNING_FIELD};") bitmap_attrs = [] for base in reversed(cl.base_mro): diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index cce2182b210b7..0cef3fd78691d 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -191,8 +191,7 @@ def generate_native_function( fn.blocks[target.label].referenced = True if running_flag_class is not None: - # Emitted before the first label, so every resume runs it, while internal - # jumps back to the first block don't. + # Place this before all labels so it runs on resume, but not on internal jumps. visitor.emit_claim_running_flag(fn) common = frequently_executed_blocks(fn.blocks[0]) @@ -233,10 +232,8 @@ def __init__( self.declarations = declarations self.source_path = source_path self.module_name = module_name - # Set if we're emitting the generator helper method of this class, which must - # hold the instance's running flag while it runs. + # Set while emitting a generator helper protected by its running flag. self.running_flag_class = running_flag_class - # C expression for the address of that flag, set up on function entry self.running_flag_ptr: str | None = None self.literals = emitter.context.literals self.rare = False @@ -314,13 +311,7 @@ def visit_branch(self, op: Branch) -> None: self.emit_lines("} else", " goto %s;" % self.label(false)) def emit_claim_running_flag(self, fn: FuncIR) -> None: - """Claim the receiver's running flag, or fail without running the body. - - Rejecting reentrant and concurrent entry matches CPython, which raises - ValueError for an already-executing generator. On free-threaded builds it's - also needed for correctness: the body's attribute accesses are only safe - because at most one thread is inside the body at a time. - """ + """Claim the generator's running flag or raise ValueError.""" cl = self.running_flag_class assert cl is not None struct = cl.struct_name(self.names) @@ -333,13 +324,7 @@ def emit_claim_running_flag(self, fn: FuncIR) -> None: self.emit_line("}") def emit_release_running_flag(self) -> None: - """Clear the running flag before leaving the generator body. - - Every exit goes through a Return op, error exits included, so this covers - suspension, completion and exceptions alike. On free-threaded builds clearing - the flag also publishes everything the body stored to the next thread that - resumes the generator. - """ + """Release the flag; every helper exit is represented by Return.""" assert self.running_flag_ptr is not None self.emit_line(f"CPyGen_Exit({self.running_flag_ptr});") @@ -473,9 +458,8 @@ def emit_load_attr_take_ref( transform_member_expr in irbuild), whose values live as long as their container. The default (GIL) build always takes the plain-load path and increfs separately. - Thread-confined attributes also use a plain load: they are only reachable from - a generator body that just one thread at a time can run, so there can be no - concurrent writer (see ClassIR.attrs_are_thread_confined). + Thread-confined attributes also use plain loads; see + ClassIR.attrs_are_thread_confined. """ use_get_attr_ref = ( IS_FREE_THREADED @@ -647,9 +631,6 @@ def visit_set_attr(self, op: SetAttr) -> None: # 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. - # Thread-confined attributes are exempt: no other thread can read or write - # the field while the generator body runs, so a plain store and an - # immediate decref of the old value are safe. # Both helpers steal the reference to src. attr_expr = self.get_attr_expr(obj, op, decl_cl) if op.is_init: diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 5e98617608b5a..01ba7e0554c59 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -708,9 +708,6 @@ def generate_c_for_modules(self) -> list[tuple[str, str]]: if cl.is_ext_class: generate_class(cl, module_name, emitter) - # Generator classes whose helper method must claim the running flag. - # Collected after generating the classes, which is where the flag field is - # added to the object struct. running_flag_classes = {cl.name: cl for cl in module.classes if cl.has_running_flag} # Generate Python extension module definitions and module initialization functions. diff --git a/mypyc/common.py b/mypyc/common.py index 93665d291c369..d17f63135de79 100644 --- a/mypyc/common.py +++ b/mypyc/common.py @@ -28,9 +28,7 @@ GENERATOR_ATTRIBUTE_PREFIX: Final = "__mypyc_generator_attribute__" CPYFUNCTION_NAME = "__cpyfunction__" -# C struct field of a generator object that holds the running flag (see -# ClassIR.has_running_flag). This isn't an IR-level attribute, so it can't collide -# with one: those are all prefixed with ATTR_PREFIX. +# Lacks the suffix added to user attribute fields, so it cannot collide with one. RUNNING_FIELD: Final = "mypyc_running" # Max short int we accept as a literal is based on 32-bit platforms, diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index f141d26f89892..88db67e63d5b1 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -250,19 +250,10 @@ def __init__( # Name of the function if this a callable class representing a coroutine. self.coroutine_name: str | None = None - # If True, instances of this generated generator/coroutine class have a running - # flag: the generator helper method claims it on entry and clears it at every - # exit, so entering an instance that is already executing fails instead of - # running the body again, like in CPython. Set for all generator classes, since - # reentrant and concurrent entry must be rejected either way. + # Does this generator or coroutine helper serialize execution using an instance flag? self.has_running_flag = False - # If True, this generated generator/coroutine class holds the function's locals - # directly instead of pointing to a separate environment class. Nothing outside - # the generator body can reach those attributes then, since the classes are only - # merged if no nested function can capture a local. Together with the running - # flag this makes the attributes thread-confined (see - # attrs_are_thread_confined). + # Does this generator object contain its merged environment? self.has_private_attrs = False def __repr__(self) -> str: @@ -323,18 +314,12 @@ def needs_getseters_table(self) -> bool: return self.needs_getseters or not self.is_generated or self.has_dict def attrs_are_thread_confined(self) -> bool: - """Can only one thread at a time reach this class's attributes? - - True for a generator class that holds its locals directly (has_private_attrs) - and serializes execution with a running flag: the attributes are then only - reachable from the generator body, and only the flag holder runs the body. On - free-threaded builds such attributes can use plain (non-atomic) loads and - stores. - - Generator classes with a separate environment class don't qualify, since their - locals live in an object that escaped nested functions can also touch, even - while the generator is suspended. Getseters must not be generated either, as - they would expose the attributes to arbitrary code. + """Can these attributes safely use plain access in free-threaded builds? + + This requires locals to live directly in the generator object, execution to be + serialized by its running flag, and no Python getseters exposing the attributes. + A separate environment does not qualify because captured locals may be accessed + by nested functions. """ return self.has_private_attrs and self.has_running_flag and not self.needs_getseters_table diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index ce55295b69d8f..3395fae029a16 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -170,15 +170,10 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: mapper = builder.mapper assert isinstance(builder.fn_info.fitem, FuncDef), builder.fn_info.fitem generator_class_ir = mapper.fdef_to_generator[builder.fn_info.fitem] - # Reject entering a generator that is already executing, like CPython does. generator_class_ir.has_running_flag = True if builder.fn_info.can_merge_generator_and_env_classes(): builder.fn_info.env_class = generator_class_ir - # The locals live directly in the generator object, and since the environment - # wasn't split out, no nested function can capture them -- every attribute is - # only reachable from the generator body. Together with the running flag this - # lets free-threaded builds use plain attribute access in the body (see - # ClassIR.attrs_are_thread_confined). + # The merged environment can be thread-confined; see attrs_are_thread_confined. generator_class_ir.has_private_attrs = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c index ed0762e98d2c0..621237063ae08 100644 --- a/mypyc/lib-rt/misc_ops.c +++ b/mypyc/lib-rt/misc_ops.c @@ -22,9 +22,7 @@ PyObject *CPy_GetCoro(PyObject *obj) } } -// Report a failed attempt to claim a generator's running flag (see CPyGen_TryEnter). -// These are the messages CPython uses. There is no third case for async generators, -// since mypyc doesn't support those. +// Raise CPython-compatible errors after a failed CPyGen_TryEnter. PyObject *CPyGen_AlreadyExecutingError(int is_coroutine) { PyErr_SetString(PyExc_ValueError, diff --git a/mypyc/lib-rt/pythonsupport.h b/mypyc/lib-rt/pythonsupport.h index 4b1c8ee7dd065..5fc759b7f8b6b 100644 --- a/mypyc/lib-rt/pythonsupport.h +++ b/mypyc/lib-rt/pythonsupport.h @@ -169,34 +169,14 @@ static inline void CPy_InitAttrRef(PyObject **field, PyObject *value) { #endif -// Running flag of a generated generator/coroutine object. +// Generated generator and coroutine helpers claim this flag while executing, +// rejecting reentrant or concurrent resumes. // -// A generator has exactly one legitimate driver at a time: resuming one that is -// already executing is an error in every Python implementation. Generated helper -// methods claim the flag on entry and clear it at every exit, rejecting both a -// reentrant entry (the generator resuming itself, directly or indirectly) and a -// concurrent one from another thread. This mirrors what CPython's own generators do -// with gi_frame_state == FRAME_EXECUTING. -// -// On free-threaded builds the flag additionally buys: -// -// - Mutual exclusion. No other thread can be inside the body while it runs, so the -// object's private attributes are thread-confined and their loads and stores need -// not be atomic (mypyc emits the same plain accesses as under the GIL). One RMW -// per resume replaces one atomic operation per attribute access. -// - Publication. The release on suspension pairs with the acquire on the next -// resume, so a generator that migrates between threads (the normal case for an -// asyncio task on a thread pool) sees everything the previous thread stored, -// including values written with plain stores. -// -// There the flag must be claimed with a real atomic read-modify-write, not a load -// followed by a store: two threads calling send() concurrently would both pass a -// non-atomic test and then race on non-atomic fields, which is a memory-safety bug -// rather than merely a confused generator. The exchange is sequentially consistent, -// which implies the acquire we need, and costs one uncontended local RMW on a line -// the resume is about to write anyway. -// -// Under the GIL a plain load and store suffice. +// On free-threaded builds, the atomic exchange provides mutual exclusion and acquire +// ordering; the release store publishes body writes to the next resume. This also +// permits plain access to private generator attributes. Claiming must be a single +// atomic operation, or two threads could both observe a clear flag and enter. Under +// the GIL, plain accesses suffice. #ifdef Py_GIL_DISABLED static inline int CPyGen_TryEnter(uint32_t *running) { return _Py_atomic_exchange_uint32(running, 1) == 0; From 2a3118f74b74658ed732b591a2220d7f521f357f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Sep 2026 11:45:17 +0100 Subject: [PATCH 09/10] More refactoring --- mypyc/common.py | 2 +- mypyc/ir/class_ir.py | 12 ++++++++---- mypyc/irbuild/generator.py | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/mypyc/common.py b/mypyc/common.py index d17f63135de79..f76a3f189dc4f 100644 --- a/mypyc/common.py +++ b/mypyc/common.py @@ -28,7 +28,7 @@ GENERATOR_ATTRIBUTE_PREFIX: Final = "__mypyc_generator_attribute__" CPYFUNCTION_NAME = "__cpyfunction__" -# Lacks the suffix added to user attribute fields, so it cannot collide with one. +# Omits the prefix added to user attribute fields, so it cannot collide with one. RUNNING_FIELD: Final = "mypyc_running" # Max short int we accept as a literal is based on 32-bit platforms, diff --git a/mypyc/ir/class_ir.py b/mypyc/ir/class_ir.py index 88db67e63d5b1..4782ceeda8bbe 100644 --- a/mypyc/ir/class_ir.py +++ b/mypyc/ir/class_ir.py @@ -254,7 +254,7 @@ def __init__( self.has_running_flag = False # Does this generator object contain its merged environment? - self.has_private_attrs = False + self.has_merged_generator_env = False def __repr__(self) -> str: return ( @@ -321,7 +321,11 @@ def attrs_are_thread_confined(self) -> bool: A separate environment does not qualify because captured locals may be accessed by nested functions. """ - return self.has_private_attrs and self.has_running_flag and not self.needs_getseters_table + return ( + self.has_merged_generator_env + and self.has_running_flag + and not self.needs_getseters_table + ) def method_decl(self, name: str) -> FuncDecl: for ir in self.mro: @@ -492,7 +496,7 @@ def serialize(self) -> JsonDict: "env_user_function": self.env_user_function.id if self.env_user_function else None, "reuse_freed_instance": self.reuse_freed_instance, "has_running_flag": self.has_running_flag, - "has_private_attrs": self.has_private_attrs, + "has_merged_generator_env": self.has_merged_generator_env, "is_acyclic": self.is_acyclic, "is_enum": self.is_enum, "is_coroutine": self.coroutine_name, @@ -557,7 +561,7 @@ def deserialize(cls, data: JsonDict, ctx: DeserMaps) -> ClassIR: ) ir.reuse_freed_instance = data["reuse_freed_instance"] ir.has_running_flag = data["has_running_flag"] - ir.has_private_attrs = data["has_private_attrs"] + ir.has_merged_generator_env = data["has_merged_generator_env"] ir.is_acyclic = data.get("is_acyclic", False) ir.is_enum = data["is_enum"] ir.coroutine_name = data["is_coroutine"] diff --git a/mypyc/irbuild/generator.py b/mypyc/irbuild/generator.py index 3395fae029a16..cd030f82d4212 100644 --- a/mypyc/irbuild/generator.py +++ b/mypyc/irbuild/generator.py @@ -174,7 +174,7 @@ def setup_generator_class(builder: IRBuilder) -> ClassIR: if builder.fn_info.can_merge_generator_and_env_classes(): builder.fn_info.env_class = generator_class_ir # The merged environment can be thread-confined; see attrs_are_thread_confined. - generator_class_ir.has_private_attrs = True + generator_class_ir.has_merged_generator_env = True else: generator_class_ir.attributes[ENV_ATTR_NAME] = RInstance(builder.fn_info.env_class) if not builder.fn_info.fitem.is_coroutine: From 8b19096d9a68083c6740b91a950e1de83c19ff63 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Sep 2026 13:47:36 +0100 Subject: [PATCH 10/10] Address feedback --- mypyc/test-data/run-generators.test | 32 +++++++---------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/mypyc/test-data/run-generators.test b/mypyc/test-data/run-generators.test index 4e19f286bd5ff..56f342edb3548 100644 --- a/mypyc/test-data/run-generators.test +++ b/mypyc/test-data/run-generators.test @@ -1005,6 +1005,7 @@ def test_borrow_across_yield_from() -> None: [case testReentrantResume] from typing import Any, Iterator, Optional +from testutil import assertRaises box: list[Optional[Iterator[int]]] = [None] @@ -1016,19 +1017,11 @@ def self_resuming() -> Iterator[int]: def test_reentrant_next() -> None: g = self_resuming() box[0] = g - try: + with assertRaises(ValueError, "generator already executing"): next(g) - except ValueError as e: - assert str(e) == "generator already executing", str(e) - else: - assert False # The generator was left in a completed state by the propagating exception. - try: + with assertRaises(StopIteration): next(g) - except StopIteration: - pass - else: - assert False cbox: list[Any] = [None] @@ -1039,15 +1032,12 @@ async def self_sending() -> int: def test_reentrant_send_to_coroutine() -> None: c: Any = self_sending() cbox[0] = c - try: + with assertRaises(ValueError, "coroutine already executing"): c.send(None) - except ValueError as e: - assert str(e) == "coroutine already executing", str(e) - else: - assert False [case testReentrantResumeWithSeparateEnvironment] from typing import Any, Iterator, Optional +from testutil import assertRaises def outer() -> int: box: list[Any] = [] @@ -1062,12 +1052,8 @@ def outer() -> int: return next(g) def test_reentrant_next_in_nested_generator() -> None: - try: + with assertRaises(ValueError, "generator already executing"): outer() - except ValueError as e: - assert str(e) == "generator already executing", str(e) - else: - assert False class Base: def gen(self) -> Iterator[int]: @@ -1086,9 +1072,5 @@ def test_generator_method_override_still_works() -> None: b: Base = Derived() g = b.gen() box[0] = g - try: + with assertRaises(ValueError, "generator already executing"): next(g) - except ValueError as e: - assert str(e) == "generator already executing", str(e) - else: - assert False