Broadcast mem-cache writer departure to transient readers (in more/all cases).

Moved transientsAbandon() call to MemStore::disconnect() to make sure we
catch all cases where a mem-cache writer stops updating the cache entry.
Transient readers need to know so that they do not get stuck when a writer
disappears.

transientsAbandon() needs StoreEntry so MemStore::disconnect requires one now.

diff --git a/src/MemObject.cc b/src/MemObject.cc
index 65686d49e8d21253a837c07207d9813981565606..ba4c32be71ba3659a433b06dd3a8337c07dd97e9 100644 (file)
--- a/src/MemObject.cc
+++ b/src/MemObject.cc
@@ -135,12 +135,6 @@ MemObject::~MemObject()
 #endif
 
     if (!shutting_down) { // Store::Root() is FATALly missing during shutdown
-        // TODO: Consider moving these to destroyMemoryObject
-        if (xitTable.index >= 0)
-            Store::Root().transientsDisconnect(*this);
-        if (memCache.index >= 0)
-            Store::Root().memoryDisconnect(*this);
-
         assert(xitTable.index < 0);
         assert(memCache.index < 0);
         assert(swapout.sio == NULL);

diff --git a/src/MemStore.cc b/src/MemStore.cc
index bd52388303328f2bd308b50d1790a18726a4c17c..0380ecc5491ddf038b73c50881c55f0b5c3de994 100644 (file)
--- a/src/MemStore.cc
+++ b/src/MemStore.cc
@@ -369,7 +369,7 @@ MemStore::copyFromShm(StoreEntry &e, const sfileno index, const Ipc::StoreMapAnc
     // would be nice to call validLength() here, but it needs e.key
 
     // we read the entire response into the local memory; no more need to lock
-    disconnect(*e.mem_obj);
+    disconnect(e);
     return true;
 }
 
@@ -642,7 +642,6 @@ MemStore::write(StoreEntry &e)
         if (!shouldCache(e) || !startCaching(e)) {
             e.mem_obj->memCache.io = MemObject::ioDone;
             Store::Root().transientsAbandon(e);
-            CollapsedForwarding::Broadcast(e);
             return;
         }
         break;
@@ -668,9 +667,7 @@ MemStore::write(StoreEntry &e)
         // fall through to the error handling code
     }
 
-    Store::Root().transientsAbandon(e);
-    disconnect(*e.mem_obj);
-    CollapsedForwarding::Broadcast(e);
+    disconnect(e);
 }
 
 void
@@ -705,9 +702,9 @@ MemStore::unlink(StoreEntry &e)
     assert(e.mem_obj);
     if (e.mem_obj->memCache.index >= 0) {
         map->freeEntry(e.mem_obj->memCache.index);
-        disconnect(*e.mem_obj);
+        disconnect(e);
     } else {
-        // the entry may bave been loaded and then disconnected from the cache
+        // the entry may have been loaded and then disconnected from the cache
         map->freeEntryByKey(reinterpret_cast<cache_key*>(e.key));
     }
         
@@ -715,17 +712,22 @@ MemStore::unlink(StoreEntry &e)
 }
 
 void
-MemStore::disconnect(MemObject &mem_obj)
+MemStore::disconnect(StoreEntry &e)
 {
+    assert(e.mem_obj);
+    MemObject &mem_obj = *e.mem_obj;
     if (mem_obj.memCache.index >= 0) {
         if (mem_obj.memCache.io == MemObject::ioWriting) {
             map->abortWriting(mem_obj.memCache.index);
+            mem_obj.memCache.index = -1;
+            mem_obj.memCache.io = MemObject::ioDone;
+            Store::Root().transientsAbandon(e); // broadcasts after the change
         } else {
             assert(mem_obj.memCache.io == MemObject::ioReading);
             map->closeForReading(mem_obj.memCache.index);
+            mem_obj.memCache.index = -1;
+            mem_obj.memCache.io = MemObject::ioDone;
         }
-        mem_obj.memCache.index = -1;
-        mem_obj.memCache.io = MemObject::ioDone;
     }
 }
 

diff --git a/src/MemStore.h b/src/MemStore.h
index 527ae13030ae30f274a9a38209512d968d7cb3ef..c08e113fee3146d76eca1c97b1d794504e1842bb 100644 (file)
--- a/src/MemStore.h
+++ b/src/MemStore.h
@@ -33,7 +33,7 @@ public:
     void unlink(StoreEntry &e);
 
     /// called when the entry is about to forget its association with mem cache
-    void disconnect(MemObject &mem_obj);
+    void disconnect(StoreEntry &e);
 
     /* Store API */
     virtual int callback();

diff --git a/src/Store.h b/src/Store.h
index 4ce61544f0fa4a2aa75ca66a2b4ab7940d58ad95..ed8df5acb384341aab03fd17b2bfb60f84b62614 100644 (file)
--- a/src/Store.h
+++ b/src/Store.h
@@ -410,7 +410,7 @@ public:
     // XXX: This method belongs to Store::Root/StoreController, but it is here
     // to avoid casting Root() to StoreController until Root() API is fixed.
     /// disassociates the entry from the memory cache, preserving cached data
-    virtual void memoryDisconnect(MemObject &mem_obj) {}
+    virtual void memoryDisconnect(StoreEntry &e) {}
 
     /// If the entry is not found, return false. Otherwise, return true after
     /// tying the entry to this cache and setting inSync to updateCollapsed().

diff --git a/src/SwapDir.h b/src/SwapDir.h
index 1dcae1198ff7a3d65b50415c91a4989449db7750..bd1e542c059b8bcdab7f35e5546110a6d0017d34 100644 (file)
--- a/src/SwapDir.h
+++ b/src/SwapDir.h
@@ -69,7 +69,7 @@ public:
     virtual void transientsDisconnect(MemObject &mem_obj);
     virtual void memoryOut(StoreEntry &e, const bool preserveSwappable);
     virtual void memoryUnlink(StoreEntry &e);
-    virtual void memoryDisconnect(MemObject &mem_obj);
+    virtual void memoryDisconnect(StoreEntry &e);
     virtual void allowCollapsing(StoreEntry *e, const RequestFlags &reqFlags, const HttpRequestMethod &reqMethod);
     virtual void syncCollapsed(const sfileno xitIndex);
 

diff --git a/src/store.cc b/src/store.cc
index 4e37a8f639e56ad11215624cd7cea0a97f0404b9..1dfd617aea6f407bcaed53bae18043c79e207ff3 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -428,10 +428,18 @@ void
 StoreEntry::destroyMemObject()
 {
     debugs(20, 3, HERE << "destroyMemObject " << mem_obj);
-    setMemStatus(NOT_IN_MEMORY);
-    MemObject *mem = mem_obj;
-    mem_obj = NULL;
-    delete mem;
+
+    if (MemObject *mem = mem_obj) {
+        // Store::Root() is FATALly missing during shutdown
+        if (mem->xitTable.index >= 0 && !shutting_down)
+            Store::Root().transientsDisconnect(*mem);
+        if (mem->memCache.index >= 0 && !shutting_down)
+            Store::Root().memoryDisconnect(*this);
+
+        setMemStatus(NOT_IN_MEMORY);
+        mem_obj = NULL;
+        delete mem;
+    }
 }
 
 void

diff --git a/src/store_dir.cc b/src/store_dir.cc
index b362896ad9285b5d9277638517b35350a10f04e4..82509bfede82b95618e96589c829b9409ebb5304 100644 (file)
--- a/src/store_dir.cc
+++ b/src/store_dir.cc
@@ -911,10 +911,10 @@ StoreController::memoryUnlink(StoreEntry &e)
 }
 
 void
-StoreController::memoryDisconnect(MemObject &mem_obj)
+StoreController::memoryDisconnect(StoreEntry &e)
 {
     if (memStore)
-        memStore->disconnect(mem_obj);
+        memStore->disconnect(e);
     // else nothing to do for non-shared memory cache
 }