From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 7 Jan 2013 17:18:10 +0000 (-0700)
Subject: Split lock() into "just lock" and "update entry reference time" interfaces.
X-Git-Tag: SQUID_3_5_0_1~444^2~88
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=189949922411cc0519ff16b7a0c962be19c49aae;p=thirdparty%2Fsquid.git

Split lock() into "just lock" and "update entry reference time" interfaces.
Improved entry lock/unlock debugging.

Several modern store entry users need to lock the entry for storage rather
then to update entry's reference time. There is an old XXX describing the
corresponding API bug that merges the two distinct operations. This change
does not modify existing code, but allows new code to use new/fixed API
instead of changing an essentially private entry lock counter.
---

diff --git a/src/Store.h b/src/Store.h
index 44c1fda799..a2a1b9ffcb 100644
--- a/src/Store.h
+++ b/src/Store.h
@@ -195,13 +195,27 @@ public:
     virtual void buffer();
     /** flush any buffered content */
     virtual void flush();
-    /** reduce the memory lock count on the entry */
-    virtual int unlock();
-    /** increate the memory lock count on the entry */
     virtual int64_t objectLen() const;
     virtual int64_t contentLen() const;
 
-    virtual void lock();
+    /** deprecated: lock() in anonymous context folowed by touch()
+     * RBC 20050104 this is wrong- memory ref counting
+     * is not at all equivalent to the store 'usage' concept
+     * which the replacement policies should be acting upon.
+     * specifically, object iteration within stores needs
+     * memory ref counting to prevent race conditions,
+     * but this should not influence store replacement.
+     */
+    void lock() { lock("somebody"); touch(); }
+
+    /// claim shared ownership of this entry (for use in a given context)
+    void lock(const char *context);
+    /// disclaim shared ownership; may remove entry from store and delete it
+    /// returns remaning lock level (zero for unlocked and possibly gone entry)
+    int unlock(const char *context = "somebody");
+    /// update last reference timestamp and related Store metadata
+    void touch();
+
     virtual void release();
 
 #if USE_ADAPTATION
diff --git a/src/store.cc b/src/store.cc
index 79ff65185f..a922cae909 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -514,20 +514,15 @@ StoreEntry::purgeMem()
         release();
 }
 
-/* RBC 20050104 this is wrong- memory ref counting
- * is not at all equivalent to the store 'usage' concept
- * which the replacement policies should be acting upon.
- * specifically, object iteration within stores needs
- * memory ref counting to prevent race conditions,
- * but this should not influence store replacement.
- */
 void
-
-StoreEntry::lock()
+StoreEntry::lock(const char *context)
 {
     ++lock_count;
-    debugs(20, 3, "StoreEntry::lock: key '" << getMD5Text() <<"' count=" <<
-           lock_count  );
+    debugs(20, 3, context << " locked key " << getMD5Text() << ' ' << *this);
+}
+
+void
+StoreEntry::touch() {
     lastref = squid_curtime;
     Store::Root().reference(*this);
 }
@@ -561,13 +556,13 @@ StoreEntry::releaseRequest()
     setPrivateKey();
 }
 
-/* unlock object, return -1 if object get released after unlock
- * otherwise lock_count */
 int
-StoreEntry::unlock()
+StoreEntry::unlock(const char *context)
 {
+	
+    debugs(20, 3, (context ? context : "somebody") <<
+           " unlocking key " << getMD5Text() << ' ' << *this);
     --lock_count;
-    debugs(20, 3, "StoreEntry::unlock: key '" << getMD5Text() << "' count=" << lock_count);
 
     if (lock_count)
         return (int) lock_count;
@@ -2033,7 +2028,7 @@ std::ostream &operator <<(std::ostream &os, const StoreEntry &e)
 {
     return os << e.swap_filen << '@' << e.swap_dirn << '=' <<
            e.mem_status << '/' << e.ping_status << '/' << e.store_status << '/' <<
-           e.swap_status;
+           e.swap_status << '*' << e.lock_count;
 }
 
 /* NullStoreEntry */