Skip to content

streams: Re-add support for arbitrary metadata for stream notifier functions - #19158

Merged
TimWolla merged 2 commits into
php:masterfrom
TimWolla:stream-notifier-ptr
Jul 17, 2025
Merged

TimWolla merged 2 commits into
php:masterfrom
TimWolla:stream-notifier-ptr

Conversation

@TimWolla

Copy link
Copy Markdown
Member

Support for attaching arbitrary metadata for consumption by stream notifier functions got broken in #19024 when the previous zval ptr got changed to zend_fcall_info_cache *fcc, making the data specific to the user_space_stream_notifier.

Fix this by changing the field to void *ptr, avoiding the indirection through a zval, but preserving the ability to store arbitrary data. If necessary to support different types than IS_PTR, extensions can heap-allocate a zval to store within ->ptr as a minimal change to keep compatibility with PHP 8.4 or lower.

…nctions

Support for attaching arbitrary metadata for consumption by stream notifier
functions got broken in php#19024 when the previous `zval ptr` got
changed to `zend_fcall_info_cache *fcc`, making the data specific to the
`user_space_stream_notifier`.

Fix this by changing the field to `void *ptr`, avoiding the indirection through
a `zval`, but preserving the ability to store arbitrary data. If necessary to
support different types than `IS_PTR`, extensions can heap-allocate a `zval` to
store within `->ptr` as a minimal change to keep compatibility with PHP 8.4 or
lower.
@TimWolla
TimWolla force-pushed the stream-notifier-ptr branch from 551dc7b to f1ef6f0 Compare July 17, 2025 13:31

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching this

@Girgias Girgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching this!

@TimWolla
TimWolla merged commit 6b3f302 into php:master Jul 17, 2025
@TimWolla
TimWolla deleted the stream-notifier-ptr branch July 17, 2025 21:01
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