From d4ab0bcd71f82b6c07099c2366f55195a0dfef85 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Wed, 12 Mar 2025 16:24:33 +0100 Subject: [PATCH 01/10] Fix #105: Ensure the callback fiber is always alive as long as the event loop lives --- phpunit.xml.dist | 1 + src/EventLoop/Internal/AbstractDriver.php | 11 +++++-- test/event_loop_destruction_order.phpt | 38 +++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 test/event_loop_destruction_order.phpt diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 6345e57..0a6176e 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -13,6 +13,7 @@ test + test diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 94de316..998f3b5 100644 --- a/src/EventLoop/Internal/AbstractDriver.php +++ b/src/EventLoop/Internal/AbstractDriver.php @@ -498,9 +498,14 @@ private function invokeCallbacks(): void { while (!$this->microtaskQueue->isEmpty() || !$this->callbackQueue->isEmpty()) { /** @noinspection PhpUnhandledExceptionInspection */ - $yielded = $this->callbackFiber->isStarted() - ? $this->callbackFiber->resume() - : $this->callbackFiber->start(); + if ($this->callbackFiber->isSuspended()) { + $yielded = $this->callbackFiber->resume(); + } else { + if ($this->callbackFiber->isTerminated()) { + $this->createCallbackFiber(); + } + $yielded = $this->callbackFiber->start(); + } if ($yielded !== $this->internalSuspensionMarker) { $this->createCallbackFiber(); diff --git a/test/event_loop_destruction_order.phpt b/test/event_loop_destruction_order.phpt new file mode 100644 index 0000000..0d13794 --- /dev/null +++ b/test/event_loop_destruction_order.phpt @@ -0,0 +1,38 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives +--FILE-- +resume(...)); + $suspension->suspend(); + echo "Finished " . self::class, "\n"; + } +} + +EventLoop::defer(function () { + echo "start\n"; +}); + +a::getInstance(); + +EventLoop::run(); + +?> +--EXPECT-- +start +Destroying a +Finished a From 4b4142ecb734e762808d76ae860a2eaad9cce645 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 May 2026 10:12:38 +0200 Subject: [PATCH 02/10] Fix --- test/Driver/TimerQueueTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index c85666d..c2a69cc 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -47,7 +47,9 @@ public function testHeapOrder(): void $id = 'a'; $callbacks = []; foreach ($values as $value) { - $callback = new TimerCallback($id++, $value, static function () { + $myId = $id; + $id = str_increment($id); + $callback = new TimerCallback($myId, $value, static function () { }, $value); $callbacks[] = $callback; } From 79bdc3b493a8901a5cf626f7fc3bfb56785cc63c Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 May 2026 10:32:37 +0200 Subject: [PATCH 03/10] Fix --- src/EventLoop/Driver/EvDriver.php | 1 + src/EventLoop/Driver/EventDriver.php | 3 ++- src/EventLoop/Driver/UvDriver.php | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/EventLoop/Driver/EvDriver.php b/src/EventLoop/Driver/EvDriver.php index e0a33a3..3de66db 100644 --- a/src/EventLoop/Driver/EvDriver.php +++ b/src/EventLoop/Driver/EvDriver.php @@ -91,6 +91,7 @@ public function __destruct() // We need to clear all references to events manually, see // https://bitbucket.org/osmanov/pecl-ev/issues/31/segfault-in-ev_timer_stop $this->events = []; + $this->handle = new \EvLoop(); } /** diff --git a/src/EventLoop/Driver/EventDriver.php b/src/EventLoop/Driver/EventDriver.php index 869c3a9..416de6d 100644 --- a/src/EventLoop/Driver/EventDriver.php +++ b/src/EventLoop/Driver/EventDriver.php @@ -91,7 +91,8 @@ public function __destruct() /** @psalm-suppress RedundantPropertyInitializationCheck */ if (isset($this->handle)) { $this->handle->free(); - unset($this->handle); + /** @psalm-suppress TooFewArguments https://github.com/JetBrains/phpstorm-stubs/pull/763 */ + $this->handle = new \EventBase(); } } diff --git a/src/EventLoop/Driver/UvDriver.php b/src/EventLoop/Driver/UvDriver.php index f46d909..5a08e0c 100644 --- a/src/EventLoop/Driver/UvDriver.php +++ b/src/EventLoop/Driver/UvDriver.php @@ -110,6 +110,14 @@ public function cancel(string $callbackId): void unset($this->events[$callbackId]); } + public function __destruct() + { + $this->events = []; + $this->uvCallbacks = []; + $this->streams = []; + $this->handle = \uv_loop_new(); + } + /** * @return \UVLoop|resource */ From 62b6483fe79f63056cad7f28eb1324f0320f646f Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 May 2026 10:33:21 +0200 Subject: [PATCH 04/10] Add more tests --- test/event_loop_destruction_order_ev.phpt | 49 +++++++++++++++++++ test/event_loop_destruction_order_event.phpt | 49 +++++++++++++++++++ ..._loop_destruction_order_stream_select.phpt | 41 ++++++++++++++++ test/event_loop_destruction_order_uv.phpt | 49 +++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 test/event_loop_destruction_order_ev.phpt create mode 100644 test/event_loop_destruction_order_event.phpt create mode 100644 test/event_loop_destruction_order_stream_select.phpt create mode 100644 test/event_loop_destruction_order_uv.phpt diff --git a/test/event_loop_destruction_order_ev.phpt b/test/event_loop_destruction_order_ev.phpt new file mode 100644 index 0000000..3a1f0bc --- /dev/null +++ b/test/event_loop_destruction_order_ev.phpt @@ -0,0 +1,49 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives (ev driver) +--SKIPIF-- + +--FILE-- +resume(...)); + $suspension->suspend(); + echo "Finished " . self::class, "\n"; + } +} + +EventLoop::defer(function () { + echo "start\n"; +}); + +a::getInstance(); + +EventLoop::run(); + +?> +--EXPECT-- +start +Destroying a +Finished a diff --git a/test/event_loop_destruction_order_event.phpt b/test/event_loop_destruction_order_event.phpt new file mode 100644 index 0000000..c193da0 --- /dev/null +++ b/test/event_loop_destruction_order_event.phpt @@ -0,0 +1,49 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives (event driver) +--SKIPIF-- + +--FILE-- +resume(...)); + $suspension->suspend(); + echo "Finished " . self::class, "\n"; + } +} + +EventLoop::defer(function () { + echo "start\n"; +}); + +a::getInstance(); + +EventLoop::run(); + +?> +--EXPECT-- +start +Destroying a +Finished a diff --git a/test/event_loop_destruction_order_stream_select.phpt b/test/event_loop_destruction_order_stream_select.phpt new file mode 100644 index 0000000..fa128d0 --- /dev/null +++ b/test/event_loop_destruction_order_stream_select.phpt @@ -0,0 +1,41 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives (stream_select driver) +--FILE-- +resume(...)); + $suspension->suspend(); + echo "Finished " . self::class, "\n"; + } +} + +EventLoop::defer(function () { + echo "start\n"; +}); + +a::getInstance(); + +EventLoop::run(); + +?> +--EXPECT-- +start +Destroying a +Finished a diff --git a/test/event_loop_destruction_order_uv.phpt b/test/event_loop_destruction_order_uv.phpt new file mode 100644 index 0000000..f154486 --- /dev/null +++ b/test/event_loop_destruction_order_uv.phpt @@ -0,0 +1,49 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives (uv driver) +--SKIPIF-- + +--FILE-- +resume(...)); + $suspension->suspend(); + echo "Finished " . self::class, "\n"; + } +} + +EventLoop::defer(function () { + echo "start\n"; +}); + +a::getInstance(); + +EventLoop::run(); + +?> +--EXPECT-- +start +Destroying a +Finished a From 0252709cd412f8f25d2f933caa903f995c776fe3 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 May 2026 10:33:43 +0200 Subject: [PATCH 05/10] Cleanup --- test/event_loop_destruction_order.phpt | 38 -------------------------- 1 file changed, 38 deletions(-) delete mode 100644 test/event_loop_destruction_order.phpt diff --git a/test/event_loop_destruction_order.phpt b/test/event_loop_destruction_order.phpt deleted file mode 100644 index 0d13794..0000000 --- a/test/event_loop_destruction_order.phpt +++ /dev/null @@ -1,38 +0,0 @@ ---TEST-- -Issue #105: Ensure the callback fiber is always alive as long as the event loop lives ---FILE-- -resume(...)); - $suspension->suspend(); - echo "Finished " . self::class, "\n"; - } -} - -EventLoop::defer(function () { - echo "start\n"; -}); - -a::getInstance(); - -EventLoop::run(); - -?> ---EXPECT-- -start -Destroying a -Finished a From c3119b300e0ceea4a3e568a277abd141f3266c0f Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Fri, 15 May 2026 10:43:17 +0200 Subject: [PATCH 06/10] Fixup --- examples/benchmark-ticks.php | 3 ++- examples/benchmark-timers.php | 3 ++- test/Driver/TimerQueueTest.php | 2 +- test/event_loop_destruction_order_ev.phpt | 4 ++++ test/event_loop_destruction_order_event.phpt | 4 ++++ test/event_loop_destruction_order_stream_select.phpt | 8 ++++++++ test/event_loop_destruction_order_uv.phpt | 4 ++++ 7 files changed, 25 insertions(+), 3 deletions(-) diff --git a/examples/benchmark-ticks.php b/examples/benchmark-ticks.php index 4ac1642..15b3ff3 100644 --- a/examples/benchmark-ticks.php +++ b/examples/benchmark-ticks.php @@ -9,7 +9,8 @@ $n = isset($argv[1]) ? (int) $argv[1] : 1000 * 100; for ($i = 0; $i < $n; ++$i) { - EventLoop::defer(function () { }); + EventLoop::defer(function () { + }); } EventLoop::run(); diff --git a/examples/benchmark-timers.php b/examples/benchmark-timers.php index 112d2f1..27c0dd4 100644 --- a/examples/benchmark-timers.php +++ b/examples/benchmark-timers.php @@ -9,7 +9,8 @@ $n = isset($argv[1]) ? (int) $argv[1] : 1000 * 100; for ($i = 0; $i < $n; ++$i) { - EventLoop::delay(0, function () { }); + EventLoop::delay(0, function () { + }); } EventLoop::run(); diff --git a/test/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index c2a69cc..c64bd37 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -48,7 +48,7 @@ public function testHeapOrder(): void $callbacks = []; foreach ($values as $value) { $myId = $id; - $id = str_increment($id); + $id = \str_increment($id); $callback = new TimerCallback($myId, $value, static function () { }, $value); $callbacks[] = $callback; diff --git a/test/event_loop_destruction_order_ev.phpt b/test/event_loop_destruction_order_ev.phpt index 3a1f0bc..0ba34d3 100644 --- a/test/event_loop_destruction_order_ev.phpt +++ b/test/event_loop_destruction_order_ev.phpt @@ -3,6 +3,10 @@ Issue #105: Ensure the callback fiber is always alive as long as the event loop --SKIPIF-- --FILE-- Date: Fri, 15 May 2026 10:46:25 +0200 Subject: [PATCH 07/10] Fix --- test/Driver/TimerQueueTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index c64bd37..507f2a1 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -48,7 +48,14 @@ public function testHeapOrder(): void $callbacks = []; foreach ($values as $value) { $myId = $id; - $id = \str_increment($id); + + if (\PHP_VERSION_ID >= 80300) { + /** @psalm-suppress UndefinedFunction */ + $id = \str_increment($id); + } else { + $id++; + } + $callback = new TimerCallback($myId, $value, static function () { }, $value); $callbacks[] = $callback; From a233f05691de64a61333d4519929945972d9463b Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 16 May 2026 12:12:20 -0500 Subject: [PATCH 08/10] Add comments to clarify handle re-initialization --- src/EventLoop/Driver/EvDriver.php | 3 +++ src/EventLoop/Driver/EventDriver.php | 3 +++ src/EventLoop/Driver/UvDriver.php | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/EventLoop/Driver/EvDriver.php b/src/EventLoop/Driver/EvDriver.php index 3de66db..8a1a008 100644 --- a/src/EventLoop/Driver/EvDriver.php +++ b/src/EventLoop/Driver/EvDriver.php @@ -91,6 +91,9 @@ public function __destruct() // We need to clear all references to events manually, see // https://bitbucket.org/osmanov/pecl-ev/issues/31/segfault-in-ev_timer_stop $this->events = []; + + // Reinitialize the loop handle due to indeterminate destruct order. + // See https://github.com/revoltphp/event-loop/issues/105 $this->handle = new \EvLoop(); } diff --git a/src/EventLoop/Driver/EventDriver.php b/src/EventLoop/Driver/EventDriver.php index 416de6d..aa1ff5e 100644 --- a/src/EventLoop/Driver/EventDriver.php +++ b/src/EventLoop/Driver/EventDriver.php @@ -91,6 +91,9 @@ public function __destruct() /** @psalm-suppress RedundantPropertyInitializationCheck */ if (isset($this->handle)) { $this->handle->free(); + + // Reinitialize the loop handle due to indeterminate destruct order. + // See https://github.com/revoltphp/event-loop/issues/105 /** @psalm-suppress TooFewArguments https://github.com/JetBrains/phpstorm-stubs/pull/763 */ $this->handle = new \EventBase(); } diff --git a/src/EventLoop/Driver/UvDriver.php b/src/EventLoop/Driver/UvDriver.php index 5a08e0c..c7dbbbe 100644 --- a/src/EventLoop/Driver/UvDriver.php +++ b/src/EventLoop/Driver/UvDriver.php @@ -115,6 +115,9 @@ public function __destruct() $this->events = []; $this->uvCallbacks = []; $this->streams = []; + + // Reinitialize the loop handle due to indeterminate destruct order. + // See https://github.com/revoltphp/event-loop/issues/105 $this->handle = \uv_loop_new(); } From df203127666a41ff54cf960d386be6b4bf01db06 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 16 May 2026 12:14:30 -0500 Subject: [PATCH 09/10] No need for temp --- test/Driver/TimerQueueTest.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index 507f2a1..a6ccb1f 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -47,7 +47,8 @@ public function testHeapOrder(): void $id = 'a'; $callbacks = []; foreach ($values as $value) { - $myId = $id; + $callback = new TimerCallback($id, $value, static function () {}, $value); + $callbacks[] = $callback; if (\PHP_VERSION_ID >= 80300) { /** @psalm-suppress UndefinedFunction */ @@ -55,10 +56,6 @@ public function testHeapOrder(): void } else { $id++; } - - $callback = new TimerCallback($myId, $value, static function () { - }, $value); - $callbacks[] = $callback; } $toRemove = $callbacks[$indexToRemove]; From e93e9e6d71b28513e30797d3c2fbbfcec76d22da Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 16 May 2026 12:18:51 -0500 Subject: [PATCH 10/10] Ugh, fine style fixer, I'll comply --- test/Driver/TimerQueueTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index a6ccb1f..6c0fc80 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -47,7 +47,8 @@ public function testHeapOrder(): void $id = 'a'; $callbacks = []; foreach ($values as $value) { - $callback = new TimerCallback($id, $value, static function () {}, $value); + $callback = new TimerCallback($id, $value, static function () { + }, $value); $callbacks[] = $callback; if (\PHP_VERSION_ID >= 80300) {