Fix garbage collection race condition

[danog]

Fixes #105

[danog]

There are still some leftover issues with this approach, caused by the reinitialization of the event loop instance:

<?php

use Amp\Process\Process;
use Revolt\EventLoop;
use Revolt\EventLoop\Driver\StreamSelectDriver;

require 'vendor/autoload.php';

final class a {
    private static self $a;
    public static function getInstance(): self {
        return self::$a ??= new self;
    }
    private Process $proc;
    public function __construct()
    {
        $this->proc = Process::start("sleep 1000");
    }

    public function __destruct()
    {
        var_dump("Destroying ".self::class);
        $suspension = EventLoop::getSuspension();
        EventLoop::delay(1.0, $suspension->resume(...));
        $suspension->suspend();
        var_dump("Finished ".self::class);
    }
}

EventLoop::setDriver(new StreamSelectDriver);

EventLoop::defer(function () {
    var_dump("start");
});


a::getInstance();

EventLoop::run();


var_dump("done");

daniil@Mac event-loop % php a.php
string(5) "start"
string(4) "done"
string(12) "Destroying a"
string(10) "Finished a"
PHP Fatal error:  Uncaught Revolt\EventLoop\InvalidCallbackError: Invalid callback identifier f in /Users/daniil/repos/event-loop/src/EventLoop/InvalidCallbackError.php:33
Stack trace:
#0 /Users/daniil/repos/event-loop/src/EventLoop/Internal/AbstractDriver.php(286): Revolt\EventLoop\InvalidCallbackError::invalidIdentifier('f')
#1 /Users/daniil/repos/event-loop/src/EventLoop.php(283): Revolt\EventLoop\Internal\AbstractDriver->reference('f')
#2 /Users/daniil/repos/event-loop/vendor/amphp/process/src/Internal/Posix/PosixHandle.php(66): Revolt\EventLoop::reference('f')
#3 /Users/daniil/repos/event-loop/vendor/amphp/process/src/Internal/Posix/PosixRunner.php(172): Amp\Process\Internal\Posix\PosixHandle->reference()
#4 /Users/daniil/repos/event-loop/vendor/amphp/process/src/Internal/Posix/PosixRunner.php(188): Amp\Process\Internal\Posix\PosixRunner->kill(Object(Amp\Process\Internal\Posix\PosixHandle))
#5 /Users/daniil/repos/event-loop/vendor/amphp/process/src/Internal/ProcHolder.php(22): Amp\Process\Internal\Posix\PosixRunner->destroy(Object(Amp\Process\Internal\Posix\PosixHandle))
#6 [internal function]: Amp\Process\Internal\ProcHolder->__destruct()
#7 {main}
  thrown in /Users/daniil/repos/event-loop/src/EventLoop/InvalidCallbackError.php on line 33

[bwoebi]

The problem isn't the destruction of the event loop, but actually the destruction of the callback fiber.
The callback fiber is destroyed early, because suspensions cause new callback fibers to be created and destruction is in reverse order. What needs to happen is that the callback fiber is recreated in that case.

This PR does not address that problem, but tries to work around it and runs into other problems as shown by your comment.

diff --git a/src/EventLoop/Internal/AbstractDriver.php b/src/EventLoop/Internal/AbstractDriver.php
index 94de316..209c4be 100644
--- a/src/EventLoop/Internal/AbstractDriver.php
+++ b/src/EventLoop/Internal/AbstractDriver.php
@@ -498,9 +498,14 @@ abstract class AbstractDriver implements Driver
     {
         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();

would be a proper fix. Created #107.

[bwoebi]

Also fwiw, when I forgot the final $yielded = assignment in my local code, I had quite a bunch of memory leaks :-D But that's a php-src problem.

[bwoebi]

By the way, #107 also works properly with the "leftover issue" from your comment.

[danog]

Perfect, it dawned upon me later, your solution solves everything!