Skip to content

Conversation

@heathdutton
Copy link

@heathdutton heathdutton commented Dec 16, 2025

When a DateInterval object has a circular reference (e.g., $obj->prop = $obj), calling json_encode() triggers an assertion failure in debug builds. This happens because the get_properties handler modifies the properties HashTable in place, but circular references cause the HashTable to have a refcount > 1, which violates the assertion in zend_hash_str_update().

The fix adds a get_properties_for handler for DateInterval that returns a duplicated HashTable. To ensure circular reference detection still works (which relies on HashTable identity), the duplicated HashTable is cached in module globals, keyed by object handle. When we detect a recursive call (cache entry exists with refcount > 1), we return the same cached HashTable so that GC_IS_RECURSIVE can properly detect the cycle.

This approach avoids any ABI break by using module globals instead of adding a field to php_interval_obj.

Fixes GH-20503

Copy link
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

Thank you @heathdutton! The approach looks right, but please check comments. (Edit: Please change the base of the PR to PHP-8.3. The branch is based on 8.3, but not the PR.)

@arnaud-lb
Copy link
Member

NB: In master, DateInterval could be greatly simplified by using standard object properties and removing all the prop handlers.

@heathdutton heathdutton changed the base branch from master to PHP-8.3 December 19, 2025 17:01
@heathdutton heathdutton marked this pull request as draft December 19, 2025 17:03
@heathdutton heathdutton force-pushed the fix-gh-20503 branch 3 times, most recently from 2af857e to 6ec27cd Compare December 19, 2025 18:14
…y hash

When a DateInterval object has a circular reference (e.g., $obj->prop = $obj), calling json_encode() triggered an assertion failure because the get_properties handler modified a HashTable with refcount > 1.

Fixed by duplicating the properties HashTable when its refcount is greater than 1 before modifying it.
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.

Assertion failure with ext/date DateInterval property hash construction

2 participants