Skip to content

Fix lazy proxy calling magic methods twice#18039

Closed
arnaud-lb wants to merge 3 commits intophp:PHP-8.4from
arnaud-lb:lazy-magic
Closed

Fix lazy proxy calling magic methods twice#18039
arnaud-lb wants to merge 3 commits intophp:PHP-8.4from
arnaud-lb:lazy-magic

Conversation

@arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Mar 13, 2025

Fixes GH-18038.

Guard the underlying property, so that the forwarded operation accesses the property directly.

&& ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data));
}

static zend_always_inline zval *forward_write_to_lazy_object(zend_object *zobj,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's worth to always_inline this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I will check if it makes a noticeable difference

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the always inline attribute

var_dump($obj->prop);

?>
--EXPECTF--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the __get function should get called before instantiating the object? I.e. why does "C::__get" appear before "init"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s how lazy objects are supposed to behave: we interact primarily with the proxy (we execute the proxy’s code), but any access to its state is forwarded to the real instance.

So here we execute the getter, which tries to access a property. This triggers initialization before the access is forwarded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see, not what I intuitively expected but it makes sense

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose there's also an implicit assumption here that the initialized properties does not contain dynamic properties. If it does, the initializer would need to be called before __get, to ensure that the property is actually still does not exist after the initializer has been called.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any other issues. If the aforementioned dynamic property behavior is correct, then this LGTM.

@arnaud-lb arnaud-lb closed this in 26f5009 Mar 27, 2025
arnaud-lb added a commit that referenced this pull request Feb 3, 2026
In GH-18039 we guard the underlying property before forwarding access
to the real instance of a lazy proxy. When the real instance lacks magic
methods, the assertion zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS fails in
zend_get_property_guard().

Fix by checking that the real instance uses guards.

Fixes GH-20504
Closes GH-21093
iliaal added a commit to iliaal/php-src that referenced this pull request Mar 17, 2026
…azy proxies

zend_std_get_property_ptr_ptr() was the only property handler that did
not propagate the IN_GET guard to the underlying object when forwarding
from a lazy proxy after initialization. This caused __get to be called
on the underlying object when it shouldn't be, leading to assertion
failures.

The same guard-copying pattern already existed in read_property,
write_property, unset_property, and has_property since commit
26f5009 (phpGH-18039).

Also fixes phpGH-20873 and phpGH-20854.

Closes phpGH-20875
iliaal added a commit to iliaal/php-src that referenced this pull request Mar 19, 2026
…azy proxies

zend_std_get_property_ptr_ptr() was the only property handler that did
not propagate the IN_GET guard to the underlying object when forwarding
from a lazy proxy after initialization. This caused __get to be called
on the underlying object when it shouldn't be, leading to assertion
failures.

The same guard-copying pattern already existed in read_property,
write_property, unset_property, and has_property since commit
26f5009 (phpGH-18039).

Also fixes phpGH-20873 and phpGH-20854.

Closes phpGH-20875
arnaud-lb pushed a commit that referenced this pull request Mar 20, 2026
… proxies

zend_std_get_property_ptr_ptr() was the only property handler that did
not propagate the IN_GET guard to the underlying object when forwarding
from a lazy proxy after initialization. This caused __get to be called
on the underlying object when it shouldn't be, leading to assertion
failures.

The same guard-copying pattern already existed in read_property,
write_property, unset_property, and has_property since commit
26f5009 (GH-18039).

Also fixes GH-20873 and GH-20854.

Closes GH-20875
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