On 02/16/2014 08:42 PM, Amos Jeffries wrote:
> On 17/02/2014 2:56 p.m., Alex Rousskov wrote:
>> If my suggestion to add shutdown() and other methods to runners is
>> accepted, I can help with the corresponding adjustments (both trunk and
>> the proposed patch).
>>
>> If my suggestion is rejected, I will post my thoughts on how to adjust
>> the proposed patch for the current two-action API.


> It seems we have discussed this same topic at least three times now in
> relation to as many components of Squid.

Sorry if I missed an earlier indication of an agreement.


> I am in agreement with the multi-action design.

The attached patch implements the multi-action design. converts old code
to use it, and adds a couple of new actions needed for the pending
patches. Please review and feel free to commit if you like it (the patch
preamble has the proposed commit message).

Here is a typical action sequence from start to exit, with one
reconfigure in between:

> 19:24:40.290| main.cc(1450) SquidMain: running 
> RegisteredRunner::finalizeConfig
> 19:24:40.290| main.cc(1451) SquidMain: running 
> RegisteredRunner::claimMemoryNeeds
> 19:24:40.290| main.cc(1452) SquidMain: running RegisteredRunner::useConfig
> 19:24:47.714| main.cc(809) mainReconfigureFinish: running 
> RegisteredRunner::syncConfig
> 19:25:09.229| main.cc(278) doShutdown: running RegisteredRunner::startShutdown
> 19:25:11.078| main.cc(1943) SquidShutdown: running 
> RegisteredRunner::finishShutdown


The patch has received only rudimentary testing -- the Vector change
still appears to cause more problems on my test platform. Please test in
your environment before commit, if any.


HTH,

Alex.

Migrated RegisteredRunners to a multi-action interface.

Old generic two-action RegisteredRunners were good for handling paired
create/destroy events, but not all main.cc events fit that model well. In
fact, even the old runners implemented the destruction action for one event
only (rrAfterConfig); all other runners implemented a single action.

The adjusted API better supports runners that are interested in any number
of the supported events. It also allows a single runner object to handle
multiple events, which simplifies current code and may help with better
[re]configuration handling in the future.


Added startShutdown() and finishShutdown() events. The former will be needed
for authentication module shutdown and more polished shutdown initiation code
in general (patch pending). The latter is needed for final cleanup code that
previously ran as the destruction action for rrAfterConfig. finishShutdown()
also destroys all runners.

Note that the master process in SMP mode does not run startShutdown because
that process lacks the main loop and startShutdown() promises at least one
main loop iteration (to help with clean connections closures, for example).


Added syncConfig() event that will be needed for the standby pool
implementation (patch pending) and future code that reacts to Squid
configuration changes caused by reconfiguration.


"after config" event is now called "use config" to better match verb+noun or
action+object naming scheme.

=== modified file 'src/CollapsedForwarding.cc'
--- src/CollapsedForwarding.cc	2014-01-27 05:27:41 +0000
+++ src/CollapsedForwarding.cc	2014-02-20 00:08:52 +0000
@@ -117,46 +117,46 @@ CollapsedForwarding::HandleNewData(const
 
 void
 CollapsedForwarding::HandleNotification(const Ipc::TypedMsgHdr &msg)
 {
     const int from = msg.getInt();
     debugs(17, 7, "from " << from);
     assert(queue.get());
     queue->clearReaderSignal(from);
     HandleNewData("after notification");
 }
 
 /// initializes shared queue used by CollapsedForwarding
 class CollapsedForwardingRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
     CollapsedForwardingRr(): owner(NULL) {}
     virtual ~CollapsedForwardingRr();
 
 protected:
-    virtual void create(const RunnerRegistry &);
-    virtual void open(const RunnerRegistry &);
+    virtual void create();
+    virtual void open();
 
 private:
     Ipc::MultiQueue::Owner *owner;
 };
 
-RunnerRegistrationEntry(rrAfterConfig, CollapsedForwardingRr);
+RunnerRegistrationEntry(CollapsedForwardingRr);
 
-void CollapsedForwardingRr::create(const RunnerRegistry &)
+void CollapsedForwardingRr::create()
 {
     Must(!owner);
     owner = Ipc::MultiQueue::Init(ShmLabel, Config.workers, 1,
                                   sizeof(CollapsedForwardingMsg),
                                   QueueCapacity);
 }
 
-void CollapsedForwardingRr::open(const RunnerRegistry &)
+void CollapsedForwardingRr::open()
 {
     CollapsedForwarding::Init();
 }
 
 CollapsedForwardingRr::~CollapsedForwardingRr()
 {
     delete owner;
 }

=== modified file 'src/DiskIO/IpcIo/IpcIoFile.cc'
--- src/DiskIO/IpcIo/IpcIoFile.cc	2014-01-27 05:27:41 +0000
+++ src/DiskIO/IpcIo/IpcIoFile.cc	2014-02-20 00:24:08 +0000
@@ -888,75 +888,69 @@ DiskerOpen(const String &path, int flags
         return false;
     }
 
     ++store_open_disk_fd;
     debugs(79,3, HERE << "rock db opened " << path << ": FD " << TheFile);
     return true;
 }
 
 static void
 DiskerClose(const String &path)
 {
     if (TheFile >= 0) {
         file_close(TheFile);
         debugs(79,3, HERE << "rock db closed " << path << ": FD " << TheFile);
         TheFile = -1;
         --store_open_disk_fd;
     }
 }
 
 /// reports our needs for shared memory pages to Ipc::Mem::Pages
-class IpcIoClaimMemoryNeedsRr: public RegisteredRunner
+/// and initializes shared memory segments used by IpcIoFile
+class IpcIoRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
-    virtual void run(const RunnerRegistry &r);
+    IpcIoRr(): owner(NULL) {}
+    virtual ~IpcIoRr();
+    virtual void claimMemoryNeeds();
+
+protected:
+    /* Ipc::Mem::RegisteredRunner API */
+    virtual void create();
+
+private:
+    Ipc::FewToFewBiQueue::Owner *owner;
 };
 
-RunnerRegistrationEntry(rrClaimMemoryNeeds, IpcIoClaimMemoryNeedsRr);
+RunnerRegistrationEntry(IpcIoRr);
 
 void
-IpcIoClaimMemoryNeedsRr::run(const RunnerRegistry &)
+IpcIoRr::claimMemoryNeeds()
 {
     const int itemsCount = Ipc::FewToFewBiQueue::MaxItemsCount(
                                ::Config.workers, ::Config.cacheSwap.n_strands, QueueCapacity);
     // the maximum number of shared I/O pages is approximately the
     // number of queue slots, we add a fudge factor to that to account
     // for corner cases where I/O pages are created before queue
     // limits are checked or destroyed long after the I/O is dequeued
     Ipc::Mem::NotePageNeed(Ipc::Mem::PageId::ioPage,
                            static_cast<int>(itemsCount * 1.1));
 }
 
-/// initializes shared memory segments used by IpcIoFile
-class IpcIoRr: public Ipc::Mem::RegisteredRunner
-{
-public:
-    /* RegisteredRunner API */
-    IpcIoRr(): owner(NULL) {}
-    virtual ~IpcIoRr();
-
-protected:
-    virtual void create(const RunnerRegistry &);
-
-private:
-    Ipc::FewToFewBiQueue::Owner *owner;
-};
-
-RunnerRegistrationEntry(rrAfterConfig, IpcIoRr);
-
-void IpcIoRr::create(const RunnerRegistry &)
+void
+IpcIoRr::create()
 {
     if (Config.cacheSwap.n_strands <= 0)
         return;
 
     Must(!owner);
     owner = Ipc::FewToFewBiQueue::Init(ShmLabel, Config.workers, 1,
                                        Config.cacheSwap.n_strands,
                                        1 + Config.workers, sizeof(IpcIoMsg),
                                        QueueCapacity);
 }
 
 IpcIoRr::~IpcIoRr()
 {
     delete owner;
 }

=== modified file 'src/MemStore.cc'
--- src/MemStore.cc	2014-01-01 19:20:49 +0000
+++ src/MemStore.cc	2014-02-20 00:25:20 +0000
@@ -729,110 +729,98 @@ MemStore::disconnect(StoreEntry &e)
             assert(mem_obj.memCache.io == MemObject::ioReading);
             map->closeForReading(mem_obj.memCache.index);
             mem_obj.memCache.index = -1;
             mem_obj.memCache.io = MemObject::ioDone;
         }
     }
 }
 
 /// calculates maximum number of entries we need to store and map
 int64_t
 MemStore::EntryLimit()
 {
     if (!Config.memShared || !Config.memMaxSize)
         return 0; // no memory cache configured
 
     const int64_t minEntrySize = Ipc::Mem::PageSize();
     const int64_t entryLimit = Config.memMaxSize / minEntrySize;
     return entryLimit;
 }
 
-/// reports our needs for shared memory pages to Ipc::Mem::Pages
-class MemStoreClaimMemoryNeedsRr: public RegisteredRunner
+/// reports our needs for shared memory pages to Ipc::Mem::Pages;
+/// decides whether to use a shared memory cache or checks its configuration;
+/// and initializes shared memory segments used by MemStore
+class MemStoreRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
-    virtual void run(const RunnerRegistry &r);
+    MemStoreRr(): spaceOwner(NULL), mapOwner(NULL) {}
+    virtual void finalizeConfig();
+    virtual void claimMemoryNeeds();
+    virtual void useConfig();
+    virtual ~MemStoreRr();
+
+protected:
+    /* Ipc::Mem::RegisteredRunner API */
+    virtual void create();
+
+private:
+    Ipc::Mem::Owner<Ipc::Mem::PageStack> *spaceOwner; ///< free slices Owner
+    MemStoreMap::Owner *mapOwner; ///< primary map Owner
 };
 
-RunnerRegistrationEntry(rrClaimMemoryNeeds, MemStoreClaimMemoryNeedsRr);
+RunnerRegistrationEntry(MemStoreRr);
 
 void
-MemStoreClaimMemoryNeedsRr::run(const RunnerRegistry &)
+MemStoreRr::claimMemoryNeeds()
 {
     Ipc::Mem::NotePageNeed(Ipc::Mem::PageId::cachePage, MemStore::EntryLimit());
 }
 
-/// decides whether to use a shared memory cache or checks its configuration
-class MemStoreCfgRr: public ::RegisteredRunner
-{
-public:
-    /* RegisteredRunner API */
-    virtual void run(const RunnerRegistry &);
-};
-
-RunnerRegistrationEntry(rrFinalizeConfig, MemStoreCfgRr);
-
-void MemStoreCfgRr::run(const RunnerRegistry &r)
+void
+MemStoreRr::finalizeConfig()
 {
     // decide whether to use a shared memory cache if the user did not specify
     if (!Config.memShared.configured()) {
         Config.memShared.configure(Ipc::Atomic::Enabled() &&
                                    Ipc::Mem::Segment::Enabled() && UsingSmp() &&
                                    Config.memMaxSize > 0);
     } else if (Config.memShared && !Ipc::Atomic::Enabled()) {
         // bail if the user wants shared memory cache but we cannot support it
         fatal("memory_cache_shared is on, but no support for atomic operations detected");
     } else if (Config.memShared && !Ipc::Mem::Segment::Enabled()) {
         fatal("memory_cache_shared is on, but no support for shared memory detected");
     } else if (Config.memShared && !UsingSmp()) {
         debugs(20, DBG_IMPORTANT, "WARNING: memory_cache_shared is on, but only"
                " a single worker is running");
     }
 }
 
-/// initializes shared memory segments used by MemStore
-class MemStoreRr: public Ipc::Mem::RegisteredRunner
-{
-public:
-    /* RegisteredRunner API */
-    MemStoreRr(): spaceOwner(NULL), mapOwner(NULL) {}
-    virtual void run(const RunnerRegistry &);
-    virtual ~MemStoreRr();
-
-protected:
-    virtual void create(const RunnerRegistry &);
-
-private:
-    Ipc::Mem::Owner<Ipc::Mem::PageStack> *spaceOwner; ///< free slices Owner
-    MemStoreMap::Owner *mapOwner; ///< primary map Owner
-};
-
-RunnerRegistrationEntry(rrAfterConfig, MemStoreRr);
-
-void MemStoreRr::run(const RunnerRegistry &r)
+void
+MemStoreRr::useConfig()
 {
     assert(Config.memShared.configured());
-    Ipc::Mem::RegisteredRunner::run(r);
+    Ipc::Mem::RegisteredRunner::useConfig();
 }
 
-void MemStoreRr::create(const RunnerRegistry &)
+void
+MemStoreRr::create()
 {
     if (!Config.memShared)
         return;
 
     const int64_t entryLimit = MemStore::EntryLimit();
     if (entryLimit <= 0) {
         if (Config.memMaxSize > 0) {
             debugs(20, DBG_IMPORTANT, "WARNING: mem-cache size is too small ("
                    << (Config.memMaxSize / 1024.0) << " KB), should be >= " <<
                    (Ipc::Mem::PageSize() / 1024.0) << " KB");
         }
         return; // no memory cache configured or a misconfiguration
     }
 
     Must(!spaceOwner);
     spaceOwner = shm_new(Ipc::Mem::PageStack)(SpaceLabel, SpacePoolId,
                  entryLimit,
                  sizeof(Ipc::Mem::PageId));
     Must(!mapOwner);
     mapOwner = MemStoreMap::Init(MapLabel, entryLimit);

=== modified file 'src/Transients.cc'
--- src/Transients.cc	2013-12-31 18:49:41 +0000
+++ src/Transients.cc	2014-02-20 00:20:36 +0000
@@ -369,57 +369,57 @@ Transients::disconnect(MemObject &mem_ob
     }
 }
 
 /// calculates maximum number of entries we need to store and map
 int64_t
 Transients::EntryLimit()
 {
     // TODO: we should also check whether any SMP-aware caching is configured
     if (!UsingSmp() || !Config.onoff.collapsed_forwarding)
         return 0; // no SMP collapsed forwarding possible or needed
 
     return 16*1024; // TODO: make configurable?
 }
 
 /// initializes shared memory segment used by Transients
 class TransientsRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
     TransientsRr(): mapOwner(NULL) {}
-    virtual void run(const RunnerRegistry &);
+    virtual void useConfig();
     virtual ~TransientsRr();
 
 protected:
-    virtual void create(const RunnerRegistry &);
+    virtual void create();
 
 private:
     TransientsMap::Owner *mapOwner;
 };
 
-RunnerRegistrationEntry(rrAfterConfig, TransientsRr);
+RunnerRegistrationEntry(TransientsRr);
 
 void
-TransientsRr::run(const RunnerRegistry &r)
+TransientsRr::useConfig()
 {
     assert(Config.memShared.configured());
-    Ipc::Mem::RegisteredRunner::run(r);
+    Ipc::Mem::RegisteredRunner::useConfig();
 }
 
 void
-TransientsRr::create(const RunnerRegistry &)
+TransientsRr::create()
 {
     if (!Config.onoff.collapsed_forwarding)
         return;
 
     const int64_t entryLimit = Transients::EntryLimit();
     if (entryLimit <= 0)
         return; // no SMP configured or a misconfiguration
 
     Must(!mapOwner);
     mapOwner = TransientsMap::Init(MapLabel, entryLimit);
 }
 
 TransientsRr::~TransientsRr()
 {
     delete mapOwner;
 }

=== modified file 'src/base/RunnersRegistry.cc'
--- src/base/RunnersRegistry.cc	2012-01-20 18:55:04 +0000
+++ src/base/RunnersRegistry.cc	2014-02-20 00:57:34 +0000
@@ -1,57 +1,45 @@
 #include "squid.h"
 #include "base/RunnersRegistry.h"
-#include <list>
-#include <map>
+#include <set>
 
-typedef std::list<RegisteredRunner*> Runners;
-typedef std::map<RunnerRegistry, Runners*> Registries;
+/// a collection of unique runners, in no particular order
+typedef std::set<RegisteredRunner*> Runners;
+/// all known runners
+static Runners *TheRunners = NULL;
 
-/// all known registries
-static Registries *TheRegistries = NULL;
-
-/// returns the requested runners list, initializing structures as needed
+/// safely returns registered runners, initializing structures as needed
 static Runners &
-GetRunners(const RunnerRegistry &registryId)
+GetRunners()
 {
-    if (!TheRegistries)
-        TheRegistries = new Registries;
-
-    if (TheRegistries->find(registryId) == TheRegistries->end())
-        (*TheRegistries)[registryId] = new Runners;
-
-    return *(*TheRegistries)[registryId];
+    if (!TheRunners)
+        TheRunners = new Runners;
+    return *TheRunners;
 }
 
 int
-RegisterRunner(const RunnerRegistry &registryId, RegisteredRunner *rr)
+RegisterRunner(RegisteredRunner *rr)
 {
-    Runners &runners = GetRunners(registryId);
-    runners.push_back(rr);
+    Runners &runners = GetRunners();
+    runners.insert(rr);
     return runners.size();
 }
 
-int
-ActivateRegistered(const RunnerRegistry &registryId)
+void
+RunRegistered(const RegisteredRunner::Method &m)
 {
-    Runners &runners = GetRunners(registryId);
+    Runners &runners = GetRunners();
     typedef Runners::iterator RRI;
     for (RRI i = runners.begin(); i != runners.end(); ++i)
-        (*i)->run(registryId);
-    return runners.size();
-}
+        ((*i)->*m)();
 
-void
-DeactivateRegistered(const RunnerRegistry &registryId)
-{
-    Runners &runners = GetRunners(registryId);
-    while (!runners.empty()) {
-        delete runners.back();
-        runners.pop_back();
+    if (m == &RegisteredRunner::finishShutdown) {
+        delete TheRunners;
+        TheRunners = NULL;
     }
 }
 
 bool
 UseThisStatic(const void *)
 {
     return true;
 }

=== modified file 'src/base/RunnersRegistry.h'
--- src/base/RunnersRegistry.h	2012-08-28 13:00:30 +0000
+++ src/base/RunnersRegistry.h	2014-02-20 02:18:36 +0000
@@ -1,71 +1,95 @@
 #ifndef SQUID_BASE_RUNNERSREGISTRY_H
 #define SQUID_BASE_RUNNERSREGISTRY_H
 
 /**
- * This API allows virtually any module to register with a well-known registry,
- * be activated by some central processor at some registry-specific time, and
- * be deactiveated by some central processor at some registry-specific time.
+ * This API allows virtually any module to register its interest in receiving
+ * notification about initial configuration availability, configuration changes
+ * and other critical events in Squid lifetime without exposing the notifier
+ * to the details of the module.
  *
  * For example, main.cc may activate registered I/O modules after parsing
- * squid.conf and deactivate them before exiting.
+ * squid.conf and deactivate them before exiting, all without knowing what
+ * those I/O modules really are.
  *
  * A module in this context is code providing a functionality or service to the
- * rest of Squid, such as src/DiskIO/Blocking, src/fs/ufs, or Cache Manager. A
- * module must declare a RegisteredRunner child class to implement activation and
- * deactivation logic using the run() method and destructor, respectively.
+ * rest of Squid, such as src/DiskIO/Blocking, src/fs/ufs, or Cache Manager. To
+ * receive notifications, a module must declare a RegisteredRunner child class
+ * and implement the methods corresponding to the events the module is
+ * interested in.
  *
- * This API allows the registry to determine the right [de]activation time for
- * each group of similar modules, without knowing any module specifics.
+ * The order of events is documented in this header (where applicable), but
+ * the order in which runners are notified about a given event is undefined.
+ * If a specific notification order is required, split the event into two or
+ * more related event(s), documenting their relative order here.
  *
  */
 
-/// well-known registries
-typedef enum {
-    /// Managed by main.cc. Activated after parsing squid.conf and
-    /// deactivated before freeing configuration-related memory or exit()-ing.
+/// a runnable registrant API
+/// kids must override [only] the methods they are interested in
+class RegisteredRunner
+{
+public:
+    /* Related methods below are declared in their calling order */
+
+    /* Configuration events */
+
+    /// Called after parsing squid.conf.
     /// Meant for setting configuration options that depend on other
     /// configuration options and were not explicitly configured.
-    rrFinalizeConfig,
+    virtual void finalizeConfig() {}
 
-    /// Managed by main.cc. Activated after rrFinalizeConfig and
-    /// deactivated before rrFinalizeConfig. Meant for announcing
-    /// memory reservations before memory is allocated.
-    rrClaimMemoryNeeds,
-
-    /// Managed by main.cc. Activated after rrClaimMemoryNeeds and
-    /// deactivated before rrClaimMemoryNeeds. Meant for activating
-    /// modules and features based on the finalized configuration.
-    rrAfterConfig,
+    /// Called after finalizeConfig().
+    /// Meant for announcing memory reservations before memory is allocated.
+    virtual void claimMemoryNeeds() {}
 
-    rrEnd ///< not a real registry, just a label to mark the end of enum
-} RunnerRegistry;
+    /// Called after claimMemoryNeeds().
+    /// Meant for activating modules and features using a finalized
+    /// configuration with known memory requirements.
+    virtual void useConfig() {}
 
-/// a runnable registrant API
-class RegisteredRunner
-{
-public:
-    // called when this runner's registry is deactivated
+    /* Reconfiguration events */
+
+    /// Called after parsing squid.conf during reconfiguration.
+    /// Meant for adjusting the module state based on configuration changes.
+    virtual void syncConfig() {}
+
+    /* Shutdown events */
+
+    /// Called after receiving a shutdown request and before stopping the main
+    /// loop. At least one main loop iteration is guaranteed after this call.
+    /// Meant for cleanup and state saving that may require other modules.
+    virtual void startShutdown() {}
+
+    /// Called after stopping the main loop.
+    /// Meant for quick/basic cleanup that does not require any other modules.
     virtual ~RegisteredRunner() {}
+    /// exists to simplify caller interface; override the destructor instead
+    void finishShutdown() { delete this; }
+
+    /// a pointer to one of the above notification methods
+    typedef void (RegisteredRunner::*Method)();
 
-    // called when this runner's registry is activated
-    virtual void run(const RunnerRegistry &r) = 0;
 };
 
 /// registers a given runner with the given registry and returns registry count
-int RegisterRunner(const RunnerRegistry &registry, RegisteredRunner *rr);
+int RegisterRunner(RegisteredRunner *rr);
 
-/// calls run() methods of all runners in the given registry
-int ActivateRegistered(const RunnerRegistry &registry);
-/// deletes all runners in the given registry
-void DeactivateRegistered(const RunnerRegistry &registry);
+/// Calls a given method of all runners.
+/// All runners are destroyed after the finishShutdown() call.
+void RunRegistered(const RegisteredRunner::Method &m);
+
+/// convenience macro to describe/debug the caller and the method being called
+#define RunRegisteredHere(m) \
+    debugs(1, 2, "running " # m); \
+    RunRegistered(&m)
 
 /// convenience function to "use" an otherwise unreferenced static variable
 bool UseThisStatic(const void *);
 
 /// convenience macro: register one RegisteredRunner kid as early as possible
-#define RunnerRegistrationEntry(Registry, Who) \
-    static const bool Who ## _RegisteredWith_ ## Registry = \
-        RegisterRunner(Registry, new Who) > 0 && \
-        UseThisStatic(& Who ## _RegisteredWith_ ## Registry);
+#define RunnerRegistrationEntry(Who) \
+    static const bool Who ## _Registered_ = \
+        RegisterRunner(new Who) > 0 && \
+        UseThisStatic(& Who ## _Registered_);
 
 #endif /* SQUID_BASE_RUNNERSREGISTRY_H */

=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2014-02-10 16:39:10 +0000
+++ src/cache_cf.cc	2014-02-20 00:33:30 +0000
@@ -4589,48 +4589,49 @@ static void dump_sslproxy_cert_sign(Stor
 }
 
 static void free_sslproxy_cert_sign(sslproxy_cert_sign **cert_sign)
 {
     while (*cert_sign) {
         sslproxy_cert_sign *cs = *cert_sign;
         *cert_sign = cs->next;
 
         if (cs->aclList)
             aclDestroyAclList(&cs->aclList);
 
         safe_free(cs);
     }
 }
 
 class sslBumpCfgRr: public ::RegisteredRunner
 {
 public:
     static Ssl::BumpMode lastDeprecatedRule;
     /* RegisteredRunner API */
-    virtual void run(const RunnerRegistry &);
+    virtual void finalizeConfig();
 };
 
 Ssl::BumpMode sslBumpCfgRr::lastDeprecatedRule = Ssl::bumpEnd;
 
-RunnerRegistrationEntry(rrFinalizeConfig, sslBumpCfgRr);
+RunnerRegistrationEntry(sslBumpCfgRr);
 
-void sslBumpCfgRr::run(const RunnerRegistry &r)
+void
+sslBumpCfgRr::finalizeConfig()
 {
     if (lastDeprecatedRule != Ssl::bumpEnd) {
         assert( lastDeprecatedRule == Ssl::bumpClientFirst || lastDeprecatedRule == Ssl::bumpNone);
         static char buf[1024];
         if (lastDeprecatedRule == Ssl::bumpClientFirst) {
             strcpy(buf, "ssl_bump deny all");
             debugs(3, DBG_CRITICAL, "WARNING: auto-converting deprecated implicit "
                    "\"ssl_bump deny all\" to \"ssl_bump none all\". New ssl_bump configurations "
                    "must not use implicit rules. Update your ssl_bump rules.");
         } else {
             strcpy(buf, "ssl_bump allow all");
             debugs(3, DBG_CRITICAL, "SECURITY NOTICE: auto-converting deprecated implicit "
                    "\"ssl_bump allow all\" to \"ssl_bump client-first all\" which is usually "
                    "inferior to the newer server-first bumping mode. New ssl_bump"
                    " configurations must not use implicit rules. Update your ssl_bump rules.");
         }
         parse_line(buf);
     }
 }
 

=== modified file 'src/client_db.cc'
--- src/client_db.cc	2014-02-08 13:36:42 +0000
+++ src/client_db.cc	2014-02-20 00:14:03 +0000
@@ -107,46 +107,47 @@ clientdbAdd(const Ip::Address &addr)
     if ((statCounter.client_http.clients > max_clients) && !cleanup_running && cleanup_scheduled < 2) {
         ++cleanup_scheduled;
         eventAdd("client_db garbage collector", clientdbScheduledGC, NULL, 90, 0);
     }
 
     return c;
 }
 
 static void
 clientdbInit(void)
 {
     if (client_table)
         return;
 
     client_table = hash_create((HASHCMP *) strcmp, CLIENT_DB_HASH_SIZE, hash_string);
 }
 
 class ClientDbRr: public RegisteredRunner
 {
 public:
-    virtual void run(const RunnerRegistry &);
+    /* RegisteredRunner API */
+    virtual void useConfig();
 };
-RunnerRegistrationEntry(rrAfterConfig, ClientDbRr);
+RunnerRegistrationEntry(ClientDbRr);
 
 void
-ClientDbRr::run(const RunnerRegistry &r)
+ClientDbRr::useConfig()
 {
     clientdbInit();
     Mgr::RegisterAction("client_list", "Cache Client List", clientdbDump, 0, 1);
 }
 
 #if USE_DELAY_POOLS
 /* returns ClientInfo for given IP addr
    Returns NULL if no such client (or clientdb turned off)
    (it is assumed that clientdbEstablished will be called before and create client record if needed)
 */
 ClientInfo * clientdbGetInfo(const Ip::Address &addr)
 {
     char key[MAX_IPSTRLEN];
     ClientInfo *c;
 
     if (!Config.onoff.client_db)
         return NULL;
 
     addr.toStr(key,MAX_IPSTRLEN);
 

=== modified file 'src/fs/rock/RockSwapDir.cc'
--- src/fs/rock/RockSwapDir.cc	2013-12-31 18:49:41 +0000
+++ src/fs/rock/RockSwapDir.cc	2014-02-19 23:44:01 +0000
@@ -987,44 +987,44 @@ Rock::SwapDir::statfs(StoreEntry &e) con
 const char *
 Rock::SwapDir::inodeMapPath() const
 {
     static String inodesPath;
     inodesPath = path;
     inodesPath.append("_inodes");
     return inodesPath.termedBuf();
 }
 
 const char *
 Rock::SwapDir::freeSlotsPath() const
 {
     static String spacesPath;
     spacesPath = path;
     spacesPath.append("_spaces");
     return spacesPath.termedBuf();
 }
 
 namespace Rock
 {
-RunnerRegistrationEntry(rrAfterConfig, SwapDirRr);
+RunnerRegistrationEntry(SwapDirRr);
 }
 
-void Rock::SwapDirRr::create(const RunnerRegistry &)
+void Rock::SwapDirRr::create()
 {
     Must(mapOwners.empty() && freeSlotsOwners.empty());
     for (int i = 0; i < Config.cacheSwap.n_configured; ++i) {
         if (const Rock::SwapDir *const sd = dynamic_cast<Rock::SwapDir *>(INDEXSD(i))) {
             const int64_t capacity = sd->entryLimitAllowed();
 
             SwapDir::DirMap::Owner *const mapOwner =
                 SwapDir::DirMap::Init(sd->inodeMapPath(), capacity);
             mapOwners.push_back(mapOwner);
 
             // TODO: somehow remove pool id and counters from PageStack?
             Ipc::Mem::Owner<Ipc::Mem::PageStack> *const freeSlotsOwner =
                 shm_new(Ipc::Mem::PageStack)(sd->freeSlotsPath(),
                                              i+1, capacity,
                                              sizeof(DbCellHeader));
             freeSlotsOwners.push_back(freeSlotsOwner);
 
             // TODO: add method to initialize PageStack with no free pages
             while (true) {
                 Ipc::Mem::PageId pageId;

=== modified file 'src/fs/rock/RockSwapDir.h'
--- src/fs/rock/RockSwapDir.h	2014-02-10 16:39:10 +0000
+++ src/fs/rock/RockSwapDir.h	2014-02-19 23:44:28 +0000
@@ -126,30 +126,30 @@ private:
     DiskIOStrategy *io;
     RefCount<DiskFile> theFile; ///< cache storage for this cache_dir
     Ipc::Mem::Pointer<Ipc::Mem::PageStack> freeSlots; ///< all unused slots
     Ipc::Mem::PageId *waitingForPage; ///< one-page cache for a "hot" free slot
 
     /* configurable options */
     DiskFile::Config fileConfig; ///< file-level configuration options
 
     static const int64_t HeaderSize; ///< on-disk db header size
 };
 
 /// initializes shared memory segments used by Rock::SwapDir
 class SwapDirRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* ::RegisteredRunner API */
     virtual ~SwapDirRr();
 
 protected:
     /* Ipc::Mem::RegisteredRunner API */
-    virtual void create(const RunnerRegistry &);
+    virtual void create();
 
 private:
     std::vector<SwapDir::DirMap::Owner *> mapOwners;
     std::vector< Ipc::Mem::Owner<Ipc::Mem::PageStack> *> freeSlotsOwners;
 };
 
 } // namespace Rock
 
 #endif /* SQUID_FS_ROCK_SWAP_DIR_H */

=== modified file 'src/ipc/mem/Pages.cc'
--- src/ipc/mem/Pages.cc	2013-10-25 00:13:46 +0000
+++ src/ipc/mem/Pages.cc	2014-02-19 23:57:59 +0000
@@ -73,64 +73,64 @@ Ipc::Mem::NotePageNeed(const int purpose
 }
 
 size_t
 Ipc::Mem::PageLevel()
 {
     return ThePagePool ? ThePagePool->level() : 0;
 }
 
 size_t
 Ipc::Mem::PageLevel(const int purpose)
 {
     return ThePagePool ? ThePagePool->level(purpose) : 0;
 }
 
 /// initializes shared memory pages
 class SharedMemPagesRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
     SharedMemPagesRr(): owner(NULL) {}
-    virtual void run(const RunnerRegistry &);
-    virtual void create(const RunnerRegistry &);
-    virtual void open(const RunnerRegistry &);
+    virtual void useConfig();
+    virtual void create();
+    virtual void open();
     virtual ~SharedMemPagesRr();
 
 private:
     Ipc::Mem::PagePool::Owner *owner;
 };
 
-RunnerRegistrationEntry(rrAfterConfig, SharedMemPagesRr);
+RunnerRegistrationEntry(SharedMemPagesRr);
 
 void
-SharedMemPagesRr::run(const RunnerRegistry &r)
+SharedMemPagesRr::useConfig()
 {
     if (Ipc::Mem::PageLimit() <= 0)
         return;
 
-    Ipc::Mem::RegisteredRunner::run(r);
+    Ipc::Mem::RegisteredRunner::useConfig();
 }
 
 void
-SharedMemPagesRr::create(const RunnerRegistry &)
+SharedMemPagesRr::create()
 {
     Must(!owner);
     owner = Ipc::Mem::PagePool::Init(PagePoolId, Ipc::Mem::PageLimit(),
                                      Ipc::Mem::PageSize());
 }
 
 void
-SharedMemPagesRr::open(const RunnerRegistry &)
+SharedMemPagesRr::open()
 {
     Must(!ThePagePool);
     ThePagePool = new Ipc::Mem::PagePool(PagePoolId);
 }
 
 SharedMemPagesRr::~SharedMemPagesRr()
 {
     if (!UsingSmp())
         return;
 
     delete ThePagePool;
     ThePagePool = NULL;
     delete owner;
 }

=== modified file 'src/ipc/mem/Segment.cc'
--- src/ipc/mem/Segment.cc	2012-09-01 14:38:36 +0000
+++ src/ipc/mem/Segment.cc	2014-02-19 23:54:15 +0000
@@ -262,38 +262,38 @@ Ipc::Mem::Segment::open()
     theSize = segment.theSize;
 
     debugs(54, 3, HERE << "opened " << theName << " fake segment: " << theSize);
 }
 
 void
 Ipc::Mem::Segment::checkSupport(const char *const context)
 {
     if (!Enabled()) {
         debugs(54, 5, HERE << context <<
                ": True shared memory segments are not supported. "
                "Cannot fake shared segments in SMP config.");
         fatalf("Ipc::Mem::Segment: Cannot fake shared segments in SMP config (%s)\n",
                context);
     }
 }
 
 #endif // HAVE_SHM
 
 void
-Ipc::Mem::RegisteredRunner::run(const RunnerRegistry &r)
+Ipc::Mem::RegisteredRunner::useConfig()
 {
     // If Squid is built with real segments, we create() real segments
     // in the master process only.  Otherwise, we create() fake
     // segments in each worker process.  We assume that only workers
     // need and can work with fake segments.
 #if HAVE_SHM
     if (IamMasterProcess())
 #else
     if (IamWorkerProcess())
 #endif
-        create(r);
+        create();
 
     // we assume that master process does not need shared segments
     // unless it is also a worker
     if (!InDaemonMode() || !IamMasterProcess())
-        open(r);
+        open();
 }

=== modified file 'src/ipc/mem/Segment.h'
--- src/ipc/mem/Segment.h	2012-09-01 14:38:36 +0000
+++ src/ipc/mem/Segment.h	2014-02-19 23:45:16 +0000
@@ -55,35 +55,35 @@ private:
 
 #else // HAVE_SHM
 
     void checkSupport(const char *const context);
 
 #endif // HAVE_SHM
 
     const String theName; ///< shared memory segment file name
     void *theMem; ///< pointer to mmapped shared memory segment
     off_t theSize; ///< shared memory segment size
     off_t theReserved; ///< the total number of reserve()d bytes
     bool doUnlink; ///< whether the segment should be unlinked on destruction
 };
 
 /// Base class for runners that create and open shared memory segments.
 /// First may run create() method and then open().
 class RegisteredRunner: public ::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
-    virtual void run(const RunnerRegistry &r);
+    virtual void useConfig();
 
 protected:
     /// called when the runner should create a new memory segment
-    virtual void create(const RunnerRegistry &) = 0;
+    virtual void create() = 0;
     /// called when the runner should open a previously created segment,
     /// not needed if segments are opened in constructor or init methods
-    virtual void open(const RunnerRegistry &) {}
+    virtual void open() {}
 };
 
 } // namespace Mem
 
 } // namespace Ipc
 
 #endif /* SQUID_IPC_MEM_SEGMENT_H */

=== modified file 'src/main.cc'
--- src/main.cc	2014-01-24 01:57:15 +0000
+++ src/main.cc	2014-02-20 01:35:19 +0000
@@ -257,40 +257,42 @@ SignalEngine::checkEvents(int timeout)
 }
 
 void
 SignalEngine::doShutdown(time_t wait)
 {
     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");
 
     shutting_down = 1;
 
 #if USE_WIN32_SERVICE
     WIN32_svcstatusupdate(SERVICE_STOP_PENDING, (wait + 1) * 1000);
 #endif
 
     /* run the closure code which can be shared with reconfigure */
     serverConnectionsClose();
 #if USE_AUTH
     /* detach the auth components (only do this on full shutdown) */
     Auth::Scheme::FreeAll();
 #endif
+
+    RunRegisteredHere(RegisteredRunner::startShutdown);
     eventAdd("SquidShutdown", &StopEventLoop, this, (double) (wait + 1), 1, false);
 }
 
 static void
 usage(void)
 {
     fprintf(stderr,
             "Usage: %s [-cdhvzCFNRVYX] [-n name] [-s | -l facility] [-f config-file] [-[au] port] [-k signal]"
 #if USE_WIN32_SERVICE
             "[-ir] [-O CommandLine]"
 #endif
             "\n"
             "       -a port   Specify HTTP port number (default: %d).\n"
             "       -d level  Write debugging to stderr also.\n"
             "       -f file   Use given config-file instead of\n"
             "                 %s\n"
             "       -h        Print help message.\n"
 #if USE_WIN32_SERVICE
             "       -i        Installs as a Windows Service (see -n option).\n"
 #endif
@@ -787,40 +789,42 @@ mainReconfigureFinish(void *)
     errorClean();
     enter_suid();		/* root to read config file */
 
     // we may have disabled the need for PURGE
     if (Config2.onoff.enable_purge)
         Config2.onoff.enable_purge = 2;
 
     // parse the config returns a count of errors encountered.
     const int oldWorkers = Config.workers;
     if ( parseConfigFile(ConfigFile) != 0) {
         // for now any errors are a fatal condition...
         self_destruct();
     }
     if (oldWorkers != Config.workers) {
         debugs(1, DBG_CRITICAL, "WARNING: Changing 'workers' (from " <<
                oldWorkers << " to " << Config.workers <<
                ") requires a full restart. It has been ignored by reconfigure.");
         Config.workers = oldWorkers;
     }
 
+    RunRegisteredHere(RegisteredRunner::syncConfig);
+
     if (IamPrimaryProcess())
         CpuAffinityCheck();
     CpuAffinityReconfigure();
 
     setUmask(Config.umask);
     Mem::Report();
     setEffectiveUser();
     _db_init(Debug::cache_log, Debug::debugOptions);
     ipcache_restart();		/* clear stuck entries */
     fqdncache_restart();	/* sigh, fqdncache too */
     parseEtcHosts();
     errorInitialize();		/* reload error pages */
     accessLogInit();
 
 #if USE_LOADABLE_MODULES
     LoadableModulesConfigure(Config.loadable_module_names);
 #endif
 
 #if USE_ADAPTATION
     bool enableAdaptation = false;
@@ -1426,43 +1430,43 @@ SquidMain(int argc, char **argv)
     return 0;
 
 #endif
 
     /* send signal to running copy and exit */
     if (opt_send_signal != -1) {
         /* chroot if configured to run inside chroot */
         mainSetCwd();
         if (Config.chroot_dir) {
             no_suid();
         } else {
             leave_suid();
         }
 
         sendSignal();
         /* NOTREACHED */
     }
 
     debugs(1,2, HERE << "Doing post-config initialization\n");
     leave_suid();
-    ActivateRegistered(rrFinalizeConfig);
-    ActivateRegistered(rrClaimMemoryNeeds);
-    ActivateRegistered(rrAfterConfig);
+    RunRegisteredHere(RegisteredRunner::finalizeConfig);
+    RunRegisteredHere(RegisteredRunner::claimMemoryNeeds);
+    RunRegisteredHere(RegisteredRunner::useConfig);
     enter_suid();
 
     if (!opt_no_daemon && Config.workers > 0)
         watch_child(argv);
 
     if (opt_create_swap_dirs) {
         /* chroot if configured to run inside chroot */
         mainSetCwd();
 
         setEffectiveUser();
         debugs(0, DBG_CRITICAL, "Creating missing swap directories");
         Store::Root().create();
 
         return 0;
     }
 
     if (IamPrimaryProcess())
         CpuAffinityCheck();
     CpuAffinityInit();
 
@@ -1787,43 +1791,43 @@ watch_child(char *argv[])
                     syslog(LOG_NOTICE, "Squid Parent: %s process %d exited",
                            kid->name().termedBuf(), kid->getPid());
                 }
                 if (kid->hopeless()) {
                     syslog(LOG_NOTICE, "Squid Parent: %s process %d will not"
                            " be restarted due to repeated, frequent failures",
                            kid->name().termedBuf(), kid->getPid());
                 }
             } else {
                 syslog(LOG_NOTICE, "Squid Parent: unknown child process %d exited", pid);
             }
 #if _SQUID_NEXT_
         } while ((pid = wait3(&status, WNOHANG, NULL)) > 0);
 #else
         }
         while ((pid = waitpid(-1, &status, WNOHANG)) > 0);
 #endif
 
         if (!TheKids.someRunning() && !TheKids.shouldRestartSome()) {
             leave_suid();
-            DeactivateRegistered(rrAfterConfig);
-            DeactivateRegistered(rrClaimMemoryNeeds);
-            DeactivateRegistered(rrFinalizeConfig);
+            // XXX: Master process has no main loop and, hence, should not call
+            // RegisteredRunner::startShutdown which promises a loop iteration.
+            RunRegisteredHere(RegisteredRunner::finishShutdown);
             enter_suid();
 
             if (TheKids.someSignaled(SIGINT) || TheKids.someSignaled(SIGTERM)) {
                 syslog(LOG_ALERT, "Exiting due to unexpected forced shutdown");
                 exit(1);
             }
 
             if (TheKids.allHopeless()) {
                 syslog(LOG_ALERT, "Exiting due to repeated, frequent failures");
                 exit(1);
             }
 
             exit(0);
         }
 
         squid_signal(SIGINT, SIG_DFL, SA_RESTART);
         sleep(3);
     }
 
     /* NOTREACHED */
@@ -1885,77 +1889,76 @@ SquidShutdown()
     authenticateReset();
 #endif
 #if USE_WIN32_SERVICE
 
     WIN32_svcstatusupdate(SERVICE_STOP_PENDING, 10000);
 #endif
 
     Store::Root().sync(); /* Flush pending object writes/unlinks */
 
     unlinkdClose();	  /* after sync/flush. NOP if !USE_UNLINKD */
 
     storeDirWriteCleanLogs(0);
     PrintRusage();
     dumpMallocStats();
     Store::Root().sync();		/* Flush log writes */
     storeLogClose();
     accessLogClose();
     Store::Root().sync();		/* Flush log close */
     StoreFileSystem::FreeAllFs();
     DiskIOModule::FreeAllModules();
-    DeactivateRegistered(rrAfterConfig);
-    DeactivateRegistered(rrClaimMemoryNeeds);
-    DeactivateRegistered(rrFinalizeConfig);
 #if LEAK_CHECK_MODE && 0 /* doesn't work at the moment */
 
     configFreeMemory();
     storeFreeMemory();
     /*stmemFreeMemory(); */
     netdbFreeMemory();
     ipcacheFreeMemory();
     fqdncacheFreeMemory();
     asnFreeMemory();
     clientdbFreeMemory();
     httpHeaderCleanModule();
     statFreeMemory();
     eventFreeMemory();
     mimeFreeMemory();
     errorClean();
 #endif
 #if !XMALLOC_TRACE
 
     if (opt_no_daemon) {
         file_close(0);
         file_close(1);
         file_close(2);
     }
 
 #endif
     // clear StoreController
     Store::Root(NULL);
 
     fdDumpOpen();
 
     comm_exit();
 
     memClean();
 
+    RunRegisteredHere(RegisteredRunner::finishShutdown);
+
 #if XMALLOC_TRACE
 
     xmalloc_find_leaks();
 
     debugs(1, DBG_CRITICAL, "Memory used after shutdown: " << xmalloc_total);
 
 #endif
 #if MEM_GEN_TRACE
 
     log_trace_done();
 
 #endif
 
     if (IamPrimaryProcess()) {
         if (Config.pidFilename && strcmp(Config.pidFilename, "none") != 0) {
             enter_suid();
             safeunlink(Config.pidFilename, 0);
             leave_suid();
         }
     }

=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc	2014-02-08 13:36:42 +0000
+++ src/ssl/support.cc	2014-02-20 00:08:09 +0000
@@ -1822,56 +1822,56 @@ Ssl::initialize_session_cache()
     }
 
     for (AnyP::PortCfg *s = ::Config.Sockaddr.http; s; s = s->next) {
         if (s->staticSslContext.get() != NULL)
             setSessionCallbacks(s->staticSslContext.get());
     }
 }
 
 void
 destruct_session_cache()
 {
     delete SslSessionCache;
 }
 
 /// initializes shared memory segments used by MemStore
 class SharedSessionCacheRr: public Ipc::Mem::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
     SharedSessionCacheRr(): owner(NULL) {}
-    virtual void run(const RunnerRegistry &);
+    virtual void useConfig();
     virtual ~SharedSessionCacheRr();
 
 protected:
-    virtual void create(const RunnerRegistry &);
+    virtual void create();
 
 private:
     Ipc::MemMap::Owner *owner;
 };
 
-RunnerRegistrationEntry(rrAfterConfig, SharedSessionCacheRr);
+RunnerRegistrationEntry(SharedSessionCacheRr);
 
 void
-SharedSessionCacheRr::run(const RunnerRegistry &r)
+SharedSessionCacheRr::useConfig()
 {
-    Ipc::Mem::RegisteredRunner::run(r);
+    Ipc::Mem::RegisteredRunner::useConfig();
 }
 
 void
-SharedSessionCacheRr::create(const RunnerRegistry &)
+SharedSessionCacheRr::create()
 {
     if (!isSslServer()) //no need to configure ssl session cache.
         return;
 
     int items;
     items = Config.SSL.sessionCacheSize / sizeof(Ipc::MemMap::Slot);
     if (items)
         owner =  Ipc::MemMap::Init(SslSessionCacheName, items);
 }
 
 SharedSessionCacheRr::~SharedSessionCacheRr()
 {
     delete owner;
 }
 
 #endif /* USE_SSL */

=== modified file 'src/tests/testRock.cc'
--- src/tests/testRock.cc	2014-02-08 13:36:42 +0000
+++ src/tests/testRock.cc	2014-02-20 02:35:21 +0000
@@ -65,55 +65,56 @@ testRock::setUp()
 
     commonInit();
 
     char *path=xstrdup(TESTDIR);
 
     char *config_line=xstrdup("10 max-size=16384");
 
     ConfigParser::SetCfgLine(config_line);
 
     store->parse(0, path);
     store_maxobjsize = 1024*1024*2;
 
     safe_free(path);
 
     safe_free(config_line);
 
     /* ok, ready to create */
     store->create();
 
     rr = new Rock::SwapDirRr;
-    rr->run(rrAfterConfig);
+    rr->useConfig();
 }
 
 void
 testRock::tearDown()
 {
     CPPUNIT_NS::TestFixture::tearDown();
 
     Store::Root(NULL);
 
     store = NULL;
 
     free_cachedir(&Config.cacheSwap);
 
-    delete rr;
+    rr->finishShutdown(); // deletes rr
+    rr = NULL;
 
     // TODO: do this once, or each time.
     // safe_free(Config.replPolicy->type);
     // delete Config.replPolicy;
 
     if (0 > system ("rm -rf " TESTDIR))
         throw std::runtime_error("Failed to clean test work directory");
 }
 
 void
 testRock::commonInit()
 {
     static bool inited = false;
 
     if (inited)
         return;
 
     StoreFileSystem::SetupAllFs();
 
     Config.Store.avgObjectSize = 1024;

Reply via email to