Fix GH-12837: Combination of phpdbg and Generator method causes memory leak #17303
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The fix for bug #72523 (d1dd474) added a check on
zend_execute_ex
to add an extra refcount toThis
. This is necessary because the VM does a re-entry here:php-src/Zend/zend_vm_def.h
Lines 4294 to 4299 in ecb90c1
and then cleans up
This
here:php-src/Zend/zend_vm_def.h
Lines 4358 to 4360 in ecb90c1
So if we don't add the refcount, we destroy
This
both in the VM and inzend_generator_close
.The reason this causes a leak in phpdbg is because phpdbg temporarily changes
zend_execute_ex
to the default here forZEND_DO_FCALL
:php-src/sapi/phpdbg/phpdbg_prompt.c
Lines 1820 to 1827 in ecb90c1
OBJ_RELEASE
on theThis
object once inzend_generator_close
instead of also doing it in theZEND_DO_FCALL
handler.To solve this, we make sure the check for the re-entrancy is consistently done between
ZEND_DO_FCALL
andZEND_GENERATOR_CREATE
by checking theZEND_CALL_TOP
flag instead of relying solely onexecute_ex
pointer, because this flag is set whenzend_execute_ex
was changed:php-src/Zend/zend_vm_def.h
Line 4298 in ecb90c1
It may over-approximate if called via
zend_call_function
, as that sets theZEND_CALL_TOP
flag too, but we could filter that by checking forZEND_CALL_DYNAMIC
too.Alternatively, we can fix this solely in phpdbg by also executing
GENERATOR_CREATE
with the normal execution handler, but I think the real problem is in the VM inconsistency.