Differenciate WeakMaps from bare HashTables used as weak maps for GC purposes#19544
Differenciate WeakMaps from bare HashTables used as weak maps for GC purposes#19544arnaud-lb wants to merge 7 commits intophp:PHP-8.3from
Conversation
TimWolla
left a comment
There was a problem hiding this comment.
Would it make sense to add explicit handling to zend_weakmap_get_*_gc()? Something like:
else if (tag == ZEND_WEAKREF_TAG_BARE_HT) {
/* Bare HashTables are intentionally ignored, since they are intended for internal usage by extensions and might be allocated in globals. */
}
Basically to serve as documentation for the future reader.
|
For proper attribution: The test case is |
Given that this does not touch any header files, it should be safe to fix for 8.3. Nevertheless extensions would need to include workarounds either way, since they can't necessarily guarantee they are running on a fixed version of PHP. |
TimWolla
left a comment
There was a problem hiding this comment.
Soft LGTM. I'll double-check this against my actual use-case (incl. my work-around) to verify that this doesn't break anything before approving properly.
TimWolla
left a comment
There was a problem hiding this comment.
For my use case I can confirm that:
- 8.3 without this PR crashes.
- 8.3 with this PR works.
- 8.3 without this PR and my workaround works.
- 8.3 with this PR and my workaround works.
Thus this PR improves the situation without the workaround and doesn't break the workaround.
Possible fix for GH-19543.
Since cbf67e4, the GC needs to find all
WeakMapsreferencing a weakly referenced object. Doing so, it treats allZEND_WEAKREF_TAG_MAPasWeakMapinstances.However, a
ZEND_WEAKREF_TAG_MAPreference may be a bareHashTablewhenzend_weakrefs_hash_add()is used.Introduce a new tag,
ZEND_WEAKREF_TAG_BARE_HT, and use this tag when weakly referencing an object from a bareHashTable. Ignore such references in GC.cc @bwoebi @TimWolla
Edit: should probably target 8.3.