Bug 3574: To avoid crashes, prohibit reconfiguration during shutdown.

Also consolidated and polished signal action handling code:

1. For any executed action X, clear do_X at the beginning of action X
   code because once we start X, we should accept/queue more X
   requests (or inform the admin if we reject them).

2. Delay any action X requested during startup or reconfiguration
   because the latter two actions modify global state that X depends
   on. Inform the admin that the requested action is being delayed.

3. Cancel any action X requested during shutdown. We cannot run X
   during shutdown because shutdown modifies global state that X
   depends on, and we never come back from shutdown so there is no
   point in delaying X. Inform the admin that the requested action is
   canceled.

The child signal handling action is exempt from rules #2 and #3
because its code does not depend on Squid state.

Repeated failed attempts to fix crashes related to various overlapping
actions confirm that this code is a lot trickier than it looks. This
change introduces a more systematic/comprehensive approach to
resolving associated conflicts compared to previous ad hoc attempts.

These changes were not inspired by bug 3574 but they provide a
more comprehensive version of the earlier bug 3574 fix (r14354).

diff --git a/src/main.cc b/src/main.cc
index 6ec0437c04e3a01d37d23c07b558a0ef4100f43c..7447889754971927bce0b30932c62bdefead63dc 100644 (file)
--- a/src/main.cc
+++ b/src/main.cc
@@ -237,29 +237,51 @@ SignalEngine::checkEvents(int)
 {
     PROF_start(SignalEngine_checkEvents);
 
-    if (do_reconfigure) {
-        if (!reconfiguring && configured_once) {
-            mainReconfigureStart();
-            do_reconfigure = 0;
-        } // else wait until previous reconfigure is done
-    } else if (do_rotate) {
+    if (do_reconfigure)
+        mainReconfigureStart();
+    else if (do_rotate)
         mainRotate();
-        do_rotate = 0;
-    } else if (do_shutdown) {
+    else if (do_shutdown)
         doShutdown(do_shutdown > 0 ? (int) Config.shutdownLifetime : 0);
-        do_shutdown = 0;
-    }
-    if (do_handle_stopped_child) {
-        do_handle_stopped_child = 0;
+    if (do_handle_stopped_child)
         handleStoppedChild();
-    }
     PROF_stop(SignalEngine_checkEvents);
     return EVENT_IDLE;
 }
 
+/// Decides whether the signal-controlled action X should be delayed, canceled,
+/// or executed immediately. Clears do_X (via signalVar) as needed.
+static bool
+AvoidSignalAction(const char *description, volatile int &signalVar)
+{
+    const char *avoiding = "delaying";
+    const char *currentEvent = "none";
+    if (shutting_down) {
+        currentEvent = "shutdown";
+        avoiding = "canceling";
+        signalVar = 0;
+    }
+    else if (!configured_once)
+        currentEvent = "startup";
+    else if (reconfiguring)
+        currentEvent = "reconfiguration";
+    else {
+        signalVar = 0;
+        return false; // do not avoid (i.e., execute immediately)
+        // the caller may produce a signal-specific debugging message
+    }
+
+    debugs(1, DBG_IMPORTANT, avoiding << ' ' << description <<
+           " request during " << currentEvent);
+    return true;
+}
+
 void
 SignalEngine::doShutdown(time_t wait)
 {
+    if (AvoidSignalAction("shutdown", do_shutdown))
+        return;
+
     debugs(1, DBG_IMPORTANT, "Preparing for shutdown after " << statCounter.client_http.requests << " requests");
     debugs(1, DBG_IMPORTANT, "Waiting " << wait << " seconds for active connections to finish");
 
@@ -297,6 +319,10 @@ SignalEngine::doShutdown(time_t wait)
 void
 SignalEngine::handleStoppedChild()
 {
+    // no AvoidSignalAction() call: This code can run at any time because it
+    // does not depend on Squid state. It does not need debugging because it
+    // handles an "internal" signal, not an external/admin command.
+    do_handle_stopped_child = 0;
 #if !_SQUID_WINDOWS_
     PidStatus status;
     pid_t pid;
@@ -805,6 +831,9 @@ serverConnectionsClose(void)
 static void
 mainReconfigureStart(void)
 {
+    if (AvoidSignalAction("reconfiguration", do_reconfigure))
+        return;
+
     debugs(1, DBG_IMPORTANT, "Reconfiguring Squid Cache (version " << version_string << ")...");
     reconfiguring = 1;
 
@@ -962,15 +991,14 @@ mainReconfigureFinish(void *)
         writePidFile(); /* write PID file */
 
     reconfiguring = 0;
-
-    // ignore any pending re-reconfigure signals if shutdown received
-    if (do_shutdown)
-        do_reconfigure = 0;
 }
 
 static void
 mainRotate(void)
 {
+    if (AvoidSignalAction("log rotation", do_rotate))
+        return;
+
     icmpEngine.Close();
     redirectShutdown();
 #if USE_AUTH
@@ -1476,7 +1504,6 @@ SquidMain(int argc, char **argv)
         Format::Token::Init(); // XXX: temporary. Use a runners registry of pre-parse runners instead.
 
         try {
-            do_reconfigure = 0; // ignore any early (boot/startup) reconfigure signals
             parse_err = parseConfigFile(ConfigFile);
         } catch (...) {
             // for now any errors are a fatal condition...