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 9be0b5d8d6f33b0d961d58477dc00a833b0ac457..101f869409b6242fe3a39c35501a0fe51b23c931 100644 (file)
--- 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 00c15b366e7b6b2d6500f4acd5b3f8a863bd7627..1bb34a50fd3514cbf16671d3601dae4febce627d 100644 (file)
--- 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;