Bug #73960 [Ver]: Leak with instance method calling static method with referenced return

From: Date: Sun, 22 Jan 2017 11:07:07 +0000
Subject: Bug #73960 [Ver]: Leak with instance method calling static method with referenced return
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-206850@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=73960&edit=1 ID: 73960 Updated by: krakjoe@php.net Reported by: highmind63 at gmail dot com Summary: Leak with instance method calling static method with referenced return Status: Verified Type: Bug Package: Class/Object related Operating System: Windows 10 and Ubuntu 14.04 PHP Version: 7.0.15 Block user comment: N Private report: N New Comment: Nikita can you turn that into a PR with test case please, so we can get CI and some attention. Previous Comments: ------------------------------------------------------------------------ [2017-01-21 14:18:16] nikic@php.net Even simpler repro: $array = array('one'); $array = $ref =& $array; The problem is that in zend_assign_to_variable() in the case where LHS and RHS point to the same value we currently do not destroy the RHS if it is a VAR. Proposed patch: diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index 554ad28..5f0caf6 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -81,6 +81,10 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval return variable_ptr; } if (ZEND_CONST_COND(value_type & (IS_VAR|IS_CV), 1) && variable_ptr == value) { + if (value_type == IS_VAR && ref) { + ZEND_ASSERT(GC_REFCOUNT(ref) > 1); + --GC_REFCOUNT(ref); + } return variable_ptr; } garbage = Z_COUNTED_P(variable_ptr); ------------------------------------------------------------------------ [2017-01-21 13:03:14] nikic@php.net Reduced testcase: function &leaked(array &$array = null) { $array = array('one'); return $array; } $array = leaked($array); ------------------------------------------------------------------------ [2017-01-21 12:50:19] cmb@php.net > This apparently affects 7.0.x but not 7.1.x Running the supplied test script on --enable-debug builds shows memory leaks for PHP-7.1 and master as well. ------------------------------------------------------------------------ [2017-01-19 21:38:16] danack@php.net This apparently affects 7.0.x but not 7.1.x https://3v4l.org/XBPgY ------------------------------------------------------------------------ [2017-01-19 18:05:46] highmind63 at gmail dot com Description: ------------ Memory leaks with referenced variable return in static method called by instance method. Test script: --------------- <pre> <?php class Leak { static protected function &leaked(array &$array = null) { $array = array('one'); return $array; } public function toLeaked($array = null) { $array = static::leaked($array); return $array; } public function testLeak() { echo "memory before loop: " . memory_get_usage(true) . PHP_EOL; for ($i = 0; $i < 5000; $i++) { $leak = $this->toLeaked(); } echo "memory after loop: " . memory_get_usage(true) . PHP_EOL; } } $leak = new Leak(); $leak->testLeak(); ?> </pre> Expected result: ---------------- No Leak Actual result: -------------- Leaks Memory: memory before loop: 2097152 memory after loop: 4194304 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=73960&edit=1

« previous php.bugs (#206850) next »