Squid crashes on shutdown while cleaning up idle ICAP connections, part2

Further polishing of r14825 patch:
  - Replace RegisterRunner(this) calls, with Independent::registerRunner() cals
    for IndependentRunner kid classes.
  - If RegisterRunner called for an IndependentRunner will throw
  - Fix Independent::registerRunner() to not use GetRidOfRunner. This is
    confuses Coverity checks, and make the code more complex.
  - Declare static FindRunners as inlinened function

This is a Measurement Factory project

diff --git a/src/base/RunnersRegistry.cc b/src/base/RunnersRegistry.cc
index 48f6c300db5af49562f9a0165b2f3da35e4a05d6..d4101b4c4e7ad4ea26b145d186f640b8bdd89d4e 100644 (file)
--- a/src/base/RunnersRegistry.cc
+++ b/src/base/RunnersRegistry.cc
@@ -8,6 +8,7 @@
 
 #include "squid.h"
 #include "base/RunnersRegistry.h"
+#include "base/TextException.h"
 #include <set>
 
 /// a collection of unique runners, in no particular order
@@ -19,7 +20,7 @@ static bool RunnersGone = false;
 
 /// creates the registered runners container if needed
 /// \return either registered runners (if they should exist) or nil (otherwise)
-static Runners *
+static inline Runners *
 FindRunners()
 {
     if (!TheRunners && !RunnersGone)
@@ -35,11 +36,21 @@ GetRidOfRunner(RegisteredRunner *rr)
     // else ignore; IndependentRunner
 }
 
+static inline void
+RegisterRunner_(RegisteredRunner *rr)
+{
+    Runners *runners = FindRunners();
+    Must(runners);
+    runners->insert(rr);
+}
+
 bool
 RegisterRunner(RegisteredRunner *rr)
 {
-    if (Runners *runners = FindRunners()) {
-        runners->insert(rr);
+    Must(!dynamic_cast<IndependentRunner*>(rr));
+
+    if (FindRunners()) {
+        RegisterRunner_(rr);
         return true;
     }
 
@@ -88,6 +99,14 @@ IndependentRunner::unregisterRunner()
     // else it is too late, finishShutdown() has been called
 }
 
+void
+IndependentRunner::registerRunner()
+{
+    if (FindRunners())
+        RegisterRunner_(this);
+    // else do nothing past finishShutdown
+}
+
 bool
 UseThisStatic(const void *)
 {

diff --git a/src/base/RunnersRegistry.h b/src/base/RunnersRegistry.h
index 5af14cfb293789ae57358b251e8401ea06c56713..df27aad893ba84cd155e60b2ce0565ba2a81216b 100644 (file)
--- a/src/base/RunnersRegistry.h
+++ b/src/base/RunnersRegistry.h
@@ -100,7 +100,7 @@ public:
     virtual ~IndependentRunner() { unregisterRunner(); }
 
 protected:
-    void registerRunner() {RegisterRunner(this);}
+    void registerRunner();
     void unregisterRunner(); ///< unregisters self; safe to call multiple times
 };
 

diff --git a/src/client_side.cc b/src/client_side.cc
index 417259abeb7123d4f1c53b4e87a8621bcf1567da..a50307cf9813568506e68ddcf2fa96ce6bdcbf47 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2448,7 +2448,7 @@ ConnStateData::ConnStateData(const MasterXaction::Pointer &xact) :
 
     // register to receive notice of Squid signal events
     // which may affect long persisting client connections
-    RegisterRunner(this);
+    registerRunner();
 }
 
 void

diff --git a/src/pconn.cc b/src/pconn.cc
index 2cb8415d942a3ab2665f380bb7f41bc2378fb6ad..752a3d84b1fad8a2756be8da65a083ff148a3d08 100644 (file)
--- a/src/pconn.cc
+++ b/src/pconn.cc
@@ -42,7 +42,7 @@ IdleConnList::IdleConnList(const char *aKey, PconnPool *thePool) :
 
     theList_ = new Comm::ConnectionPointer[capacity_];
 
-    RegisterRunner(this);
+    registerRunner();
 
 // TODO: re-attach to MemPools. WAS: theList = (?? *)pconn_fds_pool->alloc();
 }