Conversation
|
The patch misses something. The following script leaks. <?php
class ryat {
var $ryat;
var $chtg;
var $nested;
function __destruct() {
$this->chtg = $this->ryat;
$this->ryat = 1;
}
}
$o = new ryat;
$o->nested = [];
$o->nested[] =& $o->nested;
$o->ryat = $o;
$x =& $o->chtg;
unset($o);
gc_collect_cycles();
var_dump($x);
?> |
|
The new approach already removes nested data during the collect_white phase (using remove_white) instead of later. This avoids issues with insufficient nested data being removed because the removal stops at references. |
|
Unfortunately, I think that this approach is not correct. There is a race-condition here between collecting garbage (normal) and removing roots (for dtors). A part of the graph could already be visited and colored black before we see the object with dtor and try to remove roots. I don't think it's possible to do the nested data removal earlier than we do right now, so I don't see a better solution than what I originally had here, plus #4497. |
Zend/zend_gc.c
Outdated
| gc_root_buffer *root = gc_decompress(ref, GC_REF_ADDRESS(ref)); | ||
| root->ref = GC_MAKE_DTOR_GARBAGE(root->ref); | ||
| GC_REF_SET_PURPLE(ref); | ||
| *flags |= GC_HAS_DESTRUCTORS; |
There was a problem hiding this comment.
gc_remove_white() is called only when GC_HAS_DESTRUCTORS already set.
|
Merged as 60a7e60. |
3 participants
Fix for https://bugs.php.net/bug.php?id=72530.
The problem is that our current approach of comparing refcounts is not sufficient, because an external reference may be added in a way that does not change refcounts.
We can't detect this kind of situation without actually rerunning the full GC marking phase. As such, what I'm doing here is only calling destructors in the current run and leaving the actual freeing of the object and any nested data to the next run.