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/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/Driver/EvDriver.php b/src/EventLoop/Driver/EvDriver.php index e0a33a3..8a1a008 100644 --- a/src/EventLoop/Driver/EvDriver.php +++ b/src/EventLoop/Driver/EvDriver.php @@ -91,6 +91,10 @@ 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 869c3a9..aa1ff5e 100644 --- a/src/EventLoop/Driver/EventDriver.php +++ b/src/EventLoop/Driver/EventDriver.php @@ -91,7 +91,11 @@ public function __destruct() /** @psalm-suppress RedundantPropertyInitializationCheck */ if (isset($this->handle)) { $this->handle->free(); - unset($this->handle); + + // 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 f46d909..c7dbbbe 100644 --- a/src/EventLoop/Driver/UvDriver.php +++ b/src/EventLoop/Driver/UvDriver.php @@ -110,6 +110,17 @@ public function cancel(string $callbackId): void unset($this->events[$callbackId]); } + 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(); + } + /** * @return \UVLoop|resource */ diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php index 986071e..83d0840 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/Driver/TimerQueueTest.php b/test/Driver/TimerQueueTest.php index c85666d..6c0fc80 100644 --- a/test/Driver/TimerQueueTest.php +++ b/test/Driver/TimerQueueTest.php @@ -47,9 +47,16 @@ public function testHeapOrder(): void $id = 'a'; $callbacks = []; foreach ($values as $value) { - $callback = new TimerCallback($id++, $value, static function () { + $callback = new TimerCallback($id, $value, static function () { }, $value); $callbacks[] = $callback; + + if (\PHP_VERSION_ID >= 80300) { + /** @psalm-suppress UndefinedFunction */ + $id = \str_increment($id); + } else { + $id++; + } } $toRemove = $callbacks[$indexToRemove]; diff --git a/test/event_loop_destruction_order_ev.phpt b/test/event_loop_destruction_order_ev.phpt new file mode 100644 index 0000000..0ba34d3 --- /dev/null +++ b/test/event_loop_destruction_order_ev.phpt @@ -0,0 +1,53 @@ +--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..3f27168 --- /dev/null +++ b/test/event_loop_destruction_order_event.phpt @@ -0,0 +1,53 @@ +--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..a9c19ee --- /dev/null +++ b/test/event_loop_destruction_order_stream_select.phpt @@ -0,0 +1,49 @@ +--TEST-- +Issue #105: Ensure the callback fiber is always alive as long as the event loop lives (stream_select 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_uv.phpt b/test/event_loop_destruction_order_uv.phpt new file mode 100644 index 0000000..42987c4 --- /dev/null +++ b/test/event_loop_destruction_order_uv.phpt @@ -0,0 +1,53 @@ +--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