Discard {main} suspension on uncaught exception from loop (#71)

Co-authored-by: Daniil Gentili <[email protected]>

Author: trowski

src/EventLoop/Internal/AbstractDriver.php

@@ -395,6 +395,7 @@ final protected function error(\Closure $closure, \Throwable $exception): void
             $this->interrupt = static fn () => $exception instanceof UncaughtThrowable
                 ? throw $exception
                 : throw UncaughtThrowable::throwingCallback($closure, $exception);
+            unset($this->suspensions[$this]); // Remove suspension for {main}
             return;
         }
 
@@ -625,11 +626,10 @@ private function createErrorCallback(): void
             try {
                 $errorHandler($exception);
             } catch (\Throwable $exception) {
-                $this->setInterrupt(
-                    static fn () => $exception instanceof UncaughtThrowable
-                        ? throw $exception
-                        : throw UncaughtThrowable::throwingErrorHandler($errorHandler, $exception)
-                );
+                $this->interrupt = static fn () => $exception instanceof UncaughtThrowable
+                    ? throw $exception
+                    : throw UncaughtThrowable::throwingErrorHandler($errorHandler, $exception);
+                unset($this->suspensions[$this]); // Remove suspension for {main}
             }
         };
     }

src/EventLoop/Internal/DriverSuspension.php

@@ -21,10 +21,12 @@ final class DriverSuspension implements Suspension
     /** @var \WeakReference<\Fiber>|null */
     private readonly ?\WeakReference $fiberRef;
 
-    private ?\FiberError $fiberError = null;
+    private ?\Error $error = null;
 
     private bool $pending = false;
 
+    private bool $deadMain = false;
+
     public function __construct(
         private readonly \Closure $run,
         private readonly \Closure $queue,
@@ -38,8 +40,13 @@ public function __construct(
 
     public function resume(mixed $value = null): void
     {
+        // Ignore spurious resumes to old dead {main} suspension
+        if ($this->deadMain) {
+            return;
+        }
+
         if (!$this->pending) {
-            throw $this->fiberError ?? new \Error('Must call suspend() before calling resume()');
+            throw $this->error ?? new \Error('Must call suspend() before calling resume()');
         }
 
         $this->pending = false;
@@ -62,6 +69,13 @@ public function resume(mixed $value = null): void
 
     public function suspend(): mixed
     {
+        // Throw exception when trying to use old dead {main} suspension
+        if ($this->deadMain) {
+            throw new \Error(
+                'Suspension cannot be suspended after an uncaught exception is thrown from the event loop',
+            );
+        }
+
         if ($this->pending) {
             throw new \Error('Must call resume() or throw() before calling suspend() again');
         }
@@ -73,6 +87,7 @@ public function suspend(): mixed
         }
 
         $this->pending = true;
+        $this->error = null;
 
         // Awaiting from within a fiber.
         if ($fiber) {
@@ -81,12 +96,12 @@ public function suspend(): mixed
             try {
                 $value = \Fiber::suspend();
                 $this->suspendedFiber = null;
-            } catch (\FiberError $exception) {
+            } catch (\FiberError $error) {
                 $this->pending = false;
                 $this->suspendedFiber = null;
-                $this->fiberError = $exception;
+                $this->error = $error;
 
-                throw $exception;
+                throw $error;
             }
 
             // Setting $this->suspendedFiber = null in finally will set the fiber to null if a fiber is destroyed
@@ -100,7 +115,9 @@ public function suspend(): mixed
 
         /** @psalm-suppress RedundantCondition $this->pending should be changed when resumed. */
         if ($this->pending) {
-            $this->pending = false;
+            // This is now a dead {main} suspension.
+            $this->deadMain = true;
+
             $result && $result(); // Unwrap any uncaught exceptions from the event loop
 
             \gc_collect_cycles(); // Collect any circular references before dumping pending suspensions.
@@ -127,8 +144,13 @@ public function suspend(): mixed
 
     public function throw(\Throwable $throwable): void
     {
+        // Ignore spurious resumes to old dead {main} suspension
+        if ($this->deadMain) {
+            return;
+        }
+
         if (!$this->pending) {
-            throw $this->fiberError ?? new \Error('Must call suspend() before calling throw()');
+            throw $this->error ?? new \Error('Must call suspend() before calling throw()');
         }
 
         $this->pending = false;

test/EventLoopTest.php

@@ -298,6 +298,50 @@ public function testSuspensionThrowingErrorViaInterrupt(): void
         } catch (UncaughtThrowable $t) {
             self::assertSame($error, $t->getPrevious());
         }
+
+        $suspension->resume(); // Calling resume on the same suspension should not throw an Error.
+        $suspension->throw(new \RuntimeException()); // Calling throw on the same suspension should not throw an Error.
+
+        try {
+            $suspension->suspend(); // Calling suspend on the same suspension should throw an Error.
+            self::fail("Error was not thrown");
+        } catch (\Error $e) {
+            self::assertStringContainsString('suspended after an uncaught exception', $e->getMessage());
+        }
+
+        // Creating a new Suspension and re-entering the event loop (e.g. in a shutdown function) should work.
+        $suspension = EventLoop::getSuspension();
+        EventLoop::queue($suspension->resume(...));
+        $suspension->suspend();
+    }
+
+    public function testSuspensionThrowingErrorViaInterrupt2(): void
+    {
+        $suspension = EventLoop::getSuspension();
+        $error = new \Error("Test error");
+        EventLoop::queue(static fn () => throw $error);
+        EventLoop::queue($suspension->resume(...), 123);
+        try {
+            $suspension->suspend();
+            self::fail("Error was not thrown");
+        } catch (UncaughtThrowable $t) {
+            self::assertSame($error, $t->getPrevious());
+        }
+
+        $suspension->resume(); // Calling resume on the same suspension should not throw an Error.
+        $suspension->throw(new \RuntimeException()); // Calling throw on the same suspension should not throw an Error.
+
+        try {
+            $suspension->suspend(); // Calling suspend on the same suspension should throw an Error.
+            self::fail("Error was not thrown");
+        } catch (\Error $e) {
+            self::assertStringContainsString('suspended after an uncaught exception', $e->getMessage());
+        }
+
+        // Creating a new Suspension and re-entering the event loop (e.g. in a shutdown function) should work.
+        $suspension = EventLoop::getSuspension();
+        EventLoop::queue($suspension->resume(...), 321);
+        $this->assertEquals(321, $suspension->suspend());
     }
 
     public function testFiberDestroyedWhileSuspended(): void