-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix GH-22759: Keep Io\Poll watcher registry in sync #22760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
e0bb7eb
9fd52d1
2eef263
8fd71ae
69a114c
10a46fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,7 @@ typedef struct php_io_poll_watcher_object { | |
| zval data; | ||
| bool active; | ||
| php_poll_ctx *poll_ctx; /* Back reference to poll context */ | ||
| HashTable *watchers; /* Back reference to context watcher registry */ | ||
| zend_object std; | ||
| } php_io_poll_watcher_object; | ||
|
|
||
|
|
@@ -252,6 +253,7 @@ static zend_object *php_io_poll_watcher_create_object(zend_class_entry *ce) | |
| intern->triggered_events = 0; | ||
| intern->active = false; | ||
| intern->poll_ctx = NULL; | ||
| intern->watchers = NULL; | ||
| ZVAL_NULL(&intern->data); | ||
|
|
||
| return &intern->std; | ||
|
|
@@ -294,6 +296,7 @@ static void php_io_poll_context_free_object(zend_object *obj) | |
| php_io_poll_watcher_object *watcher = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(Z_OBJ_P(zv)); | ||
| watcher->active = false; | ||
| watcher->poll_ctx = NULL; | ||
| watcher->watchers = NULL; | ||
| } ZEND_HASH_FOREACH_END(); | ||
| } | ||
|
|
||
|
|
@@ -638,13 +641,21 @@ PHP_METHOD(Io_Poll_Watcher, remove) | |
| RETURN_THROWS(); | ||
| } | ||
|
|
||
| php_poll_ctx *poll_ctx = intern->poll_ctx; | ||
| HashTable *watchers = intern->watchers; | ||
| zend_ulong hash_key = php_io_poll_compute_ptr_key(intern->handle); | ||
| php_socket_t fd = php_poll_handle_get_fd(intern->handle); | ||
| if (fd != SOCK_ERR) { | ||
| php_poll_remove(intern->poll_ctx, (int) fd); | ||
| php_poll_remove(poll_ctx, (int) fd); | ||
| } | ||
|
|
||
| intern->active = false; | ||
| intern->poll_ctx = NULL; | ||
| intern->watchers = NULL; | ||
|
|
||
| if (watchers) { | ||
| zend_hash_index_del(watchers, hash_key); | ||
| } | ||
| } | ||
|
|
||
| PHP_METHOD(Io_Poll_Context, __construct) | ||
|
|
@@ -727,8 +738,6 @@ PHP_METHOD(Io_Poll_Context, add) | |
| watcher->handle = handle; | ||
| watcher->watched_events = events; | ||
| watcher->triggered_events = 0; | ||
| watcher->active = true; | ||
| watcher->poll_ctx = intern->ctx; | ||
|
|
||
| GC_ADDREF(&handle->std); | ||
|
|
||
|
|
@@ -757,7 +766,17 @@ PHP_METHOD(Io_Poll_Context, add) | |
| GC_ADDREF(&watcher->std); | ||
|
|
||
| zend_ulong hash_key = php_io_poll_compute_ptr_key(handle); | ||
| zend_hash_index_add(intern->watchers, hash_key, &watcher_zv); | ||
| if (zend_hash_index_add(intern->watchers, hash_key, &watcher_zv) == NULL) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can't. I wrote this as defensive stuff just in case but I don't spot the duplicate handle tho. Thanks. |
||
| php_poll_remove(intern->ctx, (int) fd); | ||
| zval_ptr_dtor(&watcher_zv); | ||
| zend_throw_exception( | ||
| php_io_poll_handle_already_watched_class_entry, "Handle already added", 0); | ||
| RETURN_THROWS(); | ||
| } | ||
|
|
||
| watcher->active = true; | ||
| watcher->poll_ctx = intern->ctx; | ||
| watcher->watchers = intern->watchers; | ||
| } | ||
|
|
||
| PHP_METHOD(Io_Poll_Context, wait) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| --TEST-- | ||
| GH-22759 (Io\Poll: re-adding a removed handle keeps the new watcher tracked) | ||
| --FILE-- | ||
| <?php | ||
| require_once __DIR__ . '/poll.inc'; | ||
|
|
||
| [$r, $w] = pt_new_socket_pair(); | ||
| $context = pt_new_stream_poll(); | ||
| $handle = new StreamPollHandle($r); | ||
|
|
||
| $first = $context->add($handle, [Io\Poll\Event::Read]); | ||
| $first->remove(); | ||
|
LamentXU123 marked this conversation as resolved.
Outdated
|
||
|
|
||
| $second = $context->add($handle, [Io\Poll\Event::Read]); | ||
|
|
||
| unset($context); | ||
| gc_collect_cycles(); | ||
|
|
||
| var_dump($second->isActive()); | ||
|
|
||
| try { | ||
| $second->remove(); | ||
| } catch (Io\Poll\InactiveWatcherException $e) { | ||
| echo $e->getMessage(), "\n"; | ||
| } | ||
|
|
||
| echo "done\n"; | ||
| ?> | ||
| --EXPECT-- | ||
| bool(false) | ||
| Cannot remove inactive watcher | ||
| done | ||
Uh oh!
There was an error while loading. Please reload this page.