Fix queuing of callbacks in destruct

revoltphp · Dec 7, 2024 · 38b808c · 38b808c
1 parent 358572c
commit 38b808c
Show file tree

Hide file tree

Showing 2 changed files with 143 additions and 5 deletions.
diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php
@@ -48,6 +48,8 @@ abstract class AbstractDriver implements Driver
 
     private readonly \Closure $interruptCallback;
     private readonly \Closure $queueCallback;
+
+    /** @var \Closure():(null|\Closure(): mixed) */
     private readonly \Closure $runCallback;
 
     private readonly \stdClass $internalSuspensionMarker;
@@ -87,12 +89,24 @@ public function __construct()
         /** @psalm-suppress InvalidArgument */
         $this->interruptCallback = $this->setInterrupt(...);
         $this->queueCallback = $this->queue(...);
-        $this->runCallback = function () {
-            if ($this->fiber->isTerminated()) {
-                $this->createLoopFiber();
-            }
+        $this->runCallback = function (): ?\Closure {
+            do {
+                $garbageCollected = false;
+                if ($this->fiber->isTerminated()) {
+                    $this->createLoopFiber();
+                }
+
+                $result = $this->fiber->isStarted() ? $this->fiber->resume() : $this->fiber->start();
+                if ($result) {
+                    return $result;
+                }
+
+                while (\gc_collect_cycles()) {
+                    $garbageCollected = true;
+                }
+            } while ($garbageCollected);
 
-            return $this->fiber->isStarted() ? $this->fiber->resume() : $this->fiber->start();
+            return null;
         };
     }
 

diff --git a/test/EventLoopTest.php b/test/EventLoopTest.php
@@ -9,6 +9,130 @@
 
 class EventLoopTest extends TestCase
 {
+    public function testSuspensionResumptionWithQueueInGarbageCollection(): void
+    {
+        $suspension = EventLoop::getSuspension();
+
+        $class = new class ($suspension) {
+            public function __construct(public Suspension $suspension)
+            {
+            }
+            public function __destruct()
+            {
+                $this->suspension->resume(true);
+            }
+        };
+        $cycle = [$class, &$cycle];
+        unset($class, $cycle);
+
+        $ended = $suspension->suspend();
+
+        $this->assertTrue($ended);
+    }
+
+    public function testEventLoopResumptionWithQueueInGarbageCollection(): void
+    {
+        $suspension = EventLoop::getSuspension();
+
+        $class = new class ($suspension) {
+            public function __construct(public Suspension $suspension)
+            {
+            }
+            public function __destruct()
+            {
+                EventLoop::queue($this->suspension->resume(...), true);
+            }
+        };
+        $cycle = [$class, &$cycle];
+        unset($class, $cycle);
+
+        $ended = $suspension->suspend();
+
+        $this->assertTrue($ended);
+    }
+
+
+    public function testSuspensionResumptionWithQueueInGarbageCollectionNested(): void
+    {
+        $suspension = EventLoop::getSuspension();
+
+        $resumer = new class ($suspension) {
+            public function __construct(public Suspension $suspension)
+            {
+            }
+            public function __destruct()
+            {
+                $this->suspension->resume(true);
+            }
+        };
+
+        $class = new class ($resumer) {
+            public static ?object $staticReference = null;
+            public function __construct(object $resumer)
+            {
+                self::$staticReference = $resumer;
+            }
+            public function __destruct()
+            {
+                EventLoop::queue(function () {
+                    $class = self::$staticReference;
+                    $cycle = [$class, &$cycle];
+                    unset($class, $cycle);
+
+                    self::$staticReference = null;
+                });
+            }
+        };
+        $cycle = [$class, &$cycle];
+        unset($class, $resumer, $cycle);
+
+
+        $ended = $suspension->suspend();
+
+        $this->assertTrue($ended);
+    }
+
+    public function testEventLoopResumptionWithQueueInGarbageCollectionNested(): void
+    {
+        $suspension = EventLoop::getSuspension();
+
+        $resumer = new class ($suspension) {
+            public function __construct(public Suspension $suspension)
+            {
+            }
+            public function __destruct()
+            {
+                EventLoop::queue($this->suspension->resume(...), true);
+            }
+        };
+
+        $class = new class ($resumer) {
+            public static ?object $staticReference = null;
+            public function __construct(object $resumer)
+            {
+                self::$staticReference = $resumer;
+            }
+            public function __destruct()
+            {
+                EventLoop::queue(function () {
+                    $class = self::$staticReference;
+                    $cycle = [$class, &$cycle];
+                    unset($class, $cycle);
+
+                    self::$staticReference = null;
+                });
+            }
+        };
+        $cycle = [$class, &$cycle];
+        unset($class, $resumer, $cycle);
+
+
+        $ended = $suspension->suspend();
+
+        $this->assertTrue($ended);
+    }
+
+
     public function testDelayWithNegativeDelay(): void
     {
         $this->expectException(\Error::class);