Do not release entries that should be kept in local memory cache.

Since r11969, Squid calls trimMemory() for all entries, including
non-swappable, to release unused MemObjects memory.  But it should not
release objects that are or should be stored in local memory cache.
StoreEntry::trimMemory() has a check for IN_MEMORY status for that.
But IN_MEMORY status is set too late in
StoreController::handleIdleEntry(), after trimMemory() marks entry for
release:

  clientReplyContext::removeStoreReference()

    storeUnregister()
      StoreEntry::swapOut()
        StoreEntry::trimMemory()
          StoreEntry::releaseRequest()

    StoreEntry::unlock()
      StoreController::handleIdleEntry() // never get here because entry is
        set IN_MEMORY status             // already marked for release

The patch adds StoreEntry::keepInLocalMemory() method to check if an
entry should be stored in local memory cache.  If it is true, do not
release entry in trimMemory().

=== modified file 'src/Store.h'
--- src/Store.h	2012-01-13 13:49:26 +0000
+++ src/Store.h	2012-07-05 01:23:03 +0000
@@ -101,40 +101,42 @@ public:
     void makePrivate();
     void setPublicKey();
     void setPrivateKey();
     void expireNow();
     void releaseRequest();
     void negativeCache();
     void cacheNegatively();		/** \todo argh, why both? */
     void invokeHandlers();
     void purgeMem();
     void cacheInMemory(); ///< start or continue storing in memory cache
     void swapOut();
     /// whether we are in the process of writing this entry to disk
     bool swappingOut() const { return swap_status == SWAPOUT_WRITING; }
     void swapOutFileClose(int how);
     const char *url() const;
     int checkCachable();
     int checkNegativeHit() const;
     int locked() const;
     int validToSend() const;
     bool memoryCachable() const; ///< may be cached in memory
+    /// may be cached in memory and local memory cache is not overflowing
+    bool keepInLocalMemory() const;
     void createMemObject(const char *, const char *);
     void hideMemObject(); ///< no mem_obj for callers until createMemObject
     void dump(int debug_lvl) const;
     void hashDelete();
     void hashInsert(const cache_key *);
     void registerAbort(STABH * cb, void *);
     void reset();
     void setMemStatus(mem_status_t);
     void timestampsSet();
     void unregisterAbort();
     void destroyMemObject();
     int checkTooSmall();
 
     void delayAwareRead(const Comm::ConnectionPointer &conn, char *buf, int len, AsyncCall::Pointer callback);
 
     void setNoDelay (bool const);
     bool modifiedSince(HttpRequest * request) const;
     /// has ETag matching at least one of the If-Match etags
     bool hasIfMatchEtag(const HttpRequest &request) const;
     /// has ETag matching at least one of the If-None-Match etags

=== modified file 'src/store.cc'
--- src/store.cc	2012-01-13 13:49:26 +0000
+++ src/store.cc	2012-07-05 00:30:45 +0000
@@ -1446,40 +1446,46 @@ StoreEntry::memoryCachable() const
     if (mem_obj->data_hdr.size() == 0)
         return 0;
 
     if (mem_obj->inmem_lo != 0)
         return 0;
 
     if (!Config.onoff.memory_cache_first && swap_status == SWAPOUT_DONE && refcount == 1)
         return 0;
 
     if (Config.memShared && IamWorkerProcess()) {
         const int64_t expectedSize = mem_obj->expectedReplySize();
         // objects of unknown size are not allowed into memory cache, for now
         if (expectedSize < 0 ||
                 expectedSize > static_cast<int64_t>(Config.Store.maxInMemObjSize))
             return 0;
     }
 
     return 1;
 }
 
+bool
+StoreEntry::keepInLocalMemory() const
+{
+    return memoryCachable() && (mem_node::InUseCount() <= store_pages_max);
+}
+
 int
 StoreEntry::checkNegativeHit() const
 {
     if (!EBIT_TEST(flags, ENTRY_NEGCACHED))
         return 0;
 
     if (expires <= squid_curtime)
         return 0;
 
     if (store_status != STORE_OK)
         return 0;
 
     return 1;
 }
 
 /**
  * Set object for negative caching.
  * Preserves any expiry information given by the server.
  * In absence of proper expiry info it will set to expire immediately,
  * or with HTTP-violations enabled the configured negative-TTL is observed
@@ -1882,41 +1888,41 @@ StoreEntry::startWriting()
 char const *
 StoreEntry::getSerialisedMetaData()
 {
     StoreMeta *tlv_list = storeSwapMetaBuild(this);
     int swap_hdr_sz;
     char *result = storeSwapMetaPack(tlv_list, &swap_hdr_sz);
     storeSwapTLVFree(tlv_list);
     assert (swap_hdr_sz >= 0);
     mem_obj->swap_hdr_sz = (size_t) swap_hdr_sz;
     return result;
 }
 
 void
 StoreEntry::trimMemory(const bool preserveSwappable)
 {
     /*
      * DPW 2007-05-09
      * Bug #1943.  We must not let go any data for IN_MEMORY
      * objects.  We have to wait until the mem_status changes.
      */
-    if (mem_status == IN_MEMORY)
+    if (mem_status == IN_MEMORY || keepInLocalMemory())
         return;
 
     if (!preserveSwappable) {
         if (mem_obj->policyLowestOffsetToKeep(0) == 0) {
             /* Nothing to do */
             return;
         }
         /*
          * Its not swap-able, and we're about to delete a chunk,
          * so we must make it PRIVATE.  This is tricky/ugly because
          * for the most part, we treat swapable == cachable here.
          */
         releaseRequest();
         mem_obj->trimUnSwappable ();
     } else {
         mem_obj->trimSwappable ();
     }
 }
 
 bool

=== modified file 'src/store_dir.cc'
--- src/store_dir.cc	2011-10-14 16:21:48 +0000
+++ src/store_dir.cc	2012-07-05 00:29:14 +0000
@@ -763,45 +763,42 @@ StoreController::get(const cache_key *ke
     }
 
     debugs(20, 4, HERE << "none of " << Config.cacheSwap.n_configured <<
            " cache_dirs have " << storeKeyText(key));
     return NULL;
 }
 
 void
 StoreController::get(String const key, STOREGETCLIENT aCallback, void *aCallbackData)
 {
     fatal("not implemented");
 }
 
 void
 StoreController::handleIdleEntry(StoreEntry &e)
 {
     bool keepInLocalMemory = false;
     if (memStore) {
         memStore->considerKeeping(e);
         // leave keepInLocalMemory false; memStore maintains its own cache
-    } else {
-        keepInLocalMemory = e.memoryCachable() && // entry is in good shape and
-                            // the local memory cache is not overflowing
-                            (mem_node::InUseCount() <= store_pages_max);
-    }
+    } else
+        keepInLocalMemory = e.keepInLocalMemory();
 
     // An idle, unlocked entry that belongs to a SwapDir which controls
     // its own index, should not stay in the global store_table.
     if (!dereference(e)) {
         debugs(20, 5, HERE << "destroying unlocked entry: " << &e << ' ' << e);
         destroyStoreEntry(static_cast<hash_link*>(&e));
         return;
     }
 
     // TODO: move this into [non-shared] memory cache class when we have one
     if (keepInLocalMemory) {
         e.setMemStatus(IN_MEMORY);
         e.mem_obj->unlinkRequest();
     } else {
         e.purgeMem(); // may free e
     }
 }
 
 StoreHashIndex::StoreHashIndex()
 {