Bug #63734 [Asn->Csd]: Garbage collector can free zvals that are still referenced

Edit report at https://bugs.php.net/bug.php?id=63734&edit=1 ID: 63734 Updated by: dmitry@php.net Reported by: lbarnaud@php.net Summary: Garbage collector can free zvals that are still referenced -Status: Assigned +Status: Closed Type: Bug Package: Reproducible crash PHP Version: 5.4Git-2012-12-09 (Git) Assigned To: dmitry Block user comment: N Private report: Y New Comment: Fixed in PHP-7. Previous Comments: ------------------------------------------------------------------------ [2013-06-24 08:58:07] dmitry@php.net hi Arnaud, I afraid, I didn't understand your idea. could you provide a patch? ------------------------------------------------------------------------ [2013-06-22 09:58:41] lbarnaud@php.net Proposed mitigation (may not be right) : After the GC has called destructors [1] and destroyed zvals [2], all zvals to be freed are expected to have a refcount of 1 at [3] (to be verified; since all zvals referencing the zval have been destroyed too, the only left reference should be the one added by the GC itself). If a zval has a refcount > 1, it means that a reference to it has been created during step [1]. Converting the zval to IS_NULL in [3] if refcount > 1, instead of freeing it, avoids use-after-frees. [1] https://github.com/php/php-src/blob/a666285bc2488b7f7362368c388e41428610ad1d/Zend/zend_gc.c#L805 [2] https://github.com/php/php-src/blob/a666285bc2488b7f7362368c388e41428610ad1d/Zend/zend_gc.c#L824 [3] https://github.com/php/php-src/blob/a666285bc2488b7f7362368c388e41428610ad1d/Zend/zend_gc.c#L851 Improved reproducing script (causes segfault, and simplified): <?php global $ary; class C { public $ref; public function __construct() { $this->ref = $this; } public function __destruct() { global $ary; $ary[] = $this; } } new C; gc_collect_cycles(); var_dump($ary); ?> ------------------------------------------------------------------------ [2012-12-12 06:48:08] dmitry@php.net Actually, I think it may be possible to detect mutation caused by destructors by complication of collector, but not the zval and inc/dec refcount behavior. After all the destructors called, me can put all the array and objects detected as garbage on the first step back into "possible roots of cycles" buffer and run the same algorithm once again. It's going to make GC about 2 times slower, and I'm not sure that this solution will work at all. Anyway, I won't have time in nearest future to test the idea. May be we can borrow something from Java's GC & finalizers interactions, but papers, I read, refer to 3 times GC algorithm complication because of finalizers. ------------------------------------------------------------------------ [2012-12-12 04:18:25] laruence@php.net I agree. gc doesn't record the refcount for zvals before calling destructor, so we have no idea what have been changed during destructor calling. but if we record that, a significant performance reduction will come ------------------------------------------------------------------------ [2012-12-10 13:32:54] dmitry@php.net Unfortunately, it's not possible to fix the problem without significant complication of GC algorithm that is going to increase memory consumption and to slow down PHP execution in general (even without GC). The original synchronous GC algorithm described at http://www.research.ibm.com/people/d/dfb/papers/Bacon01Concurrent.pdf doesn't care about destructors. I think, that the concurrent algorithm described in the same paper may fix the problem but with siginificant overhead. I wouldn't fix the problem in case some cheaper workaround is found. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=63734 -- Edit this bug report at https://bugs.php.net/bug.php?id=63734&edit=1