Skip to content

Commit 4d72bd7

Browse files
committed
Fix GH-12837: Combination of phpdbg and Generator method causes memory leak
The fix for bug #72523 (d1dd474) added a check on `zend_execute_ex` to add an extra refcount to `This`. This is necessary because the VM does a re-entry here: https://github.com/php/php-src/blob/ecb90c1db7efe9d87c07e31336185fc393281035/Zend/zend_vm_def.h#L4294-L4299 and then cleans up `This` here: https://github.com/php/php-src/blob/ecb90c1db7efe9d87c07e31336185fc393281035/Zend/zend_vm_def.h#L4358-L4360 So if we don't add the refcount, we destroy `This` both in the VM and in `zend_generator_close`. The reason this causes a leak in phpdbg is because it changes the `zend_execute_ex` temporarily back to the default here for `ZEND_DO_FCALL`: https://github.com/php/php-src/blob/ecb90c1db7efe9d87c07e31336185fc393281035/sapi/phpdbg/phpdbg_prompt.c#L1820-L1827 which means that we only execute `OBJ_RELEASE` on the `This` object once in `zend_generator_close` instead of also doing it in the `ZEND_DO_FCALL` handler. To solve this, we make sure the check for the re-entrancy is consistently done by checking the `ZEND_CALL_TOP` flag instead of relying solely on `execute_ex` pointer: https://github.com/php/php-src/blob/ecb90c1db7efe9d87c07e31336185fc393281035/Zend/zend_vm_def.h#L4298
1 parent b621b3a commit 4d72bd7

File tree

3 files changed

+46
-2
lines changed

3 files changed

+46
-2
lines changed

Zend/zend_vm_def.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -4534,7 +4534,7 @@ ZEND_VM_HANDLER(139, ZEND_GENERATOR_CREATE, ANY, ANY)
45344534
if ((call_info & Z_TYPE_MASK) == IS_OBJECT
45354535
&& (!(call_info & (ZEND_CALL_CLOSURE|ZEND_CALL_RELEASE_THIS))
45364536
/* Bug #72523 */
4537-
|| UNEXPECTED(zend_execute_ex != execute_ex))) {
4537+
|| (call_info & ZEND_CALL_TOP))) {
45384538
ZEND_ADD_CALL_FLAG_EX(call_info, ZEND_CALL_RELEASE_THIS);
45394539
Z_ADDREF(gen_execute_data->This);
45404540
}

Zend/zend_vm_execute.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sapi/phpdbg/tests/gh12837.phpt

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-12837 (Combination of phpdbg and Generator method causes memory leak)
3+
--CREDITS--
4+
ngyuki
5+
--FILE--
6+
<?php
7+
class A
8+
{
9+
public function __construct()
10+
{
11+
var_dump('__construct');
12+
}
13+
14+
public function __destruct()
15+
{
16+
var_dump('__destruct');
17+
}
18+
19+
public function iterate()
20+
{
21+
yield 1;
22+
}
23+
}
24+
25+
$arr = iterator_to_array((new A())->iterate()); gc_collect_cycles();
26+
$arr = iterator_to_array((new A())->iterate()); gc_collect_cycles();
27+
$arr = iterator_to_array((new A())->iterate()); gc_collect_cycles();
28+
29+
var_dump('exit');
30+
?>
31+
--PHPDBG--
32+
r
33+
q
34+
--EXPECTF--
35+
[Successful compilation of %s]
36+
prompt> string(11) "__construct"
37+
string(10) "__destruct"
38+
string(11) "__construct"
39+
string(10) "__destruct"
40+
string(11) "__construct"
41+
string(10) "__destruct"
42+
string(4) "exit"
43+
[Script ended normally]
44+
prompt>

0 commit comments

Comments
 (0)