systemd/backport-core-path-do-not-enqueue-new-job-in-.trigger_notify-.patch
2023-12-18 16:49:22 +08:00

156 lines
5.9 KiB
Diff

From 2d2b66b0bec607ce246a55a8c77805cea86ead4c Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Sat, 29 Apr 2023 04:31:53 +0900
Subject: [PATCH] core/path: do not enqueue new job in .trigger_notify callback
Otherwise,
1. X.path triggered X.service, and the service has waiting start job,
2. systemctl stop X.service
3. the waiting start job is cancelled to install new stop job,
4. path_trigger_notify() is called, and may reinstall new start job,
5. the stop job cannot be installed, and triggeres assertion.
So, instead, let's add a defer event source, then enqueue the new start
job after the stop (or any other type) job finished.
Fixes https://github.com/systemd/systemd/issues/24577#issuecomment-1522628906.
(cherry picked from commit bc6377762c210d1bdd7fd2465930731d87dda576)
(cherry picked from commit 03f2a8921ee0671710f920896c0234b9793c07c5)
Conflict:code context adaptation and ASSERT_PTR function adaptation
Reference:https://github.com/systemd/systemd-stable/commit/2d2b66b0bec607ce246a55a8c77805cea86ead4c
---
src/core/path.c | 68 +++++++++++++++++++++++++++++++++++++++++++++----
src/core/path.h | 2 ++
2 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/src/core/path.c b/src/core/path.c
index a8b2b6ae8f..a8144c344d 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -10,6 +10,7 @@
#include "dbus-path.h"
#include "dbus-unit.h"
#include "escape.h"
+#include "event-util.h"
#include "fd-util.h"
#include "fs-util.h"
#include "glob-util.h"
@@ -300,6 +301,7 @@ static void path_done(Unit *u) {
assert(p);
+ p->trigger_notify_event_source = sd_event_source_disable_unref(p->trigger_notify_event_source);
path_free_specs(p);
}
@@ -575,6 +577,9 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify)
Unit *trigger;
int r;
+ if (p->trigger_notify_event_source)
+ (void) event_source_disable(p->trigger_notify_event_source);
+
/* If the triggered unit is already running, so are we */
trigger = UNIT_TRIGGER(UNIT(p));
if (trigger && !UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(trigger))) {
@@ -799,8 +804,29 @@ fail:
return 0;
}
-static void path_trigger_notify(Unit *u, Unit *other) {
+static void path_trigger_notify_impl(Unit *u, Unit *other, bool on_defer);
+
+static int path_trigger_notify_on_defer(sd_event_source *s, void *userdata) {
+ Path *p = userdata;
+ Unit *trigger;
+
+ assert(p);
+ assert(s);
+
+ trigger = UNIT_TRIGGER(UNIT(p));
+ if (!trigger) {
+ log_unit_error(UNIT(p), "Unit to trigger vanished.");
+ path_enter_dead(p, PATH_FAILURE_RESOURCES);
+ return 0;
+ }
+
+ path_trigger_notify_impl(UNIT(p), trigger, /* on_defer = */ true);
+ return 0;
+}
+
+static void path_trigger_notify_impl(Unit *u, Unit *other, bool on_defer) {
Path *p = PATH(u);
+ int r;
assert(u);
assert(other);
@@ -826,13 +851,46 @@ static void path_trigger_notify(Unit *u, Unit *other) {
if (p->state == PATH_RUNNING &&
UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
- log_unit_debug(UNIT(p), "Got notified about unit deactivation.");
- path_enter_waiting(p, false, true);
+ if (!on_defer)
+ log_unit_debug(u, "Got notified about unit deactivation.");
} else if (p->state == PATH_WAITING &&
!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
- log_unit_debug(UNIT(p), "Got notified about unit activation.");
- path_enter_waiting(p, false, true);
+ if (!on_defer)
+ log_unit_debug(u, "Got notified about unit activation.");
+ } else
+ return;
+
+ if (on_defer) {
+ path_enter_waiting(p, /* initial = */ false, /* from_trigger_notify = */ true);
+ return;
}
+
+ /* Do not call path_enter_waiting() directly from path_trigger_notify(), as this may be called by
+ * job_install() -> job_finish_and_invalidate() -> unit_trigger_notify(), and path_enter_waiting()
+ * may install another job and will trigger assertion in job_install().
+ * https://github.com/systemd/systemd/issues/24577#issuecomment-1522628906
+ * Hence, first setup defer event source here, and call path_enter_waiting() slightly later. */
+ if (p->trigger_notify_event_source) {
+ r = sd_event_source_set_enabled(p->trigger_notify_event_source, SD_EVENT_ONESHOT);
+ if (r < 0) {
+ log_unit_warning_errno(u, r, "Failed to enable event source for triggering notify: %m");
+ path_enter_dead(p, PATH_FAILURE_RESOURCES);
+ return;
+ }
+ } else {
+ r = sd_event_add_defer(u->manager->event, &p->trigger_notify_event_source, path_trigger_notify_on_defer, p);
+ if (r < 0) {
+ log_unit_warning_errno(u, r, "Failed to allocate event source for triggering notify: %m");
+ path_enter_dead(p, PATH_FAILURE_RESOURCES);
+ return;
+ }
+
+ (void) sd_event_source_set_description(p->trigger_notify_event_source, "path-trigger-notify");
+ }
+}
+
+static void path_trigger_notify(Unit *u, Unit *other) {
+ path_trigger_notify_impl(u, other, /* on_defer = */ false);
}
static void path_reset_failed(Unit *u) {
diff --git a/src/core/path.h b/src/core/path.h
index c76103cc12..cb5b662911 100644
--- a/src/core/path.h
+++ b/src/core/path.h
@@ -65,6 +65,8 @@ struct Path {
PathResult result;
RateLimit trigger_limit;
+
+ sd_event_source *trigger_notify_event_source;
};
void path_free_specs(Path *p);
--
2.33.0