From: Alex Rousskov Date: Tue, 2 Jul 2013 19:23:49 +0000 (-0600) Subject: Broadcast mem-cache writer departure to transient readers (in more/all cases). X-Git-Tag: SQUID_3_5_0_1~444^2~39 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=29c56e41cdbabfce6d540a327ea1a2dff250ddb6;p=thirdparty%2Fsquid.git 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 65686d49e8..ba4c32be71 100644 --- 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 bd52388303..0380ecc549 100644 --- 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(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 527ae13030..c08e113fee 100644 --- 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 4ce61544f0..ed8df5acb3 100644 --- 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 1dcae1198f..bd1e542c05 100644 --- 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 4e37a8f639..1dfd617aea 100644 --- 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 b362896ad9..82509bfede 100644 --- 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 }