From 19252b254861d8c9b56e2acaeb182812c8f07e52 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Mar 2022 20:37:43 +0100 Subject: [PATCH 1/2] journald: make sure SIGTERM handling doesn't get starved out Fixes: #22642 --- src/journal/journald-server.c | 76 +++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index a7858eedff..0ea6952f9a 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1447,12 +1447,82 @@ static int dispatch_sigusr2(sd_event_source *es, const struct signalfd_siginfo * } static int dispatch_sigterm(sd_event_source *es, const struct signalfd_siginfo *si, void *userdata) { + _cleanup_(sd_event_source_disable_unrefp) sd_event_source *news = NULL; Server *s = userdata; + int r; assert(s); log_received_signal(LOG_INFO, si); + (void) sd_event_source_set_enabled(es, false); /* Make sure this handler is called at most once */ + + /* So on one hand we want to ensure that SIGTERMs are definitely handled in appropriate, bounded + * time. On the other hand we want that everything pending is first comprehensively processed and + * written to disk. These goals are incompatible, hence we try to find a middle ground: we'll process + * SIGTERM with high priority, but from the handler (this one right here) we'll install two new event + * sources: one low priority idle one that will issue the exit once everything else is processed (and + * which is hopefully the regular, clean codepath); and one high priority timer that acts as safety + * net: if our idle handler isn't run within 10s, we'll exit anyway. + * + * TLDR: we'll exit either when everything is processed, or after 10s max, depending on what happens + * first. + * + * Note that exiting before the idle event is hit doesn't typically mean that we lose any data, as + * messages will remain queued in the sockets they came in from, and thus can be processed when we + * start up next – unless we are going down for the final system shutdown, in which case everything + * is lost. */ + + r = sd_event_add_defer(s->event, &news, NULL, NULL); /* NULL handler means → exit when triggered */ + if (r < 0) { + log_error_errno(r, "Failed to allocate exit idle event handler: %m"); + goto fail; + } + + (void) sd_event_source_set_description(news, "exit-idle"); + + /* Run everything relevant before this. */ + r = sd_event_source_set_priority(news, SD_EVENT_PRIORITY_NORMAL+20); + if (r < 0) { + log_error_errno(r, "Failed to adjust priority of exit idle event handler: %m"); + goto fail; + } + + /* Give up ownership, so that this event source is freed automatically when the event loop is freed. */ + r = sd_event_source_set_floating(news, true); + if (r < 0) { + log_error_errno(r, "Failed to make exit idle event handler floating: %m"); + goto fail; + } + + news = sd_event_source_unref(news); + + r = sd_event_add_time_relative(s->event, &news, CLOCK_MONOTONIC, 10 * USEC_PER_SEC, 0, NULL, NULL); + if (r < 0) { + log_error_errno(r, "Failed to allocate exit timeout event handler: %m"); + goto fail; + } + + (void) sd_event_source_set_description(news, "exit-timeout"); + + r = sd_event_source_set_priority(news, SD_EVENT_PRIORITY_IMPORTANT-20); /* This is a safety net, with highest priority */ + if (r < 0) { + log_error_errno(r, "Failed to adjust priority of exit timeout event handler: %m"); + goto fail; + } + + r = sd_event_source_set_floating(news, true); + if (r < 0) { + log_error_errno(r, "Failed to make exit timeout event handler floating: %m"); + goto fail; + } + + news = sd_event_source_unref(news); + + log_debug("Exit event sources are now pending."); + return 0; + +fail: sd_event_exit(s->event, 0); return 0; } @@ -1504,8 +1574,8 @@ static int setup_signals(Server *s) { if (r < 0) return r; - /* Let's process SIGTERM late, so that we flush all queued messages to disk before we exit */ - r = sd_event_source_set_priority(s->sigterm_event_source, SD_EVENT_PRIORITY_NORMAL+20); + /* Let's process SIGTERM early, so that we definitely react to it */ + r = sd_event_source_set_priority(s->sigterm_event_source, SD_EVENT_PRIORITY_IMPORTANT-10); if (r < 0) return r; @@ -1515,7 +1585,7 @@ static int setup_signals(Server *s) { if (r < 0) return r; - r = sd_event_source_set_priority(s->sigint_event_source, SD_EVENT_PRIORITY_NORMAL+20); + r = sd_event_source_set_priority(s->sigint_event_source, SD_EVENT_PRIORITY_IMPORTANT-10); if (r < 0) return r; From 47f04c2a69d5a604411f17a2e660021165d09c89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Mar 2022 21:24:23 +0100 Subject: [PATCH 2/2] journal-file: if we are going down, don't use event loop to schedule post The event loop is already shutting down, hence no point in using it anymore, it's not going to run any further iteration. --- src/libsystemd/sd-journal/journal-file.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 27875fb9e7..c358772f51 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1931,11 +1931,18 @@ static int post_change_thunk(sd_event_source *timer, uint64_t usec, void *userda } static void schedule_post_change(JournalFile *f) { + sd_event *e; int r; assert(f); assert(f->post_change_timer); + assert_se(e = sd_event_source_get_event(f->post_change_timer)); + + /* If we are aleady going down, post the change immediately. */ + if (IN_SET(sd_event_get_state(e), SD_EVENT_EXITING, SD_EVENT_FINISHED)) + goto fail; + r = sd_event_source_get_enabled(f->post_change_timer, NULL); if (r < 0) { log_debug_errno(r, "Failed to get ftruncate timer state: %m");