From 303e5c70e08d72614ea95fd0c5aad7e05852b6b7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Nov 2018 09:04:11 -0500 Subject: [PATCH] Move the responsibility for delayed shutdown into the mainloop This is part of 28422, so we don't have to call consider_hibernation() once per second when we're dormant. This commit does not remove delayed shutdown from hibernate.c: it uses it as a backup shutdown mechanism, in case the regular shutdown timer mechanism fails for some reason. --- src/core/mainloop/mainloop.c | 36 ++++++++++++++++++++++++++----- src/core/mainloop/mainloop.h | 2 ++ src/feature/hibernate/hibernate.c | 20 +++++++++++------ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index ebf9735d49..d5f3cb13f9 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1703,6 +1703,30 @@ mainloop_schedule_postloop_cleanup(void) mainloop_event_activate(postloop_cleanup_ev); } +/** Event to run 'scheduled_shutdown_cb' */ +static mainloop_event_t *scheduled_shutdown_ev=NULL; + +/** Callback: run a scheduled shutdown */ +static void +scheduled_shutdown_cb(mainloop_event_t *ev, void *arg) +{ + (void)ev; + (void)arg; + log_notice(LD_GENERAL, "Clean shutdown finished. Exiting."); + tor_shutdown_event_loop_and_exit(0); +} + +/** Schedule the mainloop to exit after delay_sec seconds. */ +void +mainloop_schedule_shutdown(int delay_sec) +{ + const struct timeval delay_tv = { delay_sec, 0 }; + if (! scheduled_shutdown_ev) { + scheduled_shutdown_ev = mainloop_event_new(scheduled_shutdown_cb, NULL); + } + mainloop_event_schedule(scheduled_shutdown_ev, &delay_tv); +} + #define LONGEST_TIMER_PERIOD (30 * 86400) /** Helper: Return the number of seconds between now and next, * clipped to the range [1 second, LONGEST_TIMER_PERIOD]. */ @@ -1743,12 +1767,13 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg) */ update_current_time(now); - /* 0. See if we've been asked to shut down and our timeout has - * expired; or if our bandwidth limits are exhausted and we - * should hibernate; or if it's time to wake up from hibernation. + /* 0. See if our bandwidth limits are exhausted and we should hibernate; or + * if it's time to wake up from hibernation; or if we have a scheduled + * shutdown and it's time to run it. + * + * Note: we have redundant mechanisms to handle the */ - // TODO: Refactor or rewrite, or NET_PARTICIPANT. Needs separate wakeup - // handling. + // TODO: NET_PARTICIPANT. consider_hibernation(now); /* Maybe enough time elapsed for us to reconsider a circuit. */ @@ -2936,6 +2961,7 @@ tor_mainloop_free_all(void) mainloop_event_free(schedule_active_linked_connections_event); mainloop_event_free(postloop_cleanup_ev); mainloop_event_free(handle_deferred_signewnym_ev); + mainloop_event_free(scheduled_shutdown_ev); #ifdef HAVE_SYSTEMD_209 periodic_timer_free(systemd_watchdog_timer); diff --git a/src/core/mainloop/mainloop.h b/src/core/mainloop/mainloop.h index 632733d9a6..6f7b716858 100644 --- a/src/core/mainloop/mainloop.h +++ b/src/core/mainloop/mainloop.h @@ -86,6 +86,8 @@ void reschedule_per_second_timer(void); void do_signewnym(time_t); time_t get_last_signewnym_time(void); +void mainloop_schedule_shutdown(int delay_sec); + void tor_init_connection_lists(void); void initialize_mainloop_events(void); void tor_mainloop_free_all(void); diff --git a/src/feature/hibernate/hibernate.c b/src/feature/hibernate/hibernate.c index 6f8795cecc..968c39dd6d 100644 --- a/src/feature/hibernate/hibernate.c +++ b/src/feature/hibernate/hibernate.c @@ -66,8 +66,9 @@ static hibernate_state_t hibernate_state = HIBERNATE_STATE_INITIAL; /** If are hibernating, when do we plan to wake up? Set to 0 if we * aren't hibernating. */ static time_t hibernate_end_time = 0; -/** If we are shutting down, when do we plan finally exit? Set to 0 if - * we aren't shutting down. */ +/** If we are shutting down, when do we plan finally exit? Set to 0 if we + * aren't shutting down. (This is obsolete; scheduled shutdowns are supposed + * to happen from mainloop_schedule_shutdown() now.) */ static time_t shutdown_time = 0; /** A timed event that we'll use when it's time to wake up from @@ -867,7 +868,13 @@ hibernate_begin(hibernate_state_t new_state, time_t now) log_notice(LD_GENERAL,"Interrupt: we have stopped accepting new " "connections, and will shut down in %d seconds. Interrupt " "again to exit now.", options->ShutdownWaitLength); - shutdown_time = time(NULL) + options->ShutdownWaitLength; + /* We add an arbitrary delay here so that even if something goes wrong + * with the mainloop shutdown code, we can still shutdown from + * consider_hibernation() if we call it... but so that the + * mainloop_schedule_shutdown() mechanism will be the first one called. + */ + shutdown_time = time(NULL) + options->ShutdownWaitLength + 5; + mainloop_schedule_shutdown(options->ShutdownWaitLength); #ifdef HAVE_SYSTEMD /* tell systemd that we may need more than the default 90 seconds to shut * down so they don't kill us. add some extra time to actually finish @@ -1096,11 +1103,12 @@ consider_hibernation(time_t now) hibernate_state_t prev_state = hibernate_state; /* If we're in 'exiting' mode, then we just shut down after the interval - * elapses. */ + * elapses. The mainloop was supposed to catch this via + * mainloop_schedule_shutdown(), but apparently it didn't. */ if (hibernate_state == HIBERNATE_STATE_EXITING) { tor_assert(shutdown_time); if (shutdown_time <= now) { - log_notice(LD_GENERAL, "Clean shutdown finished. Exiting."); + log_notice(LD_BUG, "Mainloop did not catch shutdown event; exiting."); tor_shutdown_event_loop_and_exit(0); } return; /* if exiting soon, don't worry about bandwidth limits */ @@ -1112,7 +1120,7 @@ consider_hibernation(time_t now) if (hibernate_end_time > now && accounting_enabled) { /* If we're hibernating, don't wake up until it's time, regardless of * whether we're in a new interval. */ - return ; + return; } else { hibernate_end_time_elapsed(now); }