diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index ebab9ed8b3..1973e13b5b 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -165,6 +165,7 @@ struct sd_event { bool watchdog:1; bool profile_delays:1; bool exit_on_idle:1; + bool ratelimit_expired:1; int exit_code; @@ -4559,6 +4560,8 @@ _public_ int sd_event_prepare(sd_event *e) { /* Make sure that none of the preparation callbacks ends up freeing the event source under our feet */ PROTECT_EVENT(e); + e->ratelimit_expired = false; + if (!e->exit_requested && e->exit_on_idle && event_loop_idle(e)) (void) sd_event_exit(e, 0); @@ -4775,7 +4778,6 @@ static int process_epoll(sd_event *e, usec_t timeout, int64_t threshold, int64_t } _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { - bool ratelimit_expired = false; int r; assert_return(e, -EINVAL); @@ -4841,7 +4843,7 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { if (r < 0) goto finish; else if (r == 1) - ratelimit_expired = true; + e->ratelimit_expired = true; r = process_timer(e, e->timestamp.realtime, &e->realtime_alarm); if (r < 0) @@ -4855,10 +4857,11 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { if (r < 0) goto finish; - if (ratelimit_expired) { - /* Ratelimit expiry callback was called. Let's postpone processing pending sources and put - * the loop in the initial state in order to evaluate (in the next iteration) also sources - * that were potentially re-enabled by the callback. + if (e->ratelimit_expired) { + /* Ratelimit expiry callback was called. Let's postpone processing pending sources in order + * to evaluate (in the next iteration) also sources that were potentially re-enabled by the + * callback. Report this as pending rather than returning 0, as the caller would block + * otherwise. sd_event_dispatch() then returns right away. * * Wondering why we treat only the CLOCK_BOOTTIME invocation of process_timer() differently? * Once event source is ratelimited we essentially transform it into a CLOCK_BOOTTIME timer @@ -4867,8 +4870,8 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) { * Note that we don't short-circuit earlier because all timer prioqs must be fully processed * as their timerfds may have already been flushed by process_epoll(), so skipping them would * leave those sources unprocessed and the timerfds un-rearmed. */ - r = 0; - goto finish; + e->state = SD_EVENT_PENDING; + return 1; } if (event_next_pending(e)) { @@ -4894,6 +4897,12 @@ _public_ int sd_event_dispatch(sd_event *e) { assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(e->state == SD_EVENT_PENDING, -EBUSY); + if (e->ratelimit_expired) { + /* A ratelimit expiry callback was called in this iteration, don't dispatch anything else. */ + e->state = SD_EVENT_INITIAL; + return 1; + } + if (e->exit_requested) return dispatch_exit(e); diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c index 8070f3189c..e0bfad50c0 100644 --- a/src/libsystemd/sd-event/test-event.c +++ b/src/libsystemd/sd-event/test-event.c @@ -9,6 +9,7 @@ #include "event-util.h" #include "fd-util.h" #include "fs-util.h" +#include "io-util.h" #include "log.h" #include "parse-util.h" #include "path-util.h" @@ -984,6 +985,67 @@ TEST(defer_add_post) { ASSERT_TRUE(dispatched_post); } +static unsigned expire_drain_count = 0; +static int expire_drain_callback(sd_event_source *s, void *userdata) { + expire_drain_count++; + + /* Consume the readiness, so that leaving the ratelimit doesn't generate a new epoll event */ + ASSERT_OK(flush_fd(sd_event_source_get_io_fd(s))); + + return 0; +} + +TEST(ratelimit_expire_with_pending) { + _cleanup_close_pair_ int p[2] = EBADF_PAIR; + _cleanup_(sd_event_unrefp) sd_event *e = NULL; + _cleanup_(sd_event_source_unrefp) sd_event_source *io = NULL, *defer = NULL; + unsigned count = 0; + bool dispatched = false; + + ASSERT_OK(sd_event_default(&e)); + ASSERT_OK_ERRNO(pipe2(p, O_CLOEXEC|O_NONBLOCK)); + + ASSERT_OK(sd_event_add_io(e, &io, p[0], EPOLLIN, ratelimit_io_handler, &count)); + ASSERT_OK(sd_event_source_set_ratelimit(io, 1 * USEC_PER_SEC, 1)); + ASSERT_OK(sd_event_source_set_ratelimit_expire_callback(io, expire_drain_callback)); + + ASSERT_OK_EQ_ERRNO(write(p[1], "x", 1), 1); + + /* The first dispatch opens the ratelimit window, the second one hits the burst limit. */ + ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_OK_POSITIVE(sd_event_source_is_ratelimited(io)); + + /* Arm the CLOCK_BOOTTIME timer for the end of the ratelimit window, and let it elapse. */ + ASSERT_OK_ZERO(sd_event_run(e, 0)); + ASSERT_OK(usleep_safe(1100 * USEC_PER_MSEC)); + + /* Now have an event source pending only in userspace, which never makes the epoll fd readable. */ + ASSERT_OK(sd_event_add_defer(e, &defer, defer_post_handler, &dispatched)); + + /* sd_event_prepare() will see the pending defer source, and its sd_event_wait() call will run the + * ratelimit expiry callback. It must not report "nothing pending" afterwards, as otherwise callers + * like sd_event_run() would block indefinitely in epoll_wait(). */ + ASSERT_OK_POSITIVE(sd_event_prepare(e)); + ASSERT_EQ(expire_drain_count, 1U); + ASSERT_EQ(count, 1U); + ASSERT_OK_ZERO(sd_event_source_is_ratelimited(io)); + + /* No other callback is dispatched in the same iteration as the ratelimit expiry callback. */ + ASSERT_OK_POSITIVE(sd_event_dispatch(e)); + ASSERT_EQ(count, 1U); + ASSERT_FALSE(dispatched); + + /* The IO event that hit the ratelimit stayed pending and is delivered first, then the defer source. */ + ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_EQ(count, 2U); + ASSERT_FALSE(dispatched); + + ASSERT_OK_POSITIVE(sd_event_run(e, 0)); + ASSERT_TRUE(dispatched); + ASSERT_EQ(expire_drain_count, 1U); +} + static int child_handler_wnowait(sd_event_source *s, const siginfo_t *si, void *userdata) { int *counter = ASSERT_PTR(userdata);