From: Amos Jeffries <squid3@treenet.co.nz>
Date: Fri, 17 Jul 2009 02:49:49 +0000 (+1200)
Subject: Bug 2680 regression: Crash after rotate with no helpers running
X-Git-Tag: SQUID_3_2_0_1~874
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d974a0723c00b59328754bbd964e34fdd03a890a;p=thirdparty%2Fsquid.git

Bug 2680 regression: Crash after rotate with no helpers running

Regression from bug 2276 fix. n_running was used instead of n_active.
Also documents the relevant counters to prevent this recurring.
---

diff --git a/src/helper.cc b/src/helper.cc
index 9be0b5d8d6..101f869409 100644
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -103,7 +103,7 @@ helperOpenServers(helper * hlp)
         shortname = xstrdup(progname);
 
     /* dont ever start more than hlp->n_to_start processes. */
-    int need_new = hlp->n_to_start - hlp->n_running;
+    int need_new = hlp->n_to_start - hlp->n_active;
 
     debugs(84, 1, "helperOpenServers: Starting " << need_new << "/" << hlp->n_to_start << " '" << shortname << "' processes");
 
@@ -209,7 +209,8 @@ helperStatefulOpenServers(statefulhelper * hlp)
         shortname = xstrdup(progname);
 
     /* dont ever start more than hlp->n_to_start processes. */
-    int need_new = hlp->n_to_start - hlp->n_running;
+    /* n_active are the helpers which have not been shut down. */
+    int need_new = hlp->n_to_start - hlp->n_active;
 
     debugs(84, 1, "helperOpenServers: Starting " << need_new << "/" << hlp->n_to_start << " '" << shortname << "' processes");
 
@@ -545,8 +546,8 @@ helperStats(StoreEntry * sentry, helper * hlp, const char *label)
 
     storeAppendPrintf(sentry, "program: %s\n",
                       hlp->cmdline->key);
-    storeAppendPrintf(sentry, "number running: %d of %d\n",
-                      hlp->n_running, hlp->n_to_start);
+    storeAppendPrintf(sentry, "number active: %d of %d (%d shutting down)\n",
+                      hlp->n_active, hlp->n_to_start, (hlp->n_running - hlp->n_active) );
     storeAppendPrintf(sentry, "requests sent: %d\n",
                       hlp->stats.requests);
     storeAppendPrintf(sentry, "replies received: %d\n",
@@ -587,7 +588,7 @@ helperStats(StoreEntry * sentry, helper * hlp, const char *label)
     storeAppendPrintf(sentry, "   B = BUSY\n");
     storeAppendPrintf(sentry, "   W = WRITING\n");
     storeAppendPrintf(sentry, "   C = CLOSING\n");
-    storeAppendPrintf(sentry, "   S = SHUTDOWN\n");
+    storeAppendPrintf(sentry, "   S = SHUTDOWN PENDING\n");
 }
 
 void
@@ -598,8 +599,8 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label
 
     storeAppendPrintf(sentry, "program: %s\n",
                       hlp->cmdline->key);
-    storeAppendPrintf(sentry, "number running: %d of %d\n",
-                      hlp->n_running, hlp->n_to_start);
+    storeAppendPrintf(sentry, "number active: %d of %d (%d shutting down)\n",
+                      hlp->n_active, hlp->n_to_start, (hlp->n_running - hlp->n_active) );
     storeAppendPrintf(sentry, "requests sent: %d\n",
                       hlp->stats.requests);
     storeAppendPrintf(sentry, "replies received: %d\n",
@@ -644,7 +645,7 @@ helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label
     storeAppendPrintf(sentry, "   B = BUSY\n");
     storeAppendPrintf(sentry, "   C = CLOSING\n");
     storeAppendPrintf(sentry, "   R = RESERVED or DEFERRED\n");
-    storeAppendPrintf(sentry, "   S = SHUTDOWN\n");
+    storeAppendPrintf(sentry, "   S = SHUTDOWN PENDING\n");
     storeAppendPrintf(sentry, "   P = PLACEHOLDER\n");
 }
 
@@ -671,7 +672,6 @@ helperShutdown(helper * hlp)
 
         hlp->n_active--;
         assert(hlp->n_active >= 0);
-
         srv->flags.shutdown = 1;	/* request it to shut itself down */
 
         if (srv->flags.closing) {
diff --git a/src/helper.h b/src/helper.h
index 00c15b366e..1bb34a50fd 100644
--- a/src/helper.h
+++ b/src/helper.h
@@ -58,9 +58,9 @@ struct _helper {
     dlink_list servers;
     dlink_list queue;
     const char *id_name;
-    int n_to_start;
-    int n_running;
-    int n_active;
+    int n_to_start;           ///< Configuration setting of how many helper children should be running
+    int n_running;            ///< Total helper children objects currently existing
+    int n_active;             ///< Count of helper children active (not shutting down)
     int ipc_type;
     IpAddress addr;
     unsigned int concurrency;
@@ -80,9 +80,9 @@ struct _helper_stateful {
     dlink_list servers;
     dlink_list queue;
     const char *id_name;
-    int n_to_start;
-    int n_running;
-    int n_active;
+    int n_to_start;           ///< Configuration setting of how many helper children should be running
+    int n_running;            ///< Total helper children objects currently existing
+    int n_active;             ///< Count of helper children active (not shutting down)
     int ipc_type;
     IpAddress addr;
     MemAllocator *datapool;