Fix #105 (#121)

Co-authored-by: Bob Weinand <bobwei9@hotmail.com>
Co-authored-by: Aaron Piotrowski <aaron@trowski.com>

Author: 3 people

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();

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();

phpunit.xml.dist

@@ -13,6 +13,7 @@
     <testsuites>
         <testsuite name="Main">
             <directory>test</directory>
+            <directory suffix=".phpt">test</directory>
         </testsuite>
     </testsuites>
 </phpunit>

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();
     }
 
     /**

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();
         }
     }
 

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
      */

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();

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];

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--
+<?php
+
+if (PHP_VERSION_ID < 80400) {
+    echo 'skip PHP 8.4+ required';
+}
+
+if (!\extension_loaded('ev')) {
+    echo 'skip ev extension required';
+}
+
+?>
+--FILE--
+<?php
+
+use Revolt\EventLoop;
+use Revolt\EventLoop\Driver\EvDriver;
+
+require 'vendor/autoload.php';
+
+EventLoop::setDriver(new EvDriver());
+
+final class a {
+    private static self $a;
+    public static function getInstance(): self {
+        return self::$a ??= new self;
+    }
+
+    public function __destruct()
+    {
+        echo "Destroying ", self::class, "\n";
+        $suspension = EventLoop::getSuspension();
+        EventLoop::delay(1.0, $suspension->resume(...));
+        $suspension->suspend();
+        echo "Finished " . self::class, "\n";
+    }
+}
+
+EventLoop::defer(function () {
+    echo "start\n";
+});
+
+a::getInstance();
+
+EventLoop::run();
+
+?>
+--EXPECT--
+start
+Destroying a
+Finished a

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--
+<?php
+
+if (PHP_VERSION_ID < 80400) {
+    echo 'skip PHP 8.4+ required';
+}
+
+if (!\extension_loaded('event')) {
+    echo 'skip event extension required';
+}
+
+?>
+--FILE--
+<?php
+
+use Revolt\EventLoop;
+use Revolt\EventLoop\Driver\EventDriver;
+
+require 'vendor/autoload.php';
+
+EventLoop::setDriver(new EventDriver());
+
+final class a {
+    private static self $a;
+    public static function getInstance(): self {
+        return self::$a ??= new self;
+    }
+
+    public function __destruct()
+    {
+        echo "Destroying ", self::class, "\n";
+        $suspension = EventLoop::getSuspension();
+        EventLoop::delay(1.0, $suspension->resume(...));
+        $suspension->suspend();
+        echo "Finished " . self::class, "\n";
+    }
+}
+
+EventLoop::defer(function () {
+    echo "start\n";
+});
+
+a::getInstance();
+
+EventLoop::run();
+
+?>
+--EXPECT--
+start
+Destroying a
+Finished a