diff --git a/ext/standard/io_poll.c b/ext/standard/io_poll.c index c500247dc8be..693f72eaee7a 100644 --- a/ext/standard/io_poll.c +++ b/ext/standard/io_poll.c @@ -64,6 +64,7 @@ typedef struct { /* Stream poll handle specific data */ typedef struct { php_stream *stream; + zend_resource *res; } php_stream_poll_handle_data; /* Accessor macros */ @@ -250,7 +251,9 @@ static void php_stream_poll_handle_cleanup(php_poll_handle_object *handle) { php_stream_poll_handle_data *data = (php_stream_poll_handle_data *) handle->handle_data; if (data) { - /* Don't close the stream - user still owns it */ + if (data->res) { + zend_list_delete(data->res); + } efree(data); handle->handle_data = NULL; } @@ -331,6 +334,15 @@ static void php_io_poll_context_free_object(zend_object *obj) { php_io_poll_context_object *intern = PHP_POLL_CONTEXT_OBJ_FROM_ZOBJ(obj); + if (intern->watchers) { + zval *zv; + ZEND_HASH_FOREACH_VAL(intern->watchers, zv) { + php_io_poll_watcher_object *watcher = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(Z_OBJ_P(zv)); + watcher->active = false; + watcher->poll_ctx = NULL; + } ZEND_HASH_FOREACH_END(); + } + if (intern->ctx) { php_poll_destroy(intern->ctx); } @@ -343,6 +355,36 @@ static void php_io_poll_context_free_object(zend_object *obj) zend_object_std_dtor(&intern->std); } +static HashTable *php_io_poll_watcher_get_gc(zend_object *obj, zval **table, int *n) +{ + php_io_poll_watcher_object *intern = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(obj); + zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + + zend_get_gc_buffer_add_zval(gc_buffer, &intern->data); + if (intern->handle) { + zend_get_gc_buffer_add_obj(gc_buffer, &intern->handle->std); + } + + zend_get_gc_buffer_use(gc_buffer, table, n); + return NULL; +} + +static HashTable *php_io_poll_context_get_gc(zend_object *obj, zval **table, int *n) +{ + php_io_poll_context_object *intern = PHP_POLL_CONTEXT_OBJ_FROM_ZOBJ(obj); + zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + + if (intern->watchers) { + zval *zv; + ZEND_HASH_FOREACH_VAL(intern->watchers, zv) { + zend_get_gc_buffer_add_zval(gc_buffer, zv); + } ZEND_HASH_FOREACH_END(); + } + + zend_get_gc_buffer_use(gc_buffer, table, n); + return NULL; +} + /* Utility functions */ static zend_always_inline zend_ulong php_io_poll_compute_ptr_key(void *ptr) @@ -448,13 +490,19 @@ PHP_METHOD(StreamPollHandle, __construct) php_poll_handle_object *intern = PHP_POLL_HANDLE_OBJ_FROM_ZV(getThis()); + if (intern->handle_data) { + zend_throw_error(NULL, "StreamPollHandle object is already constructed"); + RETURN_THROWS(); + } + /* Set up stream-specific data */ php_stream_poll_handle_data *data = emalloc(sizeof(php_stream_poll_handle_data)); data->stream = stream; + data->res = stream->res; intern->handle_data = data; /* Add reference to stream */ - GC_ADDREF(stream->res); + GC_ADDREF(data->res); } PHP_METHOD(StreamPollHandle, getStream) @@ -657,6 +705,11 @@ PHP_METHOD(Io_Poll_Context, __construct) php_io_poll_context_object *intern = PHP_POLL_CONTEXT_OBJ_FROM_ZV(getThis()); + if (intern->ctx) { + zend_throw_error(NULL, "Io\\Poll\\Context object is already constructed"); + RETURN_THROWS(); + } + php_poll_backend_type backend_type = PHP_POLL_BACKEND_AUTO; if (backend_obj != NULL) { backend_type = php_io_poll_backend_enum_to_type(Z_OBJ_P(backend_obj)); @@ -861,6 +914,7 @@ PHP_MINIT_FUNCTION(poll) memcpy(&php_io_poll_handle_object_handlers, &std_object_handlers, sizeof(zend_object_handlers)); php_io_poll_handle_object_handlers.offset = offsetof(php_poll_handle_object, std); php_io_poll_handle_object_handlers.free_obj = php_poll_handle_object_free; + php_io_poll_handle_object_handlers.clone_obj = NULL; php_stream_poll_handle_class_entry->default_object_handlers = &php_io_poll_handle_object_handlers; /* Register Watcher class */ @@ -871,6 +925,8 @@ PHP_MINIT_FUNCTION(poll) sizeof(zend_object_handlers)); php_io_poll_watcher_object_handlers.offset = offsetof(php_io_poll_watcher_object, std); php_io_poll_watcher_object_handlers.free_obj = php_io_poll_watcher_free_object; + php_io_poll_watcher_object_handlers.get_gc = php_io_poll_watcher_get_gc; + php_io_poll_watcher_object_handlers.clone_obj = NULL; php_io_poll_watcher_class_entry->default_object_handlers = &php_io_poll_watcher_object_handlers; /* Register Context class */ @@ -881,6 +937,8 @@ PHP_MINIT_FUNCTION(poll) sizeof(zend_object_handlers)); php_io_poll_context_object_handlers.offset = offsetof(php_io_poll_context_object, std); php_io_poll_context_object_handlers.free_obj = php_io_poll_context_free_object; + php_io_poll_context_object_handlers.get_gc = php_io_poll_context_get_gc; + php_io_poll_context_object_handlers.clone_obj = NULL; php_io_poll_context_class_entry->default_object_handlers = &php_io_poll_context_object_handlers; /* Register exception hierarchy */ diff --git a/ext/standard/tests/poll/poll_clone_not_allowed.phpt b/ext/standard/tests/poll/poll_clone_not_allowed.phpt new file mode 100644 index 000000000000..f31c4c2ecd7f --- /dev/null +++ b/ext/standard/tests/poll/poll_clone_not_allowed.phpt @@ -0,0 +1,26 @@ +--TEST-- +Io\Poll: Context, Watcher and StreamPollHandle are not cloneable +--FILE-- +add($handle, [Io\Poll\Event::Read]); + +foreach ([$ctx, $handle, $watcher] as $obj) { + try { + clone $obj; + } catch (Error $e) { + echo $e->getMessage(), "\n"; + } +} + +echo "done\n"; +?> +--EXPECT-- +Trying to clone an uncloneable object of class Io\Poll\Context +Trying to clone an uncloneable object of class StreamPollHandle +Trying to clone an uncloneable object of class Io\Poll\Watcher +done diff --git a/ext/standard/tests/poll/poll_double_construct.phpt b/ext/standard/tests/poll/poll_double_construct.phpt new file mode 100644 index 000000000000..1fa5b65db38d --- /dev/null +++ b/ext/standard/tests/poll/poll_double_construct.phpt @@ -0,0 +1,28 @@ +--TEST-- +Io\Poll: calling __construct() twice throws instead of leaking +--FILE-- +__construct($r); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +$ctx = pt_new_stream_poll(); +try { + $ctx->__construct(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +echo "done\n"; +?> +--EXPECT-- +StreamPollHandle object is already constructed +Io\Poll\Context object is already constructed +done diff --git a/ext/standard/tests/poll/poll_stream_handle_close_then_free.phpt b/ext/standard/tests/poll/poll_stream_handle_close_then_free.phpt new file mode 100644 index 000000000000..9a97fad8dd1c --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_handle_close_then_free.phpt @@ -0,0 +1,20 @@ +--TEST-- +Io\Poll: StreamPollHandle cleanup is safe when the stream is closed first +--FILE-- +add(new StreamPollHandle($r), [Io\Poll\Event::Read]); + +// Close the underlying streams before the watcher and handle are freed. +fclose($r); +fclose($w); + +unset($watcher, $ctx); +gc_collect_cycles(); +echo "ok\n"; +?> +--EXPECT-- +ok diff --git a/ext/standard/tests/poll/poll_stream_handle_fd_release.phpt b/ext/standard/tests/poll/poll_stream_handle_fd_release.phpt new file mode 100644 index 000000000000..66d042c17138 --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_handle_fd_release.phpt @@ -0,0 +1,28 @@ +--TEST-- +Io\Poll: StreamPollHandle releases its stream resource (no fd leak) +--SKIPIF-- + +--FILE-- + +--EXPECT-- +bool(true) diff --git a/ext/standard/tests/poll/poll_watcher_gc_cycle.phpt b/ext/standard/tests/poll/poll_watcher_gc_cycle.phpt new file mode 100644 index 000000000000..590649acef8f --- /dev/null +++ b/ext/standard/tests/poll/poll_watcher_gc_cycle.phpt @@ -0,0 +1,31 @@ +--TEST-- +Io\Poll: cycle collector reclaims a Watcher referenced through its own data +--FILE-- +add(new StreamPollHandle($r), [Io\Poll\Event::Read]); + +$c = new Canary(); +$c->ref = $watcher; // Canary -> Watcher +$watcher->modifyData($c); // Watcher->data -> Canary (cycle) + +unset($ctx, $watcher, $c, $r, $w); + +echo "before gc\n"; +gc_collect_cycles(); +echo "after gc\n"; +?> +--EXPECT-- +before gc +Canary freed +after gc diff --git a/ext/standard/tests/poll/poll_watcher_outlives_context.phpt b/ext/standard/tests/poll/poll_watcher_outlives_context.phpt new file mode 100644 index 000000000000..2c058d5b8966 --- /dev/null +++ b/ext/standard/tests/poll/poll_watcher_outlives_context.phpt @@ -0,0 +1,35 @@ +--TEST-- +Io\Poll: Watcher operations are safe after its Context is destroyed +--FILE-- +add(new StreamPollHandle($r), [Io\Poll\Event::Read], "data"); + +// Drop the Context while still holding the Watcher it returned. +unset($ctx); +gc_collect_cycles(); + +var_dump($watcher->isActive()); + +try { + $watcher->remove(); +} catch (Io\Poll\InactiveWatcherException $e) { + echo $e->getMessage(), "\n"; +} + +try { + $watcher->modifyEvents([Io\Poll\Event::Write]); +} catch (Io\Poll\InactiveWatcherException $e) { + echo $e->getMessage(), "\n"; +} + +echo "done\n"; +?> +--EXPECT-- +bool(false) +Cannot remove inactive watcher +Cannot modify inactive watcher +done diff --git a/main/poll/poll_core.c b/main/poll/poll_core.c index 8422e46c9379..f985dbcdd3eb 100644 --- a/main/poll/poll_core.c +++ b/main/poll/poll_core.c @@ -191,7 +191,8 @@ PHPAPI php_poll_ctx *php_poll_create_by_name(const char *preferred_backend, uint /* Set event capacity hint (optional optimization) */ PHPAPI zend_result php_poll_set_max_events_hint(php_poll_ctx *ctx, int max_events) { - if (UNEXPECTED(!ctx || max_events <= 0)) { + ZEND_ASSERT(ctx); + if (UNEXPECTED(max_events <= 0)) { php_poll_set_error(ctx, PHP_POLL_ERR_INVALID); return FAILURE; } @@ -243,7 +244,8 @@ PHPAPI void php_poll_destroy(php_poll_ctx *ctx) /* Add file descriptor */ PHPAPI zend_result php_poll_add(php_poll_ctx *ctx, int fd, uint32_t events, void *data) { - if (UNEXPECTED(!ctx || !ctx->initialized || fd < 0)) { + ZEND_ASSERT(ctx); + if (UNEXPECTED(!ctx->initialized || fd < 0)) { php_poll_set_error(ctx, PHP_POLL_ERR_INVALID); return FAILURE; } @@ -259,7 +261,8 @@ PHPAPI zend_result php_poll_add(php_poll_ctx *ctx, int fd, uint32_t events, void /* Modify file descriptor */ PHPAPI zend_result php_poll_modify(php_poll_ctx *ctx, int fd, uint32_t events, void *data) { - if (UNEXPECTED(!ctx || !ctx->initialized || fd < 0)) { + ZEND_ASSERT(ctx); + if (UNEXPECTED(!ctx->initialized || fd < 0)) { php_poll_set_error(ctx, PHP_POLL_ERR_INVALID); return FAILURE; } @@ -275,7 +278,8 @@ PHPAPI zend_result php_poll_modify(php_poll_ctx *ctx, int fd, uint32_t events, v /* Remove file descriptor */ PHPAPI zend_result php_poll_remove(php_poll_ctx *ctx, int fd) { - if (UNEXPECTED(!ctx || !ctx->initialized || fd < 0)) { + ZEND_ASSERT(ctx); + if (UNEXPECTED(!ctx->initialized || fd < 0)) { php_poll_set_error(ctx, PHP_POLL_ERR_INVALID); return FAILURE; } @@ -292,7 +296,8 @@ PHPAPI zend_result php_poll_remove(php_poll_ctx *ctx, int fd) PHPAPI int php_poll_wait(php_poll_ctx *ctx, php_poll_event *events, int max_events, const struct timespec *timeout) { - if (UNEXPECTED(!ctx || !ctx->initialized || !events || max_events <= 0)) { + ZEND_ASSERT(ctx); + if (UNEXPECTED(!ctx->initialized || !events || max_events <= 0)) { php_poll_set_error(ctx, PHP_POLL_ERR_INVALID); return -1; }