From c0b87106ce2590b73d71fac2bb1b110a43001691 Mon Sep 17 00:00:00 2001 From: lazerg Date: Sun, 1 Nov 2026 21:05:10 +0100 Subject: [PATCH] Fix GH-25086: Io\Poll kqueue backend leaks memory on every modifyEvents() kqueue_backend_modify() only deleted the filters that were not in the new event set, so when no delete succeeded it assumed the FD had no filters before and added the full new set to filter_count. Going from Read to Read+Write therefore counted the already registered read filter again. get_suitable_max_events() returns filter_count, so every wait() grew the struct kevent buffer by one entry per modify. It now deletes both filters first, which gives the real number of registered filters, and then adds the requested ones. As a side effect, switching an existing watcher to one-shot or edge-triggered now takes effect, since EV_ADD on an existing filter kept its old EV_ONESHOT/EV_CLEAR flags. Re-arming a fired one-shot watcher grew the buffer the same way, because wait() never took the filters the kernel dropped out of filter_count. It now does, including the leftover filter it deletes for read+write one-shot watchers. Closes GH-25087. Co-authored-by: Jakub Zelenka --- NEWS | 2 + ext/standard/tests/poll/gh25086.phpt | 46 +++++++++++++++++++ .../poll/poll_stream_modify_oneshot_flag.phpt | 44 ++++++++++++++++++ main/poll/poll_backend_kqueue.c | 28 ++++++----- 4 files changed, 108 insertions(+), 12 deletions(-) create mode 100644 ext/standard/tests/poll/gh25086.phpt create mode 100644 ext/standard/tests/poll/poll_stream_modify_oneshot_flag.phpt 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);