From 38b808cc54a5de278cea06f93b7916c70f498ff0 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 6 Dec 2024 19:49:17 -0600 Subject: [PATCH 1/5] Fix queuing of callbacks in destruct --- src/EventLoop/Internal/AbstractDriver.php | 24 ++++- test/EventLoopTest.php | 124 ++++++++++++++++++++++ 2 files changed, 143 insertions(+), 5 deletions(-) diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index fc2e732..38969af 100644 --- 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 index 8f334f2..9aacf59 100644 --- 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); From 84ab0d024528a34d30521ee779ccbf3f821ae550 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 6 Dec 2024 19:50:42 -0600 Subject: [PATCH 2/5] Check if loop was explicitly stopped --- src/EventLoop/Internal/AbstractDriver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 38969af..6fa667a 100644 --- a/src/EventLoop/Internal/AbstractDriver.php +++ b/src/EventLoop/Internal/AbstractDriver.php @@ -104,7 +104,7 @@ public function __construct() while (\gc_collect_cycles()) { $garbageCollected = true; } - } while ($garbageCollected); + } while ($garbageCollected && !$this->stopped); return null; }; From af6757c1bfaadb680d4837fbbd39d9f8ecf4112b Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 6 Dec 2024 19:56:21 -0600 Subject: [PATCH 3/5] Comment --- src/EventLoop/Internal/AbstractDriver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 6fa667a..636648f 100644 --- a/src/EventLoop/Internal/AbstractDriver.php +++ b/src/EventLoop/Internal/AbstractDriver.php @@ -97,7 +97,7 @@ public function __construct() } $result = $this->fiber->isStarted() ? $this->fiber->resume() : $this->fiber->start(); - if ($result) { + if ($result) { // Null indicates the loop fiber terminated without suspending. return $result; } From 607623a6de283ea705ae90c3587a44450b03433e Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 7 Dec 2024 10:20:55 -0600 Subject: [PATCH 4/5] Use $runCallback in run() --- src/EventLoop/Internal/AbstractDriver.php | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 636648f..0f44c0d 100644 --- a/src/EventLoop/Internal/AbstractDriver.php +++ b/src/EventLoop/Internal/AbstractDriver.php @@ -91,7 +91,6 @@ public function __construct() $this->queueCallback = $this->queue(...); $this->runCallback = function (): ?\Closure { do { - $garbageCollected = false; if ($this->fiber->isTerminated()) { $this->createLoopFiber(); } @@ -100,11 +99,7 @@ public function __construct() if ($result) { // Null indicates the loop fiber terminated without suspending. return $result; } - - while (\gc_collect_cycles()) { - $garbageCollected = true; - } - } while ($garbageCollected && !$this->stopped); + } while (!$this->stopped && \gc_collect_cycles()); return null; }; @@ -120,17 +115,14 @@ public function run(): void throw new \Error(\sprintf("Can't call %s() within a fiber (i.e., outside of {main})", __METHOD__)); } - if ($this->fiber->isTerminated()) { - $this->createLoopFiber(); - } - - /** @noinspection PhpUnhandledExceptionInspection */ - $lambda = $this->fiber->isStarted() ? $this->fiber->resume() : $this->fiber->start(); + $lambda = ($this->runCallback)(); if ($lambda) { $lambda(); - throw new \Error('Interrupt from event loop must throw an exception: ' . ClosureHelper::getDescription($lambda)); + throw new \Error( + 'Interrupt from event loop must throw an exception: ' . ClosureHelper::getDescription($lambda) + ); } } From 883521fd5f716622ff66bb84f6fcca15fb535bc9 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 7 Dec 2024 11:32:16 -0600 Subject: [PATCH 5/5] Additional stopped check --- src/EventLoop/Internal/AbstractDriver.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 0f44c0d..fab993f 100644 --- a/src/EventLoop/Internal/AbstractDriver.php +++ b/src/EventLoop/Internal/AbstractDriver.php @@ -91,6 +91,10 @@ public function __construct() $this->queueCallback = $this->queue(...); $this->runCallback = function (): ?\Closure { do { + if ($this->stopped) { + return null; + } + if ($this->fiber->isTerminated()) { $this->createLoopFiber(); }