Skip to content

Commit 2d2b66b

Browse files
yuwatabluca
authored andcommitted
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 systemd/systemd#24577 (comment). (cherry picked from commit bc63777) (cherry picked from commit 03f2a89)
1 parent 8fc380d commit 2d2b66b

File tree

2 files changed

+65
-5
lines changed

2 files changed

+65
-5
lines changed

src/core/path.c

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "dbus-path.h"
1111
#include "dbus-unit.h"
1212
#include "escape.h"
13+
#include "event-util.h"
1314
#include "fd-util.h"
1415
#include "glob-util.h"
1516
#include "inotify-util.h"
@@ -300,6 +301,7 @@ static void path_done(Unit *u) {
300301

301302
assert(p);
302303

304+
p->trigger_notify_event_source = sd_event_source_disable_unref(p->trigger_notify_event_source);
303305
path_free_specs(p);
304306
}
305307

@@ -575,6 +577,9 @@ static void path_enter_waiting(Path *p, bool initial, bool from_trigger_notify)
575577
Unit *trigger;
576578
int r;
577579

580+
if (p->trigger_notify_event_source)
581+
(void) event_source_disable(p->trigger_notify_event_source);
582+
578583
/* If the triggered unit is already running, so are we */
579584
trigger = UNIT_TRIGGER(UNIT(p));
580585
if (trigger && !UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(trigger))) {
@@ -799,8 +804,28 @@ static int path_dispatch_io(sd_event_source *source, int fd, uint32_t revents, v
799804
return 0;
800805
}
801806

802-
static void path_trigger_notify(Unit *u, Unit *other) {
807+
static void path_trigger_notify_impl(Unit *u, Unit *other, bool on_defer);
808+
809+
static int path_trigger_notify_on_defer(sd_event_source *s, void *userdata) {
810+
Path *p = ASSERT_PTR(userdata);
811+
Unit *trigger;
812+
813+
assert(s);
814+
815+
trigger = UNIT_TRIGGER(UNIT(p));
816+
if (!trigger) {
817+
log_unit_error(UNIT(p), "Unit to trigger vanished.");
818+
path_enter_dead(p, PATH_FAILURE_RESOURCES);
819+
return 0;
820+
}
821+
822+
path_trigger_notify_impl(UNIT(p), trigger, /* on_defer = */ true);
823+
return 0;
824+
}
825+
826+
static void path_trigger_notify_impl(Unit *u, Unit *other, bool on_defer) {
803827
Path *p = PATH(u);
828+
int r;
804829

805830
assert(u);
806831
assert(other);
@@ -826,13 +851,46 @@ static void path_trigger_notify(Unit *u, Unit *other) {
826851

827852
if (p->state == PATH_RUNNING &&
828853
UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
829-
log_unit_debug(UNIT(p), "Got notified about unit deactivation.");
830-
path_enter_waiting(p, false, true);
854+
if (!on_defer)
855+
log_unit_debug(u, "Got notified about unit deactivation.");
831856
} else if (p->state == PATH_WAITING &&
832857
!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) {
833-
log_unit_debug(UNIT(p), "Got notified about unit activation.");
834-
path_enter_waiting(p, false, true);
858+
if (!on_defer)
859+
log_unit_debug(u, "Got notified about unit activation.");
860+
} else
861+
return;
862+
863+
if (on_defer) {
864+
path_enter_waiting(p, /* initial = */ false, /* from_trigger_notify = */ true);
865+
return;
835866
}
867+
868+
/* Do not call path_enter_waiting() directly from path_trigger_notify(), as this may be called by
869+
* job_install() -> job_finish_and_invalidate() -> unit_trigger_notify(), and path_enter_waiting()
870+
* may install another job and will trigger assertion in job_install().
871+
* https://github.com/systemd/systemd/issues/24577#issuecomment-1522628906
872+
* Hence, first setup defer event source here, and call path_enter_waiting() slightly later. */
873+
if (p->trigger_notify_event_source) {
874+
r = sd_event_source_set_enabled(p->trigger_notify_event_source, SD_EVENT_ONESHOT);
875+
if (r < 0) {
876+
log_unit_warning_errno(u, r, "Failed to enable event source for triggering notify: %m");
877+
path_enter_dead(p, PATH_FAILURE_RESOURCES);
878+
return;
879+
}
880+
} else {
881+
r = sd_event_add_defer(u->manager->event, &p->trigger_notify_event_source, path_trigger_notify_on_defer, p);
882+
if (r < 0) {
883+
log_unit_warning_errno(u, r, "Failed to allocate event source for triggering notify: %m");
884+
path_enter_dead(p, PATH_FAILURE_RESOURCES);
885+
return;
886+
}
887+
888+
(void) sd_event_source_set_description(p->trigger_notify_event_source, "path-trigger-notify");
889+
}
890+
}
891+
892+
static void path_trigger_notify(Unit *u, Unit *other) {
893+
path_trigger_notify_impl(u, other, /* on_defer = */ false);
836894
}
837895

838896
static void path_reset_failed(Unit *u) {

src/core/path.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ struct Path {
6565
PathResult result;
6666

6767
RateLimit trigger_limit;
68+
69+
sd_event_source *trigger_notify_event_source;
6870
};
6971

7072
struct ActivationDetailsPath {

0 commit comments

Comments
 (0)