blob: c796bd0ec54b82454b9cdcdf91ab12158e9b7332 [file] [log] [blame]
From 81107b8419c39f726fd2805517a5b9faab204e59 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 8 Jun 2021 00:07:51 -0700
Subject: [PATCH] sd-event: change ordering of pending/ratelimited events
Instead of ordering non-pending before pending we should order
"non-pending OR ratelimited" before "pending AND not-ratelimited".
This fixes a bug where ratelimited events were ordered at the end of the
priority queue and could be stuck there for an indeterminate amount of
time.
---
src/libsystemd/sd-event/sd-event.c | 45 ++++++++++++++----------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 611af45293ac..489878a3e967 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -238,25 +238,6 @@ static usec_t time_event_source_next(const sd_event_source *s) {
return USEC_INFINITY;
}
-static int earliest_time_prioq_compare(const void *a, const void *b) {
- const sd_event_source *x = a, *y = b;
-
- /* Enabled ones first */
- if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
- return -1;
- if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
- return 1;
-
- /* Move the pending ones to the end */
- if (!x->pending && y->pending)
- return -1;
- if (x->pending && !y->pending)
- return 1;
-
- /* Order by time */
- return CMP(time_event_source_next(x), time_event_source_next(y));
-}
-
static usec_t time_event_source_latest(const sd_event_source *s) {
assert(s);
@@ -275,7 +256,15 @@ static usec_t time_event_source_latest(const sd_event_source *s) {
return USEC_INFINITY;
}
-static int latest_time_prioq_compare(const void *a, const void *b) {
+static bool event_source_timer_candidate(const sd_event_source *s) {
+ assert(s);
+
+ /* Returns true for event sources that either are not pending yet (i.e. where it's worth to mark them pending)
+ * or which are currently ratelimited (i.e. where it's worth leaving the ratelimited state) */
+ return !s->pending || s->ratelimited;
+}
+
+static int time_prioq_compare(const void *a, const void *b, usec_t (*time_func)(const sd_event_source *s)) {
const sd_event_source *x = a, *y = b;
/* Enabled ones first */
@@ -284,14 +273,22 @@ static int latest_time_prioq_compare(const void *a, const void *b) {
if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
return 1;
- /* Move the pending ones to the end */
- if (!x->pending && y->pending)
+ /* Order "non-pending OR ratelimited" before "pending AND not-ratelimited" */
+ if (event_source_timer_candidate(x) && !event_source_timer_candidate(y))
return -1;
- if (x->pending && !y->pending)
+ if (!event_source_timer_candidate(x) && event_source_timer_candidate(y))
return 1;
/* Order by time */
- return CMP(time_event_source_latest(x), time_event_source_latest(y));
+ return CMP(time_func(x), time_func(y));
+}
+
+static int earliest_time_prioq_compare(const void *a, const void *b) {
+ return time_prioq_compare(a, b, time_event_source_next);
+}
+
+static int latest_time_prioq_compare(const void *a, const void *b) {
+ return time_prioq_compare(a, b, time_event_source_latest);
}
static int exit_prioq_compare(const void *a, const void *b) {
@@ -778,14 +775,15 @@ static void event_source_time_prioq_reshuffle(sd_event_source *s) {
assert(s);
/* Called whenever the event source's timer ordering properties changed, i.e. time, accuracy,
- * pending, enable state. Makes sure the two prioq's are ordered properly again. */
+ * pending, enable state, and ratelimiting state. Makes sure the two prioq's are ordered
+ * properly again. */
if (s->ratelimited)
d = &s->event->monotonic;
- else {
- assert(EVENT_SOURCE_IS_TIME(s->type));
+ else if (EVENT_SOURCE_IS_TIME(s->type))
assert_se(d = event_get_clock_data(s->event, s->type));
- }
+ else
+ return; /* no-op for an event source which is neither a timer nor ratelimited. */
prioq_reshuffle(d->earliest, s, &s->earliest_index);
prioq_reshuffle(d->latest, s, &s->latest_index);
@@ -2376,14 +2374,6 @@ static int event_source_offline(
source_io_unregister(s);
break;
- case SOURCE_TIME_REALTIME:
- case SOURCE_TIME_BOOTTIME:
- case SOURCE_TIME_MONOTONIC:
- case SOURCE_TIME_REALTIME_ALARM:
- case SOURCE_TIME_BOOTTIME_ALARM:
- event_source_time_prioq_reshuffle(s);
- break;
-
case SOURCE_SIGNAL:
event_gc_signal_data(s->event, &s->priority, s->signal.sig);
break;
@@ -2404,6 +2394,11 @@ static int event_source_offline(
prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
break;
+ case SOURCE_TIME_REALTIME:
+ case SOURCE_TIME_BOOTTIME:
+ case SOURCE_TIME_MONOTONIC:
+ case SOURCE_TIME_REALTIME_ALARM:
+ case SOURCE_TIME_BOOTTIME_ALARM:
case SOURCE_DEFER:
case SOURCE_POST:
case SOURCE_INOTIFY:
@@ -2413,6 +2408,9 @@ static int event_source_offline(
assert_not_reached("Wut? I shouldn't exist.");
}
+ /* Always reshuffle time prioq, as the ratelimited flag may be changed. */
+ event_source_time_prioq_reshuffle(s);
+
return 1;
}
@@ -2502,22 +2500,11 @@ static int event_source_online(
s->ratelimited = ratelimited;
/* Non-failing operations below */
- switch (s->type) {
- case SOURCE_TIME_REALTIME:
- case SOURCE_TIME_BOOTTIME:
- case SOURCE_TIME_MONOTONIC:
- case SOURCE_TIME_REALTIME_ALARM:
- case SOURCE_TIME_BOOTTIME_ALARM:
- event_source_time_prioq_reshuffle(s);
- break;
-
- case SOURCE_EXIT:
+ if (s->type == SOURCE_EXIT)
prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
- break;
- default:
- break;
- }
+ /* Always reshuffle time prioq, as the ratelimited flag may be changed. */
+ event_source_time_prioq_reshuffle(s);
return 1;
}