diff options
author | Luca Boccassi <bluca@debian.org> | 2023-04-29 12:35:24 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-29 12:35:24 +0100 |
commit | 708d63c0e75b01d26edf4e4c85bd047bca410d44 (patch) | |
tree | ad8501a0d1249ca444f2562f964363b597cc60fa | |
parent | 8c59feed5edd5bfde6c79093dcdd7ef91bbc9d7c (diff) | |
parent | 843cb225ad88c6d90c8992b0a6ed895bb38e4b77 (diff) | |
download | systemd-708d63c0e75b01d26edf4e4c85bd047bca410d44.tar.gz |
Merge pull request #27451 from yuwata/core-path-trigger-notify
core/path: do not install new job in .trigger_notify()
-rw-r--r-- | src/core/path.c | 72 | ||||
-rw-r--r-- | src/core/path.h | 2 | ||||
-rw-r--r-- | test/testsuite-63.units/test63-issue-24577-dep.service | 4 | ||||
-rw-r--r-- | test/testsuite-63.units/test63-issue-24577.path | 3 | ||||
-rw-r--r-- | test/testsuite-63.units/test63-issue-24577.service | 8 | ||||
-rwxr-xr-x | test/units/testsuite-33.sh | 8 | ||||
-rwxr-xr-x | test/units/testsuite-63.sh | 39 |
7 files changed, 125 insertions, 11 deletions
diff --git a/src/core/path.c b/src/core/path.c index 3881c77a86..c95663c3aa 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 "glob-util.h" #include "inotify-util.h" @@ -26,10 +27,10 @@ #include "unit.h" static const UnitActiveState state_translation_table[_PATH_STATE_MAX] = { - [PATH_DEAD] = UNIT_INACTIVE, + [PATH_DEAD] = UNIT_INACTIVE, [PATH_WAITING] = UNIT_ACTIVE, [PATH_RUNNING] = UNIT_ACTIVE, - [PATH_FAILED] = UNIT_FAILED, + [PATH_FAILED] = UNIT_FAILED, }; static int path_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); @@ -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,28 @@ 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 = ASSERT_PTR(userdata); + Unit *trigger; + + 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; }; struct ActivationDetailsPath { diff --git a/test/testsuite-63.units/test63-issue-24577-dep.service b/test/testsuite-63.units/test63-issue-24577-dep.service new file mode 100644 index 0000000000..e332ea474e --- /dev/null +++ b/test/testsuite-63.units/test63-issue-24577-dep.service @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Service] +Type=oneshot +ExecStart=bash -c 'sleep infinity' diff --git a/test/testsuite-63.units/test63-issue-24577.path b/test/testsuite-63.units/test63-issue-24577.path new file mode 100644 index 0000000000..80ba1dbe16 --- /dev/null +++ b/test/testsuite-63.units/test63-issue-24577.path @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Path] +PathExists=/tmp/hoge diff --git a/test/testsuite-63.units/test63-issue-24577.service b/test/testsuite-63.units/test63-issue-24577.service new file mode 100644 index 0000000000..568518bbf6 --- /dev/null +++ b/test/testsuite-63.units/test63-issue-24577.service @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Requires=test63-issue-24577-dep.service +After=test63-issue-24577-dep.service + +[Service] +Type=oneshot +ExecStart=bash -c 'sleep infinity' diff --git a/test/units/testsuite-33.sh b/test/units/testsuite-33.sh index c9bd66e268..d48bef73b4 100755 --- a/test/units/testsuite-33.sh +++ b/test/units/testsuite-33.sh @@ -5,7 +5,7 @@ set -eux set -o pipefail -cat >/etc/systemd/system/test-service.service <<EOF +cat >/run/systemd/system/test-service.service <<EOF [Service] ConfigurationDirectory=test-service RuntimeDirectory=test-service @@ -75,7 +75,7 @@ test ! -e /var/lib/test-service test ! -e /var/cache/test-service test ! -e /var/log/test-service -cat >/etc/systemd/system/test-service.service <<EOF +cat >/run/systemd/system/test-service.service <<EOF [Service] DynamicUser=yes ConfigurationDirectory=test-service @@ -170,7 +170,7 @@ test ! -L /var/lib/test-service test ! -L /var/cache/test-service test ! -L /var/log/test-service -cat >/etc/systemd/system/tmp-hoge.mount <<EOF +cat >/run/systemd/system/tmp-hoge.mount <<EOF [Mount] What=tmpfs Type=tmpfs @@ -245,7 +245,7 @@ test ! -d /var/lib/hoge test ! -d /var/cache/hoge test ! -d /var/log/hoge -cat >/etc/systemd/system/test-service.socket <<EOF +cat >/run/systemd/system/test-service.socket <<EOF [Socket] ListenSequentialPacket=/run/test-service.socket RemoveOnStop=yes diff --git a/test/units/testsuite-63.sh b/test/units/testsuite-63.sh index 7ee7fc1513..591e6d3104 100755 --- a/test/units/testsuite-63.sh +++ b/test/units/testsuite-63.sh @@ -3,6 +3,9 @@ set -ex set -o pipefail +# shellcheck source=test/units/assert.sh +. "$(dirname "$0")"/assert.sh + systemctl log-level debug # Test that a path unit continuously triggering a service that fails condition checks eventually fails with @@ -41,6 +44,42 @@ systemctl stop test63-glob.path test63-glob.service test "$(busctl --json=short get-property org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/test63_2dglob_2eservice org.freedesktop.systemd1.Unit ActivationDetails)" = '{"type":"a(ss)","data":[]}' +# tests for issue https://github.com/systemd/systemd/issues/24577#issuecomment-1522628906 +rm -f /tmp/hoge +systemctl start test63-issue-24577.path +systemctl status -n 0 test63-issue-24577.path +systemctl status -n 0 test63-issue-24577.service || : +systemctl list-jobs +output=$(systemctl list-jobs --no-legend) +assert_not_in "test63-issue-24577.service" "$output" +assert_not_in "test63-issue-24577-dep.service" "$output" + +touch /tmp/hoge +systemctl status -n 0 test63-issue-24577.path +systemctl status -n 0 test63-issue-24577.service || : +systemctl list-jobs +output=$(systemctl list-jobs --no-legend) +assert_in "test63-issue-24577.service" "$output" +assert_in "test63-issue-24577-dep.service" "$output" + +# even if the service is stopped, it will be soon retriggered. +systemctl stop test63-issue-24577.service +systemctl status -n 0 test63-issue-24577.path +systemctl status -n 0 test63-issue-24577.service || : +systemctl list-jobs +output=$(systemctl list-jobs --no-legend) +assert_in "test63-issue-24577.service" "$output" +assert_in "test63-issue-24577-dep.service" "$output" + +rm -f /tmp/hoge +systemctl stop test63-issue-24577.service +systemctl status -n 0 test63-issue-24577.path +systemctl status -n 0 test63-issue-24577.service || : +systemctl list-jobs +output=$(systemctl list-jobs --no-legend) +assert_not_in "test63-issue-24577.service" "$output" +assert_in "test63-issue-24577-dep.service" "$output" + systemctl log-level info echo OK >/testok |