sd-event: fix hang after ratelimit expiry
If sources are already pending, sd_event_prepare() calls sd_event_wait(e, 0). Sincefd69f22475this returns 0 when a ratelimit expiry callback was invoked, so that sources re-enabled by the callback are evaluated in the next iteration. However, sd_event_prepare() then returned 0 ("nothing pending"), so sd_event_run() blocked in epoll_wait() forever, as the pending sources never make the epoll fd readable. PID 1 can hit this via the ratelimited mount event source, which hangs the boot, e.g. in the initrd after sysroot.mount. Fix this by remembering in the sd_event object that a ratelimit expiry callback was invoked in this iteration. In that case sd_event_wait() reports the loop as pending, and sd_event_dispatch() returns right away without dispatching anything, so that the caller goes directly for sd_event_prepare() again, and no other callback is dispatched in the same iteration. Add a test that reproduces the issue. Follow-up forfd69f22475. Fixes #44107 (cherry picked from commit 1a60b10de9e4a3fe59a2efa3c6a58fd72c305807)
This commit is contained in:
committed by
Luca Boccassi
parent
3a0ad99243
commit
bc171ac92e
@@ -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);
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user