Garbage collection issue

Issue #50 new
kelunik
created an issue

There's some garbage collection issue within ext-event, see https://github.com/amphp/amp/issues/177.

It's possible to get around this by avoiding circular references, but that shouldn't be necessary, see https://github.com/amphp/amp/pull/219/files.

See https://gist.github.com/brstgt/0a83963136436ef5e266c1d807e513c4 for a reproduction case, the fetched URL doesn't matter and can be changed to example.com.

Comments (4)

  1. Ruslan Osmanov repo owner

    Apparently, you are suggesting to install amphp/amp and debug it using the code snippet called FD leak test. From the code snippet, it is not obvious that the "leak" is caused by Event extension. Another unobvious thing is the EventDriver snippet that, I guess, should be configured somehow for FD leak test.

    I'll dig into it when I have more free time, but I doubt it will be very soon. If you would like to speed up the process, please provide a short code snippet, preferably minimal, preferably without the need to dig into amphp/amp.

    However, from the code snippet I can see that by "some garbage collection issue" you likely meant "EventHttpConnection objects not being destroyed automatically". If my guess is right, then the issue is likely caused by commit 4edb5374939da853e1e60fc66ac21690b579542d. Until it is clear to me what's actually going on in your code, take my advice: destroy Event* objects (assign to null) as soon as they are no longer needed (don't delay this process).

  2. kelunik reporter

    You're absolutely right that I should have written a minimal test case to start with, but here it is:

    <?php
    
    require __DIR__ . '/vendor/autoload.php';
    
    $loop = new class {
        private $handle;
        private $ioCallback;
        private $events;
    
        public function __construct() {
            $this->handle = new EventBase;
    
            $this->ioCallback = function () {
                var_dump(__FILE__ . ':' . __LINE__);
            };
        }
    
        public function watch($stream) {
            $this->events[(int) $stream] = $event = new Event(
                $this->handle,
                $stream,
                Event::READ | Event::PERSIST,
                $this->ioCallback,
                null
            );
    
            $event->add();
        }
    
        public function run() {
            $this->handle->loop(true);
        }
    
        public function free() {
            foreach ($this->events as $event) {
                $event->free();
            }
    
            $this->events = [];
        }
    
        public function __destruct() {
            var_dump(__METHOD__);
        }
    };
    
    $loop->watch(STDOUT);
    
    if (($argv[1] ?? "") === "free") {
        print "Freeing events..." . PHP_EOL;
        $loop->free();
    }
    
    unset($loop);
    
    var_dump(\gc_collect_cycles());
    
    print " -- Object should no longer exist here -- " . PHP_EOL;
    

    We're not using EventHttpConnection objects. We can't destroy the event objects earlier, because they're still existent watchers the event loop has a reference to. But if the event loop gets swapped and is no longer referenced, these are never garbage collected. A manual shutdown helps, but we want to avoid that, as that's a task of the garbage collection system.

    Run the script with php script.php no-free and php script.php free.

  3. Log in to comment