Ignore spurious resumes of old dead {main} suspensions

revoltphp · Nov 16, 2023 · ffb30e2 · ffb30e2
1 parent ccf9d00
commit ffb30e2
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 4 deletions.
diff --git a/src/EventLoop/Internal/DriverSuspension.php b/src/EventLoop/Internal/DriverSuspension.php
@@ -25,6 +25,8 @@ final class DriverSuspension implements Suspension
 
     private bool $pending = false;
 
+    private bool $deadMain = false;
+
     public function __construct(
         private readonly \Closure $run,
         private readonly \Closure $queue,
@@ -38,6 +40,11 @@ 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->error ?? new \Error('Must call suspend() before calling resume()');
         }
@@ -62,6 +69,10 @@ 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 $this->error;
+        }
         if ($this->pending) {
             throw new \Error('Must call resume() or throw() before calling suspend() again');
         }
@@ -101,13 +112,14 @@ 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;
 
             try {
                 $result && $result(); // Unwrap any uncaught exceptions from the event loop
             } catch (\Throwable $throwable) {
                 $this->error = new \Error(
-                    'Suspension cannot be resumed after an uncaught exception is thrown from the event loop',
+                    'Suspension cannot be suspended after an uncaught exception is thrown from the event loop',
                 );
 
                 throw $throwable;
@@ -137,6 +149,11 @@ 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->error ?? new \Error('Must call suspend() before calling throw()');
         }

diff --git a/test/EventLoopTest.php b/test/EventLoopTest.php
@@ -299,11 +299,14 @@ public function testSuspensionThrowingErrorViaInterrupt(): void
             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->resume(); // Calling resume on the same suspension should throw an Error.
+            $suspension->suspend(); // Calling suspend on the same suspension should throw an Error.
             self::fail("Error was not thrown");
         } catch (\Error $e) {
-            self::assertStringContainsString('resumed after an uncaught exception', $e->getMessage());
+            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.
@@ -312,6 +315,35 @@ public function testSuspensionThrowingErrorViaInterrupt(): void
         $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
     {
         $outer = new class (new class ($this) {