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 f3f31af3b33a9a02de9de20a8c79ca50333b0d5a..333fcb761dd30f9cacf339e5357558dffd097ef9 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 24d7cda499ff05a1cc1ce695c624278d1ea0cebf..9271b9a0efb65070f7a80fd797a0c5dafd81145c 100644 (file)
--- a/src/main.cc
+++ b/src/main.cc
@@ -1418,6 +1418,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();
@@ -1789,6 +1790,7 @@ watch_child(char *argv[])
             leave_suid();
             DeactivateRegistered(rrAfterConfig);
             DeactivateRegistered(rrClaimMemoryNeeds);
+            DeactivateRegistered(rrFinalizeConfig);
             enter_suid();
 
             if (TheKids.someSignaled(SIGINT) || TheKids.someSignaled(SIGTERM)) {
@@ -1885,6 +1887,7 @@ SquidShutdown()
     DiskIOModule::FreeAllModules();
     DeactivateRegistered(rrAfterConfig);
     DeactivateRegistered(rrClaimMemoryNeeds);
+    DeactivateRegistered(rrFinalizeConfig);
 #if LEAK_CHECK_MODE && 0 /* doesn't work at the moment */
 
     configFreeMemory();