From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Thu, 26 Jan 2012 14:52:35 +0000 (-0700)
Subject: Bug 3449: shm_open failed (part 4: fixing memory_cache_shared defaults)
X-Git-Tag: BumpSslServerFirst.take05~12^2~71
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=45e8762c19e750bc25ab9a53d075de4d810d7736;p=thirdparty%2Fsquid.git

Bug 3449: shm_open failed (part 4: fixing memory_cache_shared defaults)

Properly initialize memory_cache_shared when not explicitly configured and
do not allocate shared memory segments if memory_cache_shared is off.

Prior to this change, shared memory was requested (during rrClaimMemoryNeeds)
before memory_cache_shared was initialized (during rrAfterConfig, unless
explicitly set in squid.conf). Moreover, MemStore::EntryLimit() calculation
ignored memory_cache_shared setting. All that resulted in an attempt to create
shared memory segments where none were needed or possible.

We now have a new Runners Registry (rrFinalizeConfig) that is dedicated to
finalizing complex configuration options, before those configurations options
are used to initialize modules, features, and services. As far as shared
memory is concerned, the initialization order is now:

  * rrFinalizeConfig: finalize memory_cache_shared if needed
  * rrClaimMemoryNeeds: request shared memory pages for the caches
  * rrAfterConfig: create shared memory segments

The same bug 3449 also needed a fix for shared segment paths (trunk r11961).
---

diff --git a/src/MemStore.cc b/src/MemStore.cc
index a0b6300de6..5bf7cd3880 100644
--- a/src/MemStore.cc
+++ b/src/MemStore.cc
@@ -347,7 +347,7 @@ MemStore::cleanReadable(const sfileno fileno)
 int64_t
 MemStore::EntryLimit()
 {
-    if (!Config.memMaxSize)
+    if (!Config.memShared || !Config.memMaxSize)
         return 0; // no memory cache configured
 
     const int64_t entrySize = Ipc::Mem::PageSize(); // for now
@@ -374,26 +374,17 @@ MemStoreClaimMemoryNeedsRr::run(const RunnerRegistry &)
 }
 
 
-/// initializes shared memory segments used by MemStore
-class MemStoreRr: public Ipc::Mem::RegisteredRunner
+/// decides whether to use a shared memory cache or checks its configuration
+class MemStoreCfgRr: public ::RegisteredRunner
 {
 public:
     /* RegisteredRunner API */
-    MemStoreRr(): owner(NULL) {}
     virtual void run(const RunnerRegistry &);
-    virtual ~MemStoreRr();
-
-protected:
-    virtual void create(const RunnerRegistry &);
-
-private:
-    MemStoreMap::Owner *owner;
 };
 
-RunnerRegistrationEntry(rrAfterConfig, MemStoreRr);
-
+RunnerRegistrationEntry(rrFinalizeConfig, MemStoreCfgRr);
 
-void MemStoreRr::run(const RunnerRegistry &r)
+void MemStoreCfgRr::run(const RunnerRegistry &r)
 {
     // decide whether to use a shared memory cache if the user did not specify
     if (!Config.memShared.configured()) {
@@ -409,7 +400,31 @@ void MemStoreRr::run(const RunnerRegistry &r)
         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(): owner(NULL) {}
+    virtual void run(const RunnerRegistry &);
+    virtual ~MemStoreRr();
+
+protected:
+    virtual void create(const RunnerRegistry &);
 
+private:
+    MemStoreMap::Owner *owner;
+};
+
+RunnerRegistrationEntry(rrAfterConfig, MemStoreRr);
+
+
+void MemStoreRr::run(const RunnerRegistry &r)
+{
+    assert(Config.memShared.configured());
     Ipc::Mem::RegisteredRunner::run(r);
 }
 
diff --git a/src/base/RunnersRegistry.h b/src/base/RunnersRegistry.h
index 2d5b84048d..2c08d243d8 100644
--- a/src/base/RunnersRegistry.h
+++ b/src/base/RunnersRegistry.h
@@ -21,13 +21,20 @@
 
 /// well-known registries
 typedef enum {
-    /// managed by main.cc; activated after parsing squid.conf but
-    /// before rrAfterConfig, deactivated after rrAfterConfig but
-    /// before freeing configuration-related memory or exit()-ing
+    /// Managed by main.cc. Activated after parsing squid.conf and
+    /// deactivated before freeing configuration-related memory or exit()-ing.
+    /// Meant for setting configuration options that depend on other
+    /// configuration options and were not explicitly configured.
+    rrFinalizeConfig,
+
+    /// 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 parsing squid.conf and
-    /// deactivated before freeing configuration-related memory or exit()-ing
+    /// Managed by main.cc. Activated after rrClaimMemoryNeeds and
+    /// deactivated before rrClaimMemoryNeeds. Meant for activating
+    /// modules and features based on the finalized configuration.
     rrAfterConfig,
 
     rrEnd ///< not a real registry, just a label to mark the end of enum
diff --git a/src/main.cc b/src/main.cc
index b4a3c5df6e..4d781461cd 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -1419,6 +1419,7 @@ SquidMain(int argc, char **argv)
 
     debugs(1,2, HERE << "Doing post-config initialization\n");
     leave_suid();
+    ActivateRegistered(rrFinalizeConfig);
     ActivateRegistered(rrClaimMemoryNeeds);
     ActivateRegistered(rrAfterConfig);
     enter_suid();
@@ -1790,6 +1791,7 @@ watch_child(char *argv[])
             leave_suid();
             DeactivateRegistered(rrAfterConfig);
             DeactivateRegistered(rrClaimMemoryNeeds);
+            DeactivateRegistered(rrFinalizeConfig);
             enter_suid();
 
             if (TheKids.someSignaled(SIGINT) || TheKids.someSignaled(SIGTERM)) {
@@ -1886,6 +1888,7 @@ SquidShutdown()
     DiskIOModule::FreeAllModules();
     DeactivateRegistered(rrAfterConfig);
     DeactivateRegistered(rrClaimMemoryNeeds);
+    DeactivateRegistered(rrFinalizeConfig);
 #if LEAK_CHECK_MODE && 0 /* doesn't work at the moment */
 
     configFreeMemory();