diff --git a/.release-notes/5348.md b/.release-notes/5348.md new file mode 100644 index 0000000000..88ade24fc2 --- /dev/null +++ b/.release-notes/5348.md @@ -0,0 +1,7 @@ +## Fix epoll backend losing signal counts on eventfd read failure + +The epoll ASIO backend could deliver an uninitialized missed-signal count +to signal handlers when `read()` on the signal eventfd returned an error. +On read failure, the backend now waits for the next epoll iteration instead +of waking signal handlers with a misleading zero count, so handlers only run +with a real signal count. diff --git a/packages/signals/_test.pony b/packages/signals/_test.pony index 019f4fb09b..c5d0dd6bbd 100644 --- a/packages/signals/_test.pony +++ b/packages/signals/_test.pony @@ -14,6 +14,7 @@ class \nodoc\ _TestSighupNotify is SignalNotify _h = h fun ref apply(count: U32): Bool => + _h.assert_true(count >= 1) _h.complete(true) false diff --git a/src/libponyrt/asio/epoll.c b/src/libponyrt/asio/epoll.c index 1222750dae..55c936a701 100644 --- a/src/libponyrt/asio/epoll.c +++ b/src/libponyrt/asio/epoll.c @@ -73,6 +73,23 @@ static void signal_handler(int sig) eventfd_write(ev->fd, 1); } +static bool read_signal_count(int fd, uint32_t* out_count) +{ + uint64_t missed = 0; + ssize_t rc = read(fd, &missed, sizeof(uint64_t)); + + if(rc != sizeof(uint64_t)) + return false; + + *out_count = (uint32_t)missed; + return true; +} + +bool ponyint_asio_epoll_read_signal_count(int fd, uint32_t* out_count) +{ + return read_signal_count(fd, out_count); +} + #if !defined(USE_SCHEDULER_SCALING_PTHREADS) static void empty_signal_handler(int sig) { @@ -288,9 +305,13 @@ DECLARE_THREAD_FN(ponyint_asio_backend_dispatch) { if(ep->events & (EPOLLIN | EPOLLRDHUP | EPOLLHUP | EPOLLERR)) { - uint64_t missed; + uint64_t missed = 0; ssize_t rc = read(ev->fd, &missed, sizeof(uint64_t)); + /* missed is intentionally unused for timer events: the ASIO timer + * backend does not coalesce based on missed count today. + */ (void)rc; + (void)missed; flags |= ASIO_TIMER; } } @@ -299,11 +320,8 @@ DECLARE_THREAD_FN(ponyint_asio_backend_dispatch) { if(ep->events & (EPOLLIN | EPOLLRDHUP | EPOLLHUP | EPOLLERR)) { - uint64_t missed; - ssize_t rc = read(ev->fd, &missed, sizeof(uint64_t)); - (void)rc; - flags |= ASIO_SIGNAL; - count = (uint32_t)missed; + if(read_signal_count(ev->fd, &count)) + flags |= ASIO_SIGNAL; } } diff --git a/test/libponyrt/CMakeLists.txt b/test/libponyrt/CMakeLists.txt index 3bae3205f4..551988c82f 100644 --- a/test/libponyrt/CMakeLists.txt +++ b/test/libponyrt/CMakeLists.txt @@ -4,6 +4,7 @@ project(libponyrt.tests VERSION ${PONYC_PROJECT_VERSION} LANGUAGES C CXX) add_executable(libponyrt.tests util.cc + asio_epoll.cc ds/fun.cc ds/hash.cc ds/list.cc diff --git a/test/libponyrt/asio_epoll.cc b/test/libponyrt/asio_epoll.cc new file mode 100644 index 0000000000..ac7497b0d0 --- /dev/null +++ b/test/libponyrt/asio_epoll.cc @@ -0,0 +1,65 @@ +#include + +#include + +#include + +#if defined(PLATFORM_IS_LINUX) +#include +#include + +extern "C" { + bool ponyint_asio_epoll_read_signal_count(int fd, uint32_t* out_count); +} + +static void close_eventfd(int fd) +{ + int rc = close(fd); + ASSERT_EQ(0, rc); +} + +TEST(AsioEpoll, ReadSignalCountReturnsFalseForUnwrittenNonblockingEventfd) +{ + int fd = eventfd(0, EFD_NONBLOCK); + ASSERT_NE(-1, fd); + + uint32_t count = 123; + ASSERT_FALSE(ponyint_asio_epoll_read_signal_count(fd, &count)); + ASSERT_EQ((uint32_t)123, count); + + close_eventfd(fd); +} + +TEST(AsioEpoll, ReadSignalCountReturnsWrittenValue) +{ + int fd = eventfd(0, EFD_NONBLOCK); + ASSERT_NE(-1, fd); + + ASSERT_EQ(0, eventfd_write(fd, 7)); + + uint32_t count = 0; + ASSERT_TRUE(ponyint_asio_epoll_read_signal_count(fd, &count)); + ASSERT_EQ((uint32_t)7, count); + + close_eventfd(fd); +} + +TEST(AsioEpoll, ReadSignalCountAccumulatesWritesAndDrainsEventfd) +{ + int fd = eventfd(0, EFD_NONBLOCK); + ASSERT_NE(-1, fd); + + ASSERT_EQ(0, eventfd_write(fd, 3)); + ASSERT_EQ(0, eventfd_write(fd, 4)); + + uint32_t count = 0; + ASSERT_TRUE(ponyint_asio_epoll_read_signal_count(fd, &count)); + ASSERT_EQ((uint32_t)7, count); + + count = 123; + ASSERT_FALSE(ponyint_asio_epoll_read_signal_count(fd, &count)); + ASSERT_EQ((uint32_t)123, count); + + close_eventfd(fd); +} +#endif