Skip to content

Fix GH-25238: ReflectionProperty::isInitialized() skips lazy init once cached - #25239

Merged
arnaud-lb merged 4 commits into
php:PHP-8.5from
lazerg:fix/gh-25238-isinitialized-lazy
Nov 6, 2026
Merged

arnaud-lb merged 4 commits into
php:PHP-8.5from
lazerg:fix/gh-25238-isinitialized-lazy

Conversation

@lazerg

@lazerg lazerg commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

The inline cache fast path in isInitialized() returned false as soon as the cached slot was UNDEF. All properties of an uninitialized lazy object are UNDEF, and so are the slots of a lazy proxy, so once the cache was warm, later lazy objects of the same class were reported as uninitialized and never got initialized. The fast path now falls back to has_property() only when the slot is UNDEF and the object is lazy. Non-lazy objects still get the cached answer.

Fixes #25238

Comment thread ext/reflection/php_reflection.c Outdated
Comment on lines +6491 to +6493
if (EXPECTED(!Z_ISUNDEF_P(value))) {
RETURN_TRUE;
}

@prateekbhujel prateekbhujel Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Falling through to has_property() on every IS_UNDEF slot regresses the inline cache fast path from #17698 (8af90424055) whenever isInitialized() is called on an uninitialized or unset() property of a normal (non-lazy) object. Unlike getValue() where IS_UNDEF is an exceptional slow path, returning false for uninitialized typed properties is a primary use case of isInitialized() in serializers/validators, so this completely bypasses the cache for all false results on non-lazy objects.

cc @arnaud-lb

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed in ab0be44. The fast path now returns false directly for UNDEF slots when the object is not lazy, and only lazy objects take the has_property() path.

$line = $class->newLazyGhost(function (Line $line) use ($label) {
$line->label = $label;
});
var_dump($property->isInitialized($line));

@prateekbhujel prateekbhujel Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This only tests newLazyGhost() where the property is initialized inside the callback, and misses newLazyProxy() (both before and after initialization) as well as a lazy object whose initializer leaves the property uninitialized.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in ab0be44: newLazyProxy() before and after initialization, and a ghost whose initializer leaves the property uninitialized. All of them fail without the fix.

@arnaud-lb
arnaud-lb merged commit 020d474 into php:PHP-8.5 Nov 6, 2026
1 check passed
arnaud-lb added a commit that referenced this pull request Nov 6, 2026
* PHP-8.6:
  Fix GH-25238: ReflectionProperty::isInitialized() skips lazy init once cached (#25239)
@arnaud-lb

Copy link
Copy Markdown
Member

Thank you @lazerg @prateekbhujel!

prateekbhujel pushed a commit to prateekbhujel/php-src that referenced this pull request Nov 6, 2026
* PHP-8.5:
  Fix phpGH-25238: ReflectionProperty::isInitialized() skips lazy init once cached (php#25239)
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