Do not scan generator frames more than once #15330
Conversation
5fa7c8c to
e2722f7
Compare
dstogov
left a comment
There was a problem hiding this comment.
@arnaud-lb now you know this area better than me.
Merge this if you don't see problems.
Is this related to OSS-FUZZ #70879?
| * - If the generator is not running, the Generator's GC handler | ||
| * will inspect it. In this case we have to skip the frame. | ||
| */ | ||
| if (!(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING)) { |
There was a problem hiding this comment.
How can a generator be not running and on the fiber stacktrace at the same time? As far as I'm know, running generator <=> generator exists on exactly one stacktrace?
Isn't that check redundant?
There was a problem hiding this comment.
Generators stopped in yield from do not have the ZEND_GENERATOR_CURRENTLY_RUNNING flag. We consider them to be running in most places because we check zend_generator_get_current(generator)->flags instead of generator->flags.
There was a problem hiding this comment.
Ah, right, I forgot about the placeholder frame.
|
@dstogov yes this is related to this issue |
When a suspended Fiber contains a generator frame, the frame may be scanned twice: When scanning the Fiber, and again when scanning the Generator.
This change fixes this.
This also fixes a memory leak when an internal function call frame has the
ZEND_CALL_RELEASE_THISorZEND_CALL_CLOSUREflags, as these was ignored for internal calls inzend_unfinished_execution_gc_ex().