Skip to content

Conversation

@kooldev
Copy link
Contributor

@kooldev kooldev commented Jun 28, 2021

Implements a GC handler to avoid memory leaks when a fiber callback (or result) holds a reference to the Fiber object. The following example produces a memory leak without proper GC handler:

$fiber = null;
$fiber = new Fiber(function () use (&$fiber) { });

The leak occurs because the Fiber is never started and it cannot be garbage collected due to the callback holding a reference to the Fiber object (this will also happen for transitive / nested references).

$fiber = null;
$fiber = new Fiber(function () use (&$fiber) {
   return $fiber;
});
$fiber->start();

This example will also leak memory due to the Fiber referencing itself from it's result. This PR addresses both leaks by providing a get_gc object handler and 2 additional tests that cover the demonstrated memory leaks.

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of the refactoring you did here. If the sole purpose was to get those zvals next to each other, then please use zend_get_gc_buffer instead.

@kooldev
Copy link
Contributor Author

kooldev commented Jun 28, 2021

@nikic I simplified / fixed the PR as you suggested (I was not aware that zend_get_gc_buffer even existed). 😄

@nikic nikic merged commit 7713302 into php:master Jun 28, 2021
@kooldev kooldev deleted the fiber-gc branch July 13, 2021 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants