From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 29 Feb 2012 00:42:21 +0000 (-0700)
Subject: Bug 3458: Icon Serving (squid-internal-static) Broken.
X-Git-Tag: BumpSslServerFirst.take05
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=db9b65ff83911427522006756739e7e92bf52aa0;p=thirdparty%2Fsquid.git

Bug 3458: Icon Serving (squid-internal-static) Broken.

The "trimMemory for unswappable objects" fix (trunk r11969,r11970) exposed
ENTRY_SPECIAL objects such as icons to memory trimming which they cannot
handle because Squid does not reload the missing parts of special objects.

Further testing showed that a special object may be cached in shared RAM
and/or might even be purged from all caches completely. This may explain why
icons eventually "disappear" in some environments.

We now treat special objects as not belonging to memory or disk Stores and do
not ask those Stores to manage them. This separation needs more work, but it
passes basic tests, and it is the right step towards creating a proper storage
dedicated to those objects.
---

diff --git a/src/store.cc b/src/store.cc
index 1c9a9e18a0..eb55af882d 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -1637,7 +1637,8 @@ StoreEntry::setMemStatus(mem_status_t new_status)
 
     // are we using a shared memory cache?
     if (Config.memShared && IamWorkerProcess()) {
-        assert(new_status != IN_MEMORY); // we do not call this otherwise
+        // enumerate calling cases if shared memory is enabled
+        assert(new_status != IN_MEMORY || EBIT_TEST(flags, ENTRY_SPECIAL));
         // This method was designed to update replacement policy, not to
         // actually purge something from the memory cache (TODO: rename?).
         // Shared memory cache does not have a policy that needs updates.
@@ -1902,6 +1903,9 @@ StoreEntry::trimMemory(const bool preserveSwappable)
     if (mem_status == IN_MEMORY)
         return;
 
+    if (EBIT_TEST(flags, ENTRY_SPECIAL))
+        return; // cannot trim because we do not load them again
+
     if (!preserveSwappable) {
         if (mem_obj->policyLowestOffsetToKeep(0) == 0) {
             /* Nothing to do */
diff --git a/src/store_dir.cc b/src/store_dir.cc
index 5303ce3864..2d503c84b9 100644
--- a/src/store_dir.cc
+++ b/src/store_dir.cc
@@ -685,6 +685,10 @@ free_cachedir(SquidConfig::_cacheSwap * swap)
 void
 StoreController::reference(StoreEntry &e)
 {
+    // special entries do not belong to any specific Store, but are IN_MEMORY
+    if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
+        return;
+
     /* Notify the fs that we're referencing this object again */
 
     if (e.swap_dirn > -1)
@@ -706,6 +710,10 @@ StoreController::dereference(StoreEntry & e)
 {
     bool keepInStoreTable = true; // keep if there are no objections
 
+    // special entries do not belong to any specific Store, but are IN_MEMORY
+    if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
+        return keepInStoreTable;
+
     /* Notify the fs that we're not referencing this object any more */
 
     if (e.swap_filen > -1)
@@ -777,6 +785,13 @@ void
 StoreController::handleIdleEntry(StoreEntry &e)
 {
     bool keepInLocalMemory = false;
+
+    if (EBIT_TEST(e.flags, ENTRY_SPECIAL)) {
+        // Icons (and cache digests?) should stay in store_table until we
+        // have a dedicated storage for them (that would not purge them).
+        // They are not managed [well] by any specific Store handled below.
+        keepInLocalMemory = true;
+    } else
     if (memStore) {
         memStore->considerKeeping(e);
         // leave keepInLocalMemory false; memStore maintains its own cache
@@ -794,6 +809,8 @@ StoreController::handleIdleEntry(StoreEntry &e)
         return;
     }
 
+    debugs(20, 5, HERE << "keepInLocalMemory: " << keepInLocalMemory);
+
     // TODO: move this into [non-shared] memory cache class when we have one
     if (keepInLocalMemory) {
         e.setMemStatus(IN_MEMORY);