[tor-commits] [tor/master] Move the responsibility for delayed shutdown into the mainloop

[nickm at torproject.org]

commit 303e5c70e08d72614ea95fd0c5aad7e05852b6b7
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Nov 13 09:04:11 2018 -0500

    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 ebf9735d4..d5f3cb13f 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 <b>delay_sec</b> 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 <b>now</b> and <b>next</b>,
  * 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 632733d9a..6f7b71685 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 6f8795cec..968c39dd6 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);
     }