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 a0b6300de6c78c3599b50d5693369e6df84ee0fe..5bf7cd3880a54e3c3bbd1f8565b62b892dc2eb87 100644 (file)
--- 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 2d5b84048d12fa1751c386283672f7157fd1bf8f..2c08d243d8b4b9335575c10451ac1dd69c2428f6 100644 (file)
--- 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 b4a3c5df6e853cd65ff21360c2a5b66cb4dfa89b..4d781461cde4c2a1420b97ee46ed94ee3842f68b 100644 (file)
--- 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();