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 44c1fda7995833607d8c849a3aca8aef3e7e3235..a2a1b9ffcb3ab46392913f368af7d24e9e9a4e35 100644 (file)
--- 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 79ff65185fbb062c2c1938d37570a8b5b4a51806..a922cae909fd36a41a4bc24b03a82213e8129a1f 100644 (file)
--- 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 */