Skip to content

Conversation

arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Dec 19, 2024

Possible fix for GH-17211.

See #17211 (comment).

When observer is enabled, we normally add an extra temporary to all functions, to store the previously observed frame. However, this is done in zend_observer_post_startup() so it doesn't happen to dl'ed() functions.

One possible fix would be to move that from zend_observer_post_startup() to zend_register_functions(), but this would be too early: Observer may not be enabled when zend_register_functions() is called, and may still be enabled later.

However, when zend_register_functions() is called at run-time (during dl()), we know definitively whether observer is enabled.

Here I update zend_register_functions() to add a temporary to dl'ed() functions when observer is enabled.

@arnaud-lb arnaud-lb changed the base branch from master to PHP-8.2 December 19, 2024 19:16
@arnaud-lb arnaud-lb requested review from bwoebi and removed request for bukka, dstogov and kocsismate December 19, 2024 19:17
@arnaud-lb arnaud-lb closed this in 6f57993 Dec 20, 2024
charmitro pushed a commit to wasix-org/php that referenced this pull request Mar 13, 2025
When observer is enabled, we normally add an extra temporary to all
functions, to store the previously observed frame. However, this is done in
zend_observer_post_startup() so it doesn't happen to dl'ed() functions.

One possible fix would be to move that from zend_observer_post_startup()
to zend_register_functions(), but this would be too early: Observer may
not be enabled when zend_register_functions() is called, and may still be
enabled later.

However, when zend_register_functions() is called at run-time (during dl()),
we know definitively whether observer is enabled.

Here I update zend_register_functions() to add a temporary to dl'ed()
functions when observer is enabled.

Fixes: phpGH-17211
Closes: phpGH-17220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants