From acc5dc4ca0ad0968988260493b72af88c6cdc79b Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 18 Dec 2013 21:53:35 -0700 Subject: [PATCH] Add missing StoreEntry::lock() parameters, require unlock() parameter, and polish un/lock() context debugging. --- src/Store.h | 3 ++- src/icmp/net_db.cc | 2 +- src/mgr/StoreToCommWriter.cc | 2 +- src/mime.cc | 2 +- src/neighbors.cc | 2 +- src/peer_digest.cc | 10 +++++----- src/repl/heap/store_repl_heap.cc | 4 ++-- src/store.cc | 7 ++----- src/store_digest.cc | 2 +- src/tests/testRock.cc | 2 +- src/tests/testUfs.cc | 2 +- 11 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/Store.h b/src/Store.h index c8fef091bb..b5325782ce 100644 --- a/src/Store.h +++ b/src/Store.h @@ -202,11 +202,12 @@ public: virtual int64_t contentLen() const; /// claim shared ownership of this entry (for use in a given context) + /// matching lock() and unlock() contexts eases leak triage but is optional 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"); + int unlock(const char *context); /// returns a local concurrent use counter, for debugging int locks() const { return static_cast(lock_count); } diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index c17d7754b5..cc1ccd8ffa 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -887,7 +887,7 @@ netdbExchangeDone(void *data) debugs(38, 3, "netdbExchangeDone: " << ex->e->url() ); HTTPMSGUNLOCK(ex->r); storeUnregister(ex->sc, ex->e, ex); - ex->e->unlock(); + ex->e->unlock("netdbExchangeDone"); cbdataReferenceDone(ex->p); cbdataFree(ex); } diff --git a/src/mgr/StoreToCommWriter.cc b/src/mgr/StoreToCommWriter.cc index 43a3156204..00f9226c3c 100644 --- a/src/mgr/StoreToCommWriter.cc +++ b/src/mgr/StoreToCommWriter.cc @@ -138,7 +138,7 @@ Mgr::StoreToCommWriter::swanSong() sc = NULL; } entry->unregisterAbort(); - entry->unlock(); + entry->unlock("Mgr::StoreToCommWriter::swanSong"); entry = NULL; } close(); diff --git a/src/mime.cc b/src/mime.cc index e0c2983f54..b49475612d 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -440,7 +440,7 @@ MimeIcon::created (StoreEntry *newEntry) e->flush(); e->complete(); e->timestampsSet(); - e->unlock(); + e->unlock("MimeIcon::created"); memFree(buf, MEM_4K_BUF); debugs(25, 3, "Loaded icon " << url_); } diff --git a/src/neighbors.cc b/src/neighbors.cc index a02af0726e..eb2dd52ea6 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -1426,7 +1426,7 @@ peerCountMcastPeersDone(void *data) fake->abort(); // sets ENTRY_ABORTED and initiates releated cleanup HTTPMSGUNLOCK(fake->mem_obj->request); - fake->unlock(); + fake->unlock("peerCountMcastPeersDone"); HTTPMSGUNLOCK(psstate->request); cbdataFree(psstate); } diff --git a/src/peer_digest.cc b/src/peer_digest.cc index d2efd14b23..60559dc4ce 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -371,7 +371,7 @@ peerDigestRequest(PeerDigest * pd) if (old_e) { debugs(72, 5, "peerDigestRequest: found old entry"); - old_e->lock(); + old_e->lock("peerDigestRequest"); old_e->createMemObject(url, url, req->method); fetch->old_sc = storeClientListAdd(old_e, fetch); @@ -561,7 +561,7 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) /* get rid of 304 reply */ storeUnregister(fetch->sc, fetch->entry, fetch); - fetch->entry->unlock(); + fetch->entry->unlock("peerDigestFetchReply 304"); fetch->entry = fetch->old_entry; @@ -577,7 +577,7 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) debugs(72, 3, "peerDigestFetchReply: got new digest, releasing old one"); storeUnregister(fetch->old_sc, fetch->old_entry, fetch); fetch->old_entry->releaseRequest(); - fetch->old_entry->unlock(); + fetch->old_entry->unlock("peerDigestFetchReply 200"); fetch->old_entry = NULL; } } else { @@ -910,7 +910,7 @@ peerDigestFetchFinish(DigestFetchState * fetch, int err) debugs(72, 3, "peerDigestFetchFinish: deleting old entry"); storeUnregister(fetch->old_sc, fetch->old_entry, fetch); fetch->old_entry->releaseRequest(); - fetch->old_entry->unlock(); + fetch->old_entry->unlock("peerDigestFetchFinish old"); fetch->old_entry = NULL; } @@ -926,7 +926,7 @@ peerDigestFetchFinish(DigestFetchState * fetch, int err) /* unlock everything */ storeUnregister(fetch->sc, fetch->entry, fetch); - fetch->entry->unlock(); + fetch->entry->unlock("peerDigestFetchFinish new"); HTTPMSGUNLOCK(fetch->request); diff --git a/src/repl/heap/store_repl_heap.cc b/src/repl/heap/store_repl_heap.cc index 0ac8c0714b..270f780f1c 100644 --- a/src/repl/heap/store_repl_heap.cc +++ b/src/repl/heap/store_repl_heap.cc @@ -230,7 +230,7 @@ try_again: if (entry->locked()) { - entry->lock(); + entry->lock("heap_purgeNext"); linklistPush(&heap_walker->locked_entries, entry); goto try_again; @@ -263,7 +263,7 @@ heap_purgeDone(RemovalPurgeWalker * walker) while ((entry = (StoreEntry *)linklistShift(&heap_walker->locked_entries))) { heap_node *node = heap_insert(h->theHeap, entry); h->setPolicyNode(entry, node); - entry->unlock(); + entry->unlock("heap_purgeDone"); } safe_free(walker->_data); diff --git a/src/store.cc b/src/store.cc index 17a634a6d1..69c0a0403d 100644 --- a/src/store.cc +++ b/src/store.cc @@ -1243,11 +1243,8 @@ StoreEntry::release() setPrivateKey(); if (swap_filen > -1) { - /* - * Fake a call to StoreEntry->lock() When rebuilding is done, - * we'll just call StoreEntry->unlock() on these. - */ - lock("StoreEntry::release+rebuilding"); + // lock the entry until rebuilding is done + lock("storeLateRelease"); setReleaseFlag(); LateReleaseStack.push_back(this); } else { diff --git a/src/store_digest.cc b/src/store_digest.cc index 1a636252f9..fd3c647074 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -442,7 +442,7 @@ storeDigestRewriteFinish(StoreEntry * e) " (" << std::showpos << (int) (e->expires - squid_curtime) << ")"); /* is this the write order? @?@ */ e->mem_obj->unlinkRequest(); - e->unlock(); + e->unlock("storeDigestRewriteFinish"); sd_state.rewrite_lock = NULL; ++sd_state.rewrite_count; eventAdd("storeDigestRewriteStart", storeDigestRewriteStart, NULL, (double) diff --git a/src/tests/testRock.cc b/src/tests/testRock.cc index 3f209553b7..cf7cc4055e 100644 --- a/src/tests/testRock.cc +++ b/src/tests/testRock.cc @@ -251,7 +251,7 @@ testRock::testRockSwapOut() CPPUNIT_ASSERT_EQUAL(SWAPOUT_DONE, pe->swap_status); - pe->unlock(); + pe->unlock("testRock::testRockSwapOut"); } CPPUNIT_ASSERT_EQUAL((uint64_t)5, store->currentCount()); diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc index ca53aa9660..342687ae30 100644 --- a/src/tests/testUfs.cc +++ b/src/tests/testUfs.cc @@ -165,7 +165,7 @@ testUfs::testUfsSearch() pe->swapOut(); CPPUNIT_ASSERT_EQUAL(0, pe->swap_dirn); CPPUNIT_ASSERT_EQUAL(0, pe->swap_filen); - pe->unlock(); + pe->unlock("testUfs::testUfsSearch vary"); } storeDirWriteCleanLogs(0); -- 2.47.2