From 31dacc783c30c615276d838220d9b32199713076 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Thu, 5 Nov 2026 16:36:34 -0800 Subject: [PATCH 1/2] Improve generator for concurrent access --- src/core/IronPython/Runtime/Generator.cs | 131 ++++++++++++++++------- 1 file changed, 95 insertions(+), 36 deletions(-) diff --git a/src/core/IronPython/Runtime/Generator.cs b/src/core/IronPython/Runtime/Generator.cs index f6be74fb9..1ce01e6a1 100644 --- a/src/core/IronPython/Runtime/Generator.cs +++ b/src/core/IronPython/Runtime/Generator.cs @@ -76,7 +76,26 @@ public object __next__() { if (Closed) { return LightExceptions.Throw(PythonOps.StopIteration()); } - + + CheckSetActive(); + try { + // Another thread may have closed the generator after the check above. + if (Closed) { + return LightExceptions.Throw(PythonOps.StopIteration()); + } + + return Next(); + } finally { + ClearActive(); + } + } + + /// + /// Runs the body to the next yield and returns the yielded value, or a StopIteration once the body has finished. + /// The calling thread must hold the generator, see . + /// + [LightThrowing] + private object Next() { object res = NextWorker(); if (res == OperationFailed.Value) { if (FinalValue is null) @@ -122,29 +141,38 @@ private object @throw(object? type, object? value, object? traceback, bool final // Do argument validation outside of the generator's body and do not update the state on failure if (PythonOps.ValidateThrowArguments(type, value) is Exception error) return LightExceptions.Throw(error); - // Set fields which will then be used by CheckThrowable. - // We create the actual exception from inside the generator so that if the exception's __init__ - // throws, the traceback matches that which we get from CPython2.5. - _excInfo = new object?[] { type, value, traceback }; - Debug.Assert(_sendValue == null); - - // Pep explicitly says that Throw on a closed generator throws the exception, + // Pep explicitly says that Throw on a closed generator throws the exception, // and not a StopIteration exception. (This is different than Next()). if (Closed) { - // this will throw the exception that we just set the fields for. - var throwable = CheckThrowable(); - if (throwable != null) { - return throwable; - } + return ThrowOnClosed(); } if (finalizing) { // we are running on the finalizer thread - things can be already collected return LightExceptions.Throw(PythonOps.StopIteration()); } - if (!((IEnumerator)this).MoveNext()) { - return LightExceptions.Throw(PythonOps.StopIteration()); + + CheckSetActive(); + try { + // Another thread may have closed the generator after the check above. + if (!Closed) { + // Set fields which will then be used by CheckThrowable. + // We create the actual exception from inside the generator so that if the exception's __init__ + // throws, the traceback matches that which we get from CPython. + Debug.Assert(_sendValue == null); + _excInfo = new object?[] { type, value, traceback }; + + if (!MoveNextCore()) { + return LightExceptions.Throw(PythonOps.StopIteration()); + } + return CurrentValue; + } + } finally { + ClearActive(); } - return CurrentValue; + return ThrowOnClosed(); + + object ThrowOnClosed() + => LightExceptions.Throw(PythonOps.MakeExceptionForGenerator(Context, type, value, traceback, null)); } /// @@ -153,17 +181,30 @@ private object @throw(object? type, object? value, object? traceback, bool final /// [LightThrowing] public object send(object? value) { - Debug.Assert(_excInfo == null); - - // CPython2.5's behavior is that Send(non-null) on unstarted generator should: - // - throw a TypeError exception - // - not change generator state. So leave as unstarted, and allow future calls to succeed. - if (value != null && State == GeneratorRewriter.NotStarted) { - throw PythonOps.TypeErrorForIllegalSend(); + if (Closed) { + return LightExceptions.Throw(PythonOps.StopIteration()); } - _sendValue = value; - return __next__(); + CheckSetActive(); + try { + // Another thread may have closed the generator after the check above. + if (Closed) { + return LightExceptions.Throw(PythonOps.StopIteration()); + } + + // CPython's behavior is that Send(non-null) on unstarted generator should: + // - throw a TypeError exception + // - not change generator state. So leave as unstarted, and allow future calls to succeed. + if (value != null && State == GeneratorRewriter.NotStarted) { + throw PythonOps.TypeErrorForIllegalSend(); + } + + Debug.Assert(_excInfo == null); + _sendValue = value; + return Next(); + } finally { + ClearActive(); + } } [LightThrowing] @@ -315,9 +356,11 @@ private void HandleFinalizerException(Exception e) { // Sample error message from CPython 2.5 looks like: // Exception __main__.MyError: MyError() in ignored + // CPython 3: + // Exception ignored in: try { - string message = "Exception in generator " + __repr__(Context) + " ignored"; + string message = "Exception ignored in: " + __repr__(Context); PythonOps.PrintWithDest(Context, Context.LanguageContext.SystemStandardError, message); PythonOps.PrintWithDest(Context, Context.LanguageContext.SystemStandardError, Context.LanguageContext.FormatException(e)); @@ -333,7 +376,18 @@ bool IEnumerator.MoveNext() { } CheckSetActive(); + try { + // Another thread may have closed the generator after the check above. + return !Closed && MoveNextCore(); + } finally { + ClearActive(); + } + } + /// + /// Runs the body to the next yield. The calling thread must hold the generator, see . + /// + private bool MoveNextCore() { if (!CanSetSysExcInfo) { return MoveNextWorker(); } else { @@ -410,8 +464,6 @@ private bool MoveNextWorker() { try { ret = GetNext(); } finally { - Active = false; - SaveFunctionStack(!ret); if (!ret) { @@ -428,9 +480,6 @@ private bool MoveNextWorker() { /// Core implementation of Python's next() method. /// private object NextWorker() { - // Generators can not be called re-entrantly. - CheckSetActive(); - RestoreFunctionStack(); _state.PrevException = PythonOps.CurrentExceptionState; @@ -463,8 +512,6 @@ private object NextWorker() { PythonOps.CurrentExceptionState = _state.PrevException; _state.PrevException = null; - Active = false; - SaveFunctionStack(!ret); // If _next() returned false, or did not return (thus leaving ret assigned to its initial value of false), then @@ -477,13 +524,23 @@ private object NextWorker() { return CurrentValue; } + /// + /// Claims the generator for the calling thread, or throws if a thread is already running it (also when the body calls back + /// into its own generator). The caller must release the claim with once it is done with + /// the generator's state, after the generator has been closed if the body finished, so that the next thread to claim it + /// sees it closed. + /// private void CheckSetActive() { - if (Active) { + // Test and set in one step, so that of two threads entering the generator at the same time only one gets in. + if (Interlocked.CompareExchange(ref _active, 1, 0) != 0) { // A generator could catch this exception and continue executing, so this does // not necessarily close the generator. AlreadyExecuting(); } - Active = true; + } + + private void ClearActive() { + Volatile.Write(ref _active, 0); } private static void AlreadyExecuting() { @@ -574,7 +631,9 @@ private bool Closed { /// This can be used to enforce that a generator does not call back into itself. /// Pep255 says that a generator should throw a ValueError if called reentrantly. /// - private bool Active { get; set; } + private bool Active => Volatile.Read(ref _active) != 0; + + private int _active; // 1 while a thread is running the body; set by CheckSetActive, cleared by ClearActive private bool GetNext() { _next(_data); From 16b94b1da4a11dc3305a6399b961b608cdee406c Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Fri, 6 Nov 2026 19:19:26 -0800 Subject: [PATCH 2/2] Add test for accessing a busy generator --- tests/suite/test_generator.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/suite/test_generator.py b/tests/suite/test_generator.py index 21c991960..e8f7f1ce8 100644 --- a/tests/suite/test_generator.py +++ b/tests/suite/test_generator.py @@ -695,4 +695,25 @@ def resume(g): self.assertEqual(resume(g), expected) self.assertEqual(_frame_names(), expected[:-1]) + def test_entry_while_running_on_other_thread(self): + entered, release = threading.Event(), threading.Event() + def g(): + entered.set() + release.wait() + yield 1 + yield 2 + gen = g() + results = [] + t = threading.Thread(target=lambda: results.append(next(gen))) + t.start() + self.assertTrue(entered.wait(10)) + self.assertEqual(gen.gi_running, 1) + for f in (lambda: next(gen), lambda: gen.send(None), lambda: gen.throw(KeyError), gen.close): + self.assertRaises(ValueError, f) + release.set() + t.join() + self.assertEqual(gen.gi_running, 0) + self.assertEqual(results, [1]) + self.assertEqual(list(gen), [2]) + run_test(__name__)