From 390097724b36cb849ffc89caa223dc60a3762e35 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Jul 2025 18:19:05 +0900 Subject: [PATCH 1/4] journalctl: do not fail on SIGTERM/SIGINT, or when STDOUT is disconnected The current behavior is not useful when e.g. pipefail is enabled. Let's exit cleanly in such cases. Closes #38114. --- src/journal/journalctl-show.c | 16 ++++++---------- src/journal/journalctl.c | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/journal/journalctl-show.c b/src/journal/journalctl-show.c index 10af3c7b36..b5e149d7de 100644 --- a/src/journal/journalctl-show.c +++ b/src/journal/journalctl-show.c @@ -371,7 +371,7 @@ static int on_synchronize_reply( static int on_signal(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { _cleanup_(sd_varlink_flush_close_unrefp) sd_varlink *vl = NULL; Context *c = ASSERT_PTR(userdata); - int r; + int r = 0; assert(s); assert(si); @@ -381,7 +381,7 @@ static int on_signal(sd_event_source *s, const struct signalfd_siginfo *si, void goto finish; if (c->synchronize_varlink) /* Already pending? Then exit immediately, so that user can cancel the sync */ - return sd_event_exit(c->event, EXIT_SUCCESS); + goto finish; r = varlink_connect_journal(&vl); if (r < 0) { @@ -417,7 +417,7 @@ static int on_signal(sd_event_source *s, const struct signalfd_siginfo *si, void return 0; finish: - return sd_event_exit(c->event, si->ssi_signo); + return sd_event_exit(c->event, r); } static int setup_event(Context *c, int fd) { @@ -436,12 +436,12 @@ static int setup_event(Context *c, int fd) { (void) sd_event_add_signal(e, /* ret= */ NULL, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_signal, c); (void) sd_event_add_signal(e, /* ret= */ NULL, SIGINT | SD_EVENT_SIGNAL_PROCMASK, on_signal, c); - r = sd_event_add_io(e, NULL, fd, EPOLLIN, &on_journal_event, c); + r = sd_event_add_io(e, /* ret = */ NULL, fd, EPOLLIN, &on_journal_event, c); if (r < 0) return log_error_errno(r, "Failed to add io event source for journal: %m"); /* Also keeps an eye on STDOUT, and exits as soon as we see a POLLHUP on that, i.e. when it is closed. */ - r = sd_event_add_io(e, NULL, STDOUT_FILENO, EPOLLHUP|EPOLLERR, NULL, INT_TO_PTR(-ECANCELED)); + r = sd_event_add_io(e, /* ret = */ NULL, STDOUT_FILENO, EPOLLHUP|EPOLLERR, /* callback = */ NULL, /* userdata = */ NULL); if (r == -EPERM) /* Installing an epoll watch on a regular file doesn't work and fails with EPERM. Which is * totally OK, handle it gracefully. epoll_ctl() documents EPERM as the error returned when @@ -544,8 +544,6 @@ int action_show(char **matches) { } if (arg_follow) { - int sig; - assert(poll_fd >= 0); r = setup_event(&c, poll_fd); @@ -555,14 +553,12 @@ int action_show(char **matches) { r = sd_event_loop(c.event); if (r < 0) return r; - sig = r; r = update_cursor(c.journal); if (r < 0) return r; - /* re-send the original signal. */ - return sig; + return 0; } (void) sd_notify(/* unset_environment= */ false, "READY=1"); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 7d138df03a..4642cf4576 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1189,4 +1189,4 @@ static int run(int argc, char *argv[]) { } } -DEFINE_MAIN_FUNCTION_WITH_POSITIVE_SIGNAL(run); +DEFINE_MAIN_FUNCTION(run); From 054464ad520f2d9c9863d3b4c053618abfc5df24 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Jul 2025 18:30:48 +0900 Subject: [PATCH 2/4] main-func: drop unused DEFINE_MAIN_FUNCTION_WITH_POSITIVE_SIGNAL() --- src/shared/generator.h | 1 - src/shared/main-func.c | 24 ------------------------ src/shared/main-func.h | 20 +++++++------------- 3 files changed, 7 insertions(+), 38 deletions(-) diff --git a/src/shared/generator.h b/src/shared/generator.h index 8abf50f3ab..57c854e790 100644 --- a/src/shared/generator.h +++ b/src/shared/generator.h @@ -115,7 +115,6 @@ bool generator_soft_rebooted(void); impl(argv[1], \ argv[argc == 4 ? 2 : 1], \ argv[argc == 4 ? 3 : 1]), \ - exit_failure_if_negative, \ exit_failure_if_negative) typedef enum GptAutoRoot { diff --git a/src/shared/main-func.c b/src/shared/main-func.c index d16b6f1b9d..4aa6d10433 100644 --- a/src/shared/main-func.c +++ b/src/shared/main-func.c @@ -1,20 +1,15 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#if HAVE_VALGRIND_VALGRIND_H -# include -#endif #include "sd-daemon.h" #include "argv-util.h" #include "ask-password-agent.h" -#include "hashmap.h" #include "main-func.h" #include "pager.h" #include "polkit-agent.h" #include "selinux-util.h" -#include "signal-util.h" #include "string-util.h" void main_prepare(int argc, char *argv[]) { @@ -32,25 +27,6 @@ void main_finalize(int r, int exit_status) { mac_selinux_finish(); } -int raise_or_exit_status(int ret) { - if (ret < 0) - return EXIT_FAILURE; - if (ret == 0) - return EXIT_SUCCESS; - if (!SIGNAL_VALID(ret)) - return EXIT_FAILURE; - -#if HAVE_VALGRIND_VALGRIND_H - /* If raise() below succeeds, the destructor cleanup_pools() in hashmap.c will never called. */ - if (RUNNING_ON_VALGRIND) - hashmap_trim_pools(); -#endif - - (void) raise(ret); - /* exit with failure if raise() does not immediately abort the program. */ - return EXIT_FAILURE; -} - int exit_failure_if_negative(int result) { return result < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/shared/main-func.h b/src/shared/main-func.h index ffe6fb5227..ecd23fe601 100644 --- a/src/shared/main-func.h +++ b/src/shared/main-func.h @@ -8,15 +8,16 @@ void main_prepare(int argc, char *argv[]); void main_finalize(int r, int exit_status); -#define _DEFINE_MAIN_FUNCTION(intro, impl, result_to_exit_status, result_to_return_value) \ +#define _DEFINE_MAIN_FUNCTION(intro, impl, result_to_exit_status) \ int main(int argc, char *argv[]) { \ - int r; \ + int r, s; \ main_prepare(argc, argv); \ intro; \ r = impl; \ - main_finalize(r, result_to_exit_status(r)); \ + s = result_to_exit_status(r); \ + main_finalize(r, s); \ static_destruct(); \ - return result_to_return_value(r); \ + return s; \ } int exit_failure_if_negative(int result) _const_; @@ -24,7 +25,7 @@ int exit_failure_if_negative(int result) _const_; /* Negative return values from impl are mapped to EXIT_FAILURE, and * everything else means success! */ #define DEFINE_MAIN_FUNCTION(impl) \ - _DEFINE_MAIN_FUNCTION(,impl(argc, argv), exit_failure_if_negative, exit_failure_if_negative) + _DEFINE_MAIN_FUNCTION(, impl(argc, argv), exit_failure_if_negative) int exit_failure_if_nonzero(int result) _const_; @@ -32,11 +33,4 @@ int exit_failure_if_nonzero(int result) _const_; * and positive values are propagated. * Note: "true" means failure! */ #define DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(impl) \ - _DEFINE_MAIN_FUNCTION(,impl(argc, argv), exit_failure_if_nonzero, exit_failure_if_nonzero) - -int raise_or_exit_status(int ret); - -/* Negative return values from impl are mapped to EXIT_FAILURE, zero is mapped to EXIT_SUCCESS, - * and raise if a positive signal is returned from impl. */ -#define DEFINE_MAIN_FUNCTION_WITH_POSITIVE_SIGNAL(impl) \ - _DEFINE_MAIN_FUNCTION(,impl(argc, argv), exit_failure_if_negative, raise_or_exit_status) + _DEFINE_MAIN_FUNCTION(, impl(argc, argv), exit_failure_if_nonzero) From d6b379370485b9651f86b628ce980de719299eb4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Jul 2025 18:36:09 +0900 Subject: [PATCH 3/4] test: drop unnecessary disablement of pipefail --- test/units/TEST-04-JOURNAL.journal.sh | 2 -- test/units/TEST-17-UDEV.rename-netif.sh | 2 -- test/units/TEST-35-LOGIN.sh | 4 ---- test/units/TEST-50-DISSECT.dissect.sh | 2 -- test/units/TEST-60-MOUNT-RATELIMIT.sh | 2 -- test/units/TEST-75-RESOLVED.sh | 1 - 6 files changed, 13 deletions(-) diff --git a/test/units/TEST-04-JOURNAL.journal.sh b/test/units/TEST-04-JOURNAL.journal.sh index bd9f8a5e82..56b5731788 100755 --- a/test/units/TEST-04-JOURNAL.journal.sh +++ b/test/units/TEST-04-JOURNAL.journal.sh @@ -210,12 +210,10 @@ sleep 3 [[ ! -f "/tmp/i-lose-my-logs" ]] systemctl stop forever-print-hola -set +o pipefail # https://github.com/systemd/systemd/issues/15528 journalctl --follow --file=/var/log/journal/*/* | head -n1 | grep . # https://github.com/systemd/systemd/issues/24565 journalctl --follow --merge | head -n1 | grep . -set -o pipefail # https://github.com/systemd/systemd/issues/26746 rm -f /tmp/issue-26746-log /tmp/issue-26746-cursor diff --git a/test/units/TEST-17-UDEV.rename-netif.sh b/test/units/TEST-17-UDEV.rename-netif.sh index 96430e64ae..a9fcf28bfb 100755 --- a/test/units/TEST-17-UDEV.rename-netif.sh +++ b/test/units/TEST-17-UDEV.rename-netif.sh @@ -174,9 +174,7 @@ EOF test -n "$found" journalctl --sync - set +o pipefail timeout -v 30 journalctl _PID=1 _COMM=systemd --since "$since" -n all --follow | grep -m 1 -q -F 'foobar: systemd-udevd failed to process the device, ignoring: File exists' - set -o pipefail # check if the invalid SYSTEMD_ALIAS property for the interface foobar is ignored by PID1 assert_eq "$(systemctl show --property=SysFSPath --value /sys/subsystem/net/devices/hoge)" "/sys/devices/virtual/net/hoge" } diff --git a/test/units/TEST-35-LOGIN.sh b/test/units/TEST-35-LOGIN.sh index a543023cea..f5b1911edc 100755 --- a/test/units/TEST-35-LOGIN.sh +++ b/test/units/TEST-35-LOGIN.sh @@ -528,9 +528,7 @@ EOF # become idle again. 'Lock' signal is sent out for each session, we have at # least one session, so minimum of 2 "Lock" signals must have been sent. journalctl --sync - set +o pipefail timeout -v 35 journalctl -b -u systemd-logind.service --since="$ts" -n all --follow | grep -m 1 -q 'Sent message type=signal .* member=Lock' - set -o pipefail # We need to know that a new message was sent after waking up, # so we must track how many happened before sleeping to check we have extra. @@ -541,10 +539,8 @@ EOF # Wait again journalctl --sync - set +o pipefail timeout -v 35 journalctl -b -u systemd-logind.service --since="$ts" -n all --follow | grep -m "$((locks + 1))" -q 'Sent message type=signal .* member=Lock' timeout -v 35 journalctl -b -u systemd-logind.service --since="$ts" -n all --follow | grep -m 2 -q -F 'System idle. Will be locked now.' - set -o pipefail } testcase_session_properties() { diff --git a/test/units/TEST-50-DISSECT.dissect.sh b/test/units/TEST-50-DISSECT.dissect.sh index 8cfeeb66d4..e9ae17ca27 100755 --- a/test/units/TEST-50-DISSECT.dissect.sh +++ b/test/units/TEST-50-DISSECT.dissect.sh @@ -933,12 +933,10 @@ systemd-sysext merge --no-reload systemd-sysext unmerge --no-reload systemd-sysext merge journalctl --sync -set +o pipefail # "journalctl -u foo.service" may not work as expected, especially entries for _TRANSPORT=stdout, # for short-living services or when the service manager generates debugging logs. # Instead, SYSLOG_IDENTIFIER= should be reliable for stdout. Let's use it. timeout -v 30s journalctl -b SYSLOG_IDENTIFIER=echo _TRANSPORT=stdout -o cat -n all --follow | grep -m 1 -q '^foo$' -set -o pipefail systemd-sysext unmerge --no-reload # Grep on the Warning to find the warning helper mentioning the daemon reload. systemctl status foo.service 2>&1 | grep -q -F "Warning" diff --git a/test/units/TEST-60-MOUNT-RATELIMIT.sh b/test/units/TEST-60-MOUNT-RATELIMIT.sh index 67d44c281a..fdef5999ef 100755 --- a/test/units/TEST-60-MOUNT-RATELIMIT.sh +++ b/test/units/TEST-60-MOUNT-RATELIMIT.sh @@ -305,13 +305,11 @@ testcase_mount_ratelimit() { # Figure out if we have entered the rate limit state. # If the infra is slow we might not enter the rate limit state; in that case skip the exit check. - set +o pipefail journalctl --sync if timeout 2m journalctl -u init.scope --since="$ts" -n all --follow | grep -m 1 -q -F '(mount-monitor-dispatch) entered rate limit'; then journalctl --sync timeout 2m journalctl -u init.scope --since="$ts" -n all --follow | grep -m 1 -q -F '(mount-monitor-dispatch) left rate limit' fi - set -o pipefail # Verify that the mount units are always cleaned up at the end. # Give some time for units to settle so we don't race between exiting the rate limit state and cleaning up the units. diff --git a/test/units/TEST-75-RESOLVED.sh b/test/units/TEST-75-RESOLVED.sh index c4ed53ec6c..1bde5f3b76 100755 --- a/test/units/TEST-75-RESOLVED.sh +++ b/test/units/TEST-75-RESOLVED.sh @@ -53,7 +53,6 @@ enable_ipv6() { monitor_check_rr() ( set +x - set +o pipefail local since="${1:?}" local match="${2:?}" From 49bdc8b1e84bf8c1c5639cb34fa27bb131577605 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Jul 2025 18:42:02 +0900 Subject: [PATCH 4/4] NEWS: mention about the exit code change in journalctl --follow --- NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS b/NEWS index 2af46a61e0..2f31221ad1 100644 --- a/NEWS +++ b/NEWS @@ -135,6 +135,9 @@ CHANGES WITH 258 in spe: incompatible with PrivateDevices= and resulted in automatic extension of the DeviceAllow= list. The latter behaviour has been removed. + * The command 'journalctl --follow' now exits with success on + SIGTERM/SIGINT, or its pipe STDOUT is disconnected. + Announcements of Future Feature Removals: * Support for System V service scripts is deprecated and will be