sd-event: Make sure iterations of defer and exit sources are updated

Defer and exit event sources are marked pending once when they are added
and never again afterwards. This means their pending_iteration is never
incremented after they are initially added, which breaks fairness among
event sources with equal priority which depend on the pending_iteration
variable getting updated in source_set_pending(). To fix this, let's assign
iterations for defer and exit sources in source_dispatch() instead so that
those get their pending_iteration updated as well.

diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 42b5ba1f7eb9ee95aa0535db561a8e316425c8c2..94b178eafd9d0a16548a9a1632aaf519366646a9 100644 (file)
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -4120,7 +4120,13 @@ static int source_dispatch(sd_event_source *s) {
                 return 1;
         }
 
-        if (!IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
+        if (IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) {
+                /* Make sure this event source is moved to the end of the priority list now. We do this here
+                 * because defer and exit event sources are always pending from the moment they're added so
+                 * the same logic in source_set_pending() is never triggered. */
+                s->pending_iteration = s->event->iteration;
+                event_source_pp_prioq_reshuffle(s);
+        } else {
                 r = source_set_pending(s, false);
                 if (r < 0)
                         return r;

diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c
index e85fa2e4314a114667153bf7d390afa3a317aa7a..eb986c9e082421e2660aa8b391afaed9ccfa727e 100644 (file)
--- a/src/libsystemd/sd-event/test-event.c
+++ b/src/libsystemd/sd-event/test-event.c
@@ -1131,4 +1131,40 @@ TEST(exit_on_idle_no_sources) {
         ASSERT_OK(sd_event_loop(e));
 }
 
+static int defer_fair_handler(sd_event_source *s, void *userdata) {
+        unsigned *counter = ASSERT_PTR(userdata);
+
+        /* If we're about to increment above 5, exit the event loop */
+        if (*counter >= 5)
+                return sd_event_exit(sd_event_source_get_event(s), 0);
+
+        (*counter)++;
+
+        return 0;
+}
+
+TEST(defer_fair_scheduling) {
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        sd_event_source *sources[5] = {};
+        unsigned counters[5] = {};
+
+        ASSERT_OK(sd_event_new(&e));
+        ASSERT_OK(sd_event_set_exit_on_idle(e, true));
+
+        /* Create 5 defer sources with equal priority */
+        for (unsigned i = 0; i < 5; i++) {
+                ASSERT_OK(sd_event_add_defer(e, &sources[i], defer_fair_handler, &counters[i]));
+                ASSERT_OK(sd_event_source_set_enabled(sources[i], SD_EVENT_ON));
+        }
+
+        /* Run the event loop until one of the handlers exits */
+        ASSERT_OK(sd_event_loop(e));
+
+        /* All counters should be equal to 5, demonstrating fair scheduling */
+        for (unsigned i = 0; i < 5; i++) {
+                ASSERT_EQ(counters[i], 5u);
+                sd_event_source_unref(sources[i]);
+        }
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);