From aeba1ac49c6bfd0a74be3303f21623f55259712c Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Wed, 30 Sep 2026 13:18:16 +0200 Subject: [PATCH] Report Io\Poll level watchers with buffered stream data A stream can hold unread data in its buffer while its descriptor is idle. stream_select() treats such a stream as readable but a level watcher looked at the descriptor only, so a loop waiting for readability never woke up (GH-23948). Context::wait() now checks level watchers with Read interest for buffered data first and reports them with Read triggered. The backend is then asked without waiting so descriptor events are merged in and a watcher readable both ways is returned once. Edge watchers are not affected. A one-shot reported from its buffer is disarmed like one fired by the backend and modifyEvents() re-arms it. The check reads the stream buffer positions only and does no IO. The check is a handle operation so other handle types can provide it. It is registered separately as php_poll_handle_ext_ops because the handle ops struct cannot grow without an ABI change. This is temporary and the operation moves to the ops struct in master. --- NEWS | 2 + ext/standard/io_poll.c | 126 +++++++++++++++--- .../tests/poll/poll_stream_buffered_edge.phpt | 32 +++++ .../poll/poll_stream_buffered_level.phpt | 55 ++++++++ .../poll/poll_stream_buffered_max_events.phpt | 42 ++++++ .../poll/poll_stream_buffered_merge.phpt | 51 +++++++ .../poll/poll_stream_buffered_oneshot.phpt | 57 ++++++++ main/php_poll.h | 10 ++ main/poll/poll_handle.c | 30 +++++ 9 files changed, 390 insertions(+), 15 deletions(-) create mode 100644 ext/standard/tests/poll/poll_stream_buffered_edge.phpt create mode 100644 ext/standard/tests/poll/poll_stream_buffered_level.phpt create mode 100644 ext/standard/tests/poll/poll_stream_buffered_max_events.phpt create mode 100644 ext/standard/tests/poll/poll_stream_buffered_merge.phpt create mode 100644 ext/standard/tests/poll/poll_stream_buffered_oneshot.phpt diff --git a/NEWS b/NEWS index 1e633eb223fd..20daec0d4979 100644 --- a/NEWS +++ b/NEWS @@ -550,6 +550,8 @@ PHP NEWS (Nicolas Grekas) . Fixed convert.quoted-printable-decode decoding lowercase hex digits to the wrong byte. (Ilia Alshanetsky) + . Fixed bug GH-23948 (Io\Poll: data buffered by the stream layer is not + reported). (Jakub Zelenka) - Tidy: . Fixed a use-after-free when a tidyNode is used after its document is diff --git a/ext/standard/io_poll.c b/ext/standard/io_poll.c index c92201c1f299..583dfdbc0335 100644 --- a/ext/standard/io_poll.c +++ b/ext/standard/io_poll.c @@ -56,6 +56,9 @@ typedef struct php_io_poll_watcher_object { zval data; bool active; bool closed; /* Deactivated because its stream was closed */ + bool disarmed; /* Fired one-shot, until modifyEvents() */ + bool reported; /* Reported from its buffer by the running wait() */ + const php_poll_handle_ext_ops *ext_ops; php_io_poll_context_object *context; /* Back reference to Context object */ php_socket_t fd; /* Registered fd, SOCK_ERR when inactive */ php_stream *stream; /* Watched stream, NULL when not registered in its watcher list */ @@ -66,6 +69,7 @@ typedef struct php_io_poll_watcher_object { struct php_io_poll_context_object { php_poll_ctx *ctx; HashTable *watchers; /* Maps fd -> watcher object */ + bool buffered_readers; /* Some watcher can report buffered data */ zend_object std; }; @@ -239,6 +243,17 @@ static php_poll_handle_ops php_stream_poll_handle_ops = { .cleanup = php_stream_poll_handle_cleanup }; +/* The stream buffer only, without any IO */ +static bool php_stream_poll_handle_has_buffered_data(php_poll_handle_object *handle) +{ + php_stream *stream = php_stream_poll_handle_get_stream(handle); + return stream && stream->writepos > stream->readpos; +} + +static const php_poll_handle_ext_ops php_stream_poll_handle_ext_ops = { + .has_buffered_data = php_stream_poll_handle_has_buffered_data +}; + /* Handle interface internal only */ static int php_stream_poll_handle_implement_interface(zend_class_entry *interface, zend_class_entry *implementor) { @@ -271,6 +286,9 @@ static zend_object *php_io_poll_watcher_create_object(zend_class_entry *ce) intern->triggered_events = 0; intern->active = false; intern->closed = false; + intern->disarmed = false; + intern->reported = false; + intern->ext_ops = NULL; intern->context = NULL; intern->fd = SOCK_ERR; intern->stream = NULL; @@ -288,6 +306,7 @@ static zend_object *php_io_poll_context_create_object(zend_class_entry *ce) intern->ctx = NULL; intern->watchers = NULL; + intern->buffered_readers = false; return &intern->std; } @@ -300,6 +319,13 @@ static zend_always_inline zend_ulong php_io_poll_compute_ptr_key(void *ptr) return (key >> 3) | (key << ((sizeof(key) * 8) - 3)); } +/* Level Read watcher whose handle can have buffered data */ +static zend_always_inline bool php_io_poll_watcher_reads_buffered(php_io_poll_watcher_object *watcher) +{ + return watcher->ext_ops && watcher->ext_ops->has_buffered_data + && (watcher->watched_events & (PHP_POLL_READ | PHP_POLL_ET)) == PHP_POLL_READ; +} + static zend_always_inline void php_io_poll_watcher_deactivate(php_io_poll_watcher_object *watcher) { watcher->active = false; @@ -461,6 +487,10 @@ static zend_result php_io_poll_watcher_modify_events( /* Update watcher state */ watcher->watched_events = events; + watcher->disarmed = false; + if (php_io_poll_watcher_reads_buffered(watcher)) { + watcher->context->buffered_readers = true; + } return SUCCESS; } @@ -841,6 +871,10 @@ PHP_METHOD(Io_Poll_Context, add) watcher->active = true; watcher->context = intern; watcher->fd = fd; + watcher->ext_ops = php_poll_handle_get_ext_ops(handle->ops); + if (php_io_poll_watcher_reads_buffered(watcher)) { + intern->buffered_readers = true; + } if (handle->ops == &php_stream_poll_handle_ops) { php_stream *stream = php_stream_poll_handle_get_stream(handle); @@ -889,33 +923,94 @@ PHP_METHOD(Io_Poll_Context, wait) RETURN_THROWS(); } - php_poll_event *events = safe_emalloc((size_t) max_events, sizeof(*events), 0); - int num_events = php_poll_wait(intern->ctx, events, (int) max_events, timeout ? &timeout_ts : NULL); + /* Buffered watchers go first, then a non-blocking backend check */ + php_io_poll_watcher_object **buffered = NULL; + int n_buffered = 0; + if (intern->buffered_readers) { + ZEND_HASH_FOREACH_VAL(intern->watchers, zval *zv) { + php_io_poll_watcher_object *watcher = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(Z_OBJ_P(zv)); + if (watcher->disarmed || !php_io_poll_watcher_reads_buffered(watcher) + || !watcher->ext_ops->has_buffered_data(watcher->handle)) { + continue; + } + if (!buffered) { + buffered = safe_emalloc((size_t) max_events, sizeof(*buffered), 0); + } + buffered[n_buffered++] = watcher; + if (n_buffered == max_events) { + break; + } + } ZEND_HASH_FOREACH_END(); + } - if (num_events < 0) { - php_poll_error err = php_poll_get_error(intern->ctx); - efree(events); - php_io_poll_throw_failed_operation( - php_io_poll_failed_wait_class_entry, "Poll wait failed", err); - RETURN_THROWS(); + php_poll_event *events = safe_emalloc((size_t) max_events, sizeof(*events), 0); + int num_events = 0; + if (n_buffered < max_events) { + struct timespec zero_ts = {0, 0}; + num_events = php_poll_wait(intern->ctx, events, (int) max_events - n_buffered, + n_buffered ? &zero_ts : (timeout ? &timeout_ts : NULL)); + if (num_events < 0) { + php_poll_error err = php_poll_get_error(intern->ctx); + if (n_buffered == 0 || err != PHP_POLL_ERR_INTERRUPTED) { + efree(events); + if (buffered) { + efree(buffered); + } + php_io_poll_throw_failed_operation( + php_io_poll_failed_wait_class_entry, "Poll wait failed", err); + RETURN_THROWS(); + } + /* The buffered data is the result */ + num_events = 0; + } } - array_init(return_value); + array_init_size(return_value, (uint32_t) (n_buffered + num_events)); + + for (int i = 0; i < n_buffered; i++) { + php_io_poll_watcher_object *watcher = buffered[i]; + watcher->triggered_events = PHP_POLL_READ; + watcher->reported = true; + + zval watcher_zv; + ZVAL_OBJ_COPY(&watcher_zv, &watcher->std); + add_next_index_zval(return_value, &watcher_zv); + } for (int i = 0; i < num_events; i++) { php_io_poll_watcher_object *watcher = (php_io_poll_watcher_object *) events[i].data; - if (watcher) { - watcher->triggered_events = events[i].revents; + if (!watcher) { + continue; + } + if (watcher->watched_events & PHP_POLL_ONESHOT) { + watcher->disarmed = true; + } + if (watcher->reported) { + /* Already reported from its buffer */ + watcher->triggered_events |= events[i].revents; + continue; + } + watcher->triggered_events = events[i].revents; - zval watcher_zv; - ZVAL_OBJ(&watcher_zv, &watcher->std); - GC_ADDREF(&watcher->std); + zval watcher_zv; + ZVAL_OBJ_COPY(&watcher_zv, &watcher->std); + add_next_index_zval(return_value, &watcher_zv); + } - add_next_index_zval(return_value, &watcher_zv); + /* Reset the per-wait flag. A buffer-only one-shot is still armed in the backend */ + for (int i = 0; i < n_buffered; i++) { + php_io_poll_watcher_object *watcher = buffered[i]; + watcher->reported = false; + if ((watcher->watched_events & PHP_POLL_ONESHOT) && !watcher->disarmed) { + watcher->disarmed = true; + php_poll_remove(intern->ctx, (int) watcher->fd); } } efree(events); + if (buffered) { + efree(buffered); + } } PHP_METHOD(Io_Poll_Context, getBackend) @@ -1015,6 +1110,7 @@ PHP_MINIT_FUNCTION(poll) /* Initialize polling backends */ php_poll_register_backends(); + php_poll_handle_register_ext_ops(&php_stream_poll_handle_ops, &php_stream_poll_handle_ext_ops); return SUCCESS; } diff --git a/ext/standard/tests/poll/poll_stream_buffered_edge.phpt b/ext/standard/tests/poll/poll_stream_buffered_edge.phpt new file mode 100644 index 000000000000..08495511fd8a --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_buffered_edge.phpt @@ -0,0 +1,32 @@ +--TEST-- +Io\Poll: buffered data does not fire an edge-triggered watcher +--SKIPIF-- + +--FILE-- +wait(Time\Duration::fromMilliseconds(100)))); + +// A new edge on the descriptor is reported as usual +fwrite($b, "third\n"); +$events = $ctx->wait(Time\Duration::fromMilliseconds(100)); +var_dump(count($events), $events[0] === $w); +?> +--EXPECT-- +string(6) "first +" +int(0) +int(1) +bool(true) diff --git a/ext/standard/tests/poll/poll_stream_buffered_level.phpt b/ext/standard/tests/poll/poll_stream_buffered_level.phpt new file mode 100644 index 000000000000..b1ef50c3ed2c --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_buffered_level.phpt @@ -0,0 +1,55 @@ +--TEST-- +Io\Poll: a level watcher reports Read while the stream layer holds buffered data +--FILE-- +wait(Time\Duration::fromSeconds(5)); +var_dump(count($events), $events[0] === $w, $events[0]->hasTriggered(Io\Poll\Event::Read)); +var_dump(hrtime(true) - $start < 2_000_000_000); +var_dump(stream_get_meta_data($a)['unread_bytes']); + +// Level: reported on every wait while the data stays buffered +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); + +var_dump(fgets($a)); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); + +// Write interest is not concerned with read data +fwrite($b, "third\nfourth\n"); +var_dump(fgets($a)); +$w->modifyEvents([Io\Poll\Event::Write]); +$events = $ctx->wait(Time\Duration::fromSeconds(0)); +var_dump(count($events), $events[0]->hasTriggered(Io\Poll\Event::Write), $events[0]->hasTriggered(Io\Poll\Event::Read)); + +// The buffer counts as soon as Read interest is back +$w->modifyEvents([Io\Poll\Event::Read]); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +?> +--EXPECT-- +string(6) "first +" +int(1) +bool(true) +bool(true) +bool(true) +int(7) +int(1) +string(7) "second +" +int(0) +string(6) "third +" +int(1) +bool(true) +bool(false) +int(1) diff --git a/ext/standard/tests/poll/poll_stream_buffered_max_events.phpt b/ext/standard/tests/poll/poll_stream_buffered_max_events.phpt new file mode 100644 index 000000000000..6bb2109afb6b --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_buffered_max_events.phpt @@ -0,0 +1,42 @@ +--TEST-- +Io\Poll: buffered data reports count against maxEvents +--FILE-- +wait(Time\Duration::fromSeconds(0), 1))); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0), 2))); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0), 3))); + +var_dump(fgets($a), fgets($c), fread($e, 10)); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +?> +--EXPECT-- +string(6) "first +" +string(6) "first +" +int(1) +int(2) +int(3) +int(3) +string(7) "second +" +string(7) "second +" +string(1) "x" +int(0) diff --git a/ext/standard/tests/poll/poll_stream_buffered_merge.phpt b/ext/standard/tests/poll/poll_stream_buffered_merge.phpt new file mode 100644 index 000000000000..90111306ae4c --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_buffered_merge.phpt @@ -0,0 +1,51 @@ +--TEST-- +Io\Poll: buffered data is merged with descriptor events, one entry per watcher +--FILE-- +wait(Time\Duration::fromMilliseconds(100)); +$seen = []; +foreach ($events as $e) { + $seen[$e->getData()] = $e->hasTriggered(Io\Poll\Event::Read); +} +ksort($seen); +var_dump($seen); + +// $a: buffered data and data on the socket at once: one entry, Read once +fwrite($b, "third\n"); +var_dump(fread($c, 10)); +$events = $ctx->wait(Time\Duration::fromMilliseconds(100)); +var_dump(count($events), $events[0] === $wa, $events[0]->hasTriggered(Io\Poll\Event::Read)); +var_dump(fgets($a), fgets($a)); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +?> +--EXPECT-- +string(6) "first +" +array(2) { + ["a"]=> + bool(true) + ["c"]=> + bool(true) +} +string(1) "x" +int(1) +bool(true) +bool(true) +string(7) "second +" +string(6) "third +" +int(0) diff --git a/ext/standard/tests/poll/poll_stream_buffered_oneshot.phpt b/ext/standard/tests/poll/poll_stream_buffered_oneshot.phpt new file mode 100644 index 000000000000..80eba04300c9 --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_buffered_oneshot.phpt @@ -0,0 +1,57 @@ +--TEST-- +Io\Poll: a one-shot watcher reported from buffered data is disarmed until re-armed +--FILE-- +wait(Time\Duration::fromSeconds(0)); +var_dump(count($events), $events[0] === $w); + +// Disarmed: neither the buffer nor new data on the socket reports it +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +fwrite($b, "third\n"); +var_dump(count($ctx->wait(Time\Duration::fromMilliseconds(50)))); +var_dump($w->isActive()); + +// Re-armed: the buffer reports it at once, again once +$w->modifyEvents([Io\Poll\Event::Read, Io\Poll\Event::OneShot]); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); + +// Drained, re-armed: the descriptor reports it +var_dump(fgets($a), fgets($a)); +$w->modifyEvents([Io\Poll\Event::Read, Io\Poll\Event::OneShot]); +var_dump(count($ctx->wait(Time\Duration::fromSeconds(0)))); +fwrite($b, "fourth\n"); +$events = $ctx->wait(Time\Duration::fromMilliseconds(100)); +var_dump(count($events), $events[0]->hasTriggered(Io\Poll\Event::Read)); + +$w->remove(); +var_dump($w->isActive()); +?> +--EXPECT-- +string(6) "first +" +int(1) +bool(true) +int(0) +int(0) +bool(true) +int(1) +int(0) +string(7) "second +" +string(6) "third +" +int(0) +int(1) +bool(true) +bool(false) diff --git a/main/php_poll.h b/main/php_poll.h index 9b878d44bd20..7f92e70ce789 100644 --- a/main/php_poll.h +++ b/main/php_poll.h @@ -173,4 +173,14 @@ PHPAPI void php_poll_handle_object_free(zend_object *obj); /* Get file descriptor from any poll handle */ PHPAPI php_socket_t php_poll_handle_get_fd(php_poll_handle_object *handle); +/* Extra handle ops registered per ops pointer (temporary, ABI) */ +typedef struct php_poll_handle_ext_ops { + /* Data above the descriptor that a read returns without waiting */ + bool (*has_buffered_data)(php_poll_handle_object *handle); +} php_poll_handle_ext_ops; + +PHPAPI zend_result php_poll_handle_register_ext_ops( + const php_poll_handle_ops *ops, const php_poll_handle_ext_ops *ext_ops); +PHPAPI const php_poll_handle_ext_ops *php_poll_handle_get_ext_ops(const php_poll_handle_ops *ops); + #endif /* PHP_POLL_H */ diff --git a/main/poll/poll_handle.c b/main/poll/poll_handle.c index 228d44e1e420..41adaebf91a3 100644 --- a/main/poll/poll_handle.c +++ b/main/poll/poll_handle.c @@ -91,3 +91,33 @@ PHPAPI php_socket_t php_poll_handle_get_fd(php_poll_handle_object *handle) return handle->ops->get_fd(handle); } + +#define PHP_POLL_HANDLE_EXT_OPS_MAX 16 + +static struct { + const php_poll_handle_ops *ops; + const php_poll_handle_ext_ops *ext_ops; +} php_poll_handle_ext_ops_registry[PHP_POLL_HANDLE_EXT_OPS_MAX]; +static int php_poll_handle_ext_ops_count = 0; + +PHPAPI zend_result php_poll_handle_register_ext_ops( + const php_poll_handle_ops *ops, const php_poll_handle_ext_ops *ext_ops) +{ + if (!ops || !ext_ops || php_poll_handle_ext_ops_count == PHP_POLL_HANDLE_EXT_OPS_MAX) { + return FAILURE; + } + php_poll_handle_ext_ops_registry[php_poll_handle_ext_ops_count].ops = ops; + php_poll_handle_ext_ops_registry[php_poll_handle_ext_ops_count].ext_ops = ext_ops; + php_poll_handle_ext_ops_count++; + return SUCCESS; +} + +PHPAPI const php_poll_handle_ext_ops *php_poll_handle_get_ext_ops(const php_poll_handle_ops *ops) +{ + for (int i = 0; i < php_poll_handle_ext_ops_count; i++) { + if (php_poll_handle_ext_ops_registry[i].ops == ops) { + return php_poll_handle_ext_ops_registry[i].ext_ops; + } + } + return NULL; +}