Remove bogus "found KEY_PRIVATE" WARNINGs (#862)

... triggered by private bumped StoreEntry unlock()ed in ~ServerBump().

The WARNINGs were added long time ago (commit fc8b9fc) because, AFAICT,
earlier Store code expected StoreEntry owners to release() uncachable
entries, including KEY_PRIVATE ones, right before unlocking them.
However, there is no compile-time enforcement of that expectation, and
unlocking code does not always know whether the entry is cachable (as
ServerBump constructor/destructor RAII code illustrates).

This change stops tying release and unlocking decisions/actions together
but makes sure that idle KEY_PRIVATE entries are still released (because
we do not want to index unneeded/unusable Store entries).

diff --git a/src/clients/FtpRelay.cc b/src/clients/FtpRelay.cc
index 13056db48e44d634073938434cdf3d9c32dcbdd3..b18dce21f57f992a274dfc859aff735d550ff28d 100644 (file)
--- a/src/clients/FtpRelay.cc
+++ b/src/clients/FtpRelay.cc
@@ -160,8 +160,8 @@ Ftp::Relay::Relay(FwdState *const fwdState):
     savedReply.lastReply = NULL;
     savedReply.replyCode = 0;
 
-    // Nothing we can do at request creation time can mark the response as
-    // uncachable, unfortunately. This prevents "found KEY_PRIVATE" WARNINGs.
+    // Prevent the future response from becoming public and being shared/cached
+    // because FTP does not support response cachability and freshness checks.
     entry->releaseRequest();
     AsyncCall::Pointer call = asyncCall(9, 4, "Ftp::Relay::Abort", cbdataDialer(&Relay::HandleStoreAbort, this));
     entry->registerAbortCallback(call);

diff --git a/src/store.cc b/src/store.cc
index 6cb14e49344074bda1870b501ac8073fc963e92b..61266c2ee4d36f7951b641dfd4b2bb579e681a25 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -512,9 +512,6 @@ StoreEntry::doAbandon(const char *context)
         return;
     }
 
-    if (EBIT_TEST(flags, KEY_PRIVATE))
-        debugs(20, DBG_IMPORTANT, "WARNING: " << __FILE__ << ":" << __LINE__ << ": found KEY_PRIVATE");
-
     Store::Root().handleIdleEntry(*this); // may delete us
 }
 

diff --git a/src/store/Controller.cc b/src/store/Controller.cc
index f40eeab29a66f0a8d0478fe42607d01ef680d662..03c2fc76803fbc5bb1394b1eb6bdd949453a0429 100644 (file)
--- a/src/store/Controller.cc
+++ b/src/store/Controller.cc
@@ -276,6 +276,10 @@ Store::Controller::dereferenceIdle(StoreEntry &e, bool wantsLocalMemory)
     if (EBIT_TEST(e.flags, ENTRY_SPECIAL))
         return true;
 
+    // idle private entries cannot be reused
+    if (EBIT_TEST(e.flags, KEY_PRIVATE))
+        return false;
+
     bool keepInStoreTable = false; // keep only if somebody needs it there
 
     // Notify the fs that we are not referencing this object any more. This
@@ -703,6 +707,9 @@ Store::Controller::handleIdleEntry(StoreEntry &e)
 
     debugs(20, 5, HERE << "keepInLocalMemory: " << keepInLocalMemory);
 
+    // formerly known as "WARNING: found KEY_PRIVATE"
+    assert(!EBIT_TEST(e.flags, KEY_PRIVATE));
+
     // TODO: move this into [non-shared] memory cache class when we have one
     if (keepInLocalMemory) {
         e.setMemStatus(IN_MEMORY);