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; +}