diff --git a/NEWS b/NEWS index 1e633eb223fd..e5892e70515b 100644 --- a/NEWS +++ b/NEWS @@ -195,6 +195,8 @@ PHP NEWS properties). (Lazizbek Ergashev) - Standard: + . Fixed bug GH-25086 (Io\Poll kqueue backend leaks memory on every + Watcher::modifyEvents() call). (Lazizbek Ergashev, Jakub Zelenka) . Fixed memory leaks in checkdnsrr() and dns_check_record() on Windows. (Ilia Alshanetsky) . Fixed __unserialize() and __wakeup() being called for objects with a missing diff --git a/ext/standard/tests/poll/gh25086.phpt b/ext/standard/tests/poll/gh25086.phpt new file mode 100644 index 000000000000..8b6a9e864f0d --- /dev/null +++ b/ext/standard/tests/poll/gh25086.phpt @@ -0,0 +1,46 @@ +--TEST-- +GH-25086 (Io\Poll kqueue backend leaks memory on every Watcher::modifyEvents() call) +--FILE-- +wait(Time\Duration::fromSeconds(0)); + $watcher->modifyEvents($events_list[$i % count($events_list)]); + } + }; + + // Warm up to absorb one-time allocations + $modify(10); + $before = memory_get_usage(); + $modify(1000); + $growth = memory_get_usage() - $before; + echo "$name: ", $growth < 1024 ? "no growth" : "grew by $growth bytes", "\n"; + $watcher->remove(); +} + +use Io\Poll\Event; + +test_modify("read <-> read+write", [[Event::Read], [Event::Read, Event::Write]], true); +test_modify("read <-> write", [[Event::Read], [Event::Write]], true); +test_modify("read -> read one-shot -> read", [[Event::Read], [Event::Read, Event::OneShot]], true); +test_modify("re-arm read one-shot", [[Event::Read, Event::OneShot]], true); +test_modify("re-arm read+write one-shot, both fire", [[Event::Read, Event::Write, Event::OneShot]], true); +test_modify("re-arm read+write one-shot, write fires", [[Event::Read, Event::Write, Event::OneShot]], false); +?> +--EXPECT-- +read <-> read+write: no growth +read <-> write: no growth +read -> read one-shot -> read: no growth +re-arm read one-shot: no growth +re-arm read+write one-shot, both fire: no growth +re-arm read+write one-shot, write fires: no growth diff --git a/ext/standard/tests/poll/poll_stream_modify_oneshot_flag.phpt b/ext/standard/tests/poll/poll_stream_modify_oneshot_flag.phpt new file mode 100644 index 000000000000..14a2553f5ab0 --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_modify_oneshot_flag.phpt @@ -0,0 +1,44 @@ +--TEST-- +Io\Poll: modifyEvents() switching OneShot on and off takes effect on the kept filter +--FILE-- +wait(Time\Duration::fromSeconds(0))), " ", + count($ctx->wait(Time\Duration::fromSeconds(0))), "\n"; + +// Read stays registered, only the one-shot flag changes +$watcher->modifyEvents([Event::Read, Event::OneShot]); +echo "One-shot: ", count($ctx->wait(Time\Duration::fromSeconds(0))), " ", + count($ctx->wait(Time\Duration::fromSeconds(0))), "\n"; +var_dump($watcher->isActive()); + +// Back to level-triggered, which re-arms the watcher +$watcher->modifyEvents([Event::Read]); +echo "Level: ", count($ctx->wait(Time\Duration::fromSeconds(0))), " ", + count($ctx->wait(Time\Duration::fromSeconds(0))), "\n"; + +// Add write to a one-shot read, only one event is reported for the watcher +$watcher->modifyEvents([Event::Read, Event::OneShot]); +$watcher->modifyEvents([Event::Read, Event::Write, Event::OneShot]); +$events = $ctx->wait(Time\Duration::fromSeconds(0)); +echo "Read+write one-shot: ", count($events), " ", + count($ctx->wait(Time\Duration::fromSeconds(0))), "\n"; +var_dump($events[0] === $watcher); + +$watcher->remove(); +?> +--EXPECT-- +Level: 1 1 +One-shot: 1 0 +bool(true) +Level: 1 1 +Read+write one-shot: 1 0 +bool(true) diff --git a/main/poll/poll_backend_kqueue.c b/main/poll/poll_backend_kqueue.c index f2c0212f4d15..65e825dbf858 100644 --- a/main/poll/poll_backend_kqueue.c +++ b/main/poll/poll_backend_kqueue.c @@ -167,7 +167,6 @@ static zend_result kqueue_backend_modify(php_poll_ctx *ctx, int fd, uint32_t eve struct kevent deletes[2]; struct kevent adds[2]; - int delete_count = 0; int add_count = 0; int successful_deletes = 0; @@ -179,15 +178,9 @@ static zend_result kqueue_backend_modify(php_poll_ctx *ctx, int fd, uint32_t eve add_flags |= EV_CLEAR; } - /* Delete existing filters that are not in the new events */ - if (!(events & PHP_POLL_READ)) { - EV_SET(&deletes[delete_count], fd, EVFILT_READ, EV_DELETE, 0, 0, NULL); - delete_count++; - } - if (!(events & PHP_POLL_WRITE)) { - EV_SET(&deletes[delete_count], fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL); - delete_count++; - } + /* Delete all existing filters so the counters below know which ones were registered */ + EV_SET(&deletes[0], fd, EVFILT_READ, EV_DELETE, 0, 0, NULL); + EV_SET(&deletes[1], fd, EVFILT_WRITE, EV_DELETE, 0, 0, NULL); /* Prepare add operations for requested events */ if (events & PHP_POLL_READ) { @@ -200,7 +193,7 @@ static zend_result kqueue_backend_modify(php_poll_ctx *ctx, int fd, uint32_t eve } /* Delete existing filters individually to count successes */ - for (int i = 0; i < delete_count; i++) { + for (int i = 0; i < 2; i++) { int result = kevent(backend_data->kqueue_fd, &deletes[i], 1, NULL, 0, NULL); if (result == 0) { successful_deletes++; @@ -347,6 +340,11 @@ static int kqueue_backend_wait( if (backend_data->events[i].flags & EV_ERROR) { events[i].revents |= PHP_POLL_ERROR; } + + /* Oneshot filters are deleted by the kernel once retrieved */ + if (backend_data->events[i].flags & EV_ONESHOT) { + backend_data->filter_count--; + } } return nfds; @@ -385,6 +383,10 @@ static int kqueue_backend_wait( void *data = backend_data->events[i].udata; bool is_oneshot = (backend_data->events[i].flags & EV_ONESHOT) != 0; + if (is_oneshot) { + backend_data->filter_count--; + } + /* Convert this event */ if (backend_data->events[i].filter == EVFILT_READ) { revents |= PHP_POLL_READ; @@ -466,7 +468,9 @@ static int kqueue_backend_wait( if (flags & KQUEUE_FD_HAS_GARBAGE) { int filter = (flags & KQUEUE_FD_GARBAGE_READ) ? EVFILT_WRITE : EVFILT_READ; EV_SET(&cleanup_change, fd_key, filter, EV_DELETE, 0, 0, NULL); - kevent(backend_data->kqueue_fd, &cleanup_change, 1, NULL, 0, NULL); + if (kevent(backend_data->kqueue_fd, &cleanup_change, 1, NULL, 0, NULL) == 0) { + backend_data->filter_count--; + } /* Remove FD from tracking after cleanup */ zend_hash_index_del(backend_data->fd_tracking, fd_key);