-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix lazy proxy calling magic methods twice #18039
base: PHP-8.4
Are you sure you want to change the base?
Conversation
Zend/zend_object_handlers.c
Outdated
@@ -970,6 +982,43 @@ static zend_always_inline bool property_uses_strict_types(void) { | |||
&& ZEND_CALL_USES_STRICT_TYPES(EG(current_execute_data)); | |||
} | |||
|
|||
static zend_always_inline zval *forward_write_to_lazy_object(zend_object *zobj, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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-- |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Fixes GH-18038.
Guard the underlying property, so that the forwarded operation accesses the property directly.