From 6f463ede321102a8029410ad4a968fc92bbd3753 Mon Sep 17 00:00:00 2001 From: edguloien Date: Mon, 29 Aug 2022 17:29:32 +0000 Subject: [PATCH] Bug 1745972 - Dispatch cache wrapper deletion to caller thread to acquire lock before deletion and safety-removal from frecency array. r=kershaw,necko-reviewers,valentin Differential Revision: https://phabricator.services.mozilla.com/D151764 --- netwerk/cache2/CacheIndex.cpp | 35 +++++++++++++++++++++++++++++++++++ netwerk/cache2/CacheIndex.h | 6 +++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/netwerk/cache2/CacheIndex.cpp b/netwerk/cache2/CacheIndex.cpp index a0de065572d2..13bdb81d3cbb 100644 --- a/netwerk/cache2/CacheIndex.cpp +++ b/netwerk/cache2/CacheIndex.cpp @@ -72,6 +72,41 @@ class FrecencyComparator { } // namespace +// used to dispatch a wrapper deletion the caller's thread +// cannot be used on IOThread after shutdown begins +class DeleteCacheIndexRecordWrapper : public Runnable { + CacheIndexRecordWrapper* mWrapper; + + public: + explicit DeleteCacheIndexRecordWrapper(CacheIndexRecordWrapper* wrapper) + : Runnable("net::CacheIndex::DeleteCacheIndexRecordWrapper"), + mWrapper(wrapper) {} + NS_IMETHOD Run() override { + StaticMutexAutoLock lock(CacheIndex::sLock); + + // if somehow the item is still in the frecency array, remove it + RefPtr index = CacheIndex::gInstance; + if (index) { + bool found = index->mFrecencyArray.RecordExistedUnlocked(mWrapper); + if (found) { + LOG( + ("DeleteCacheIndexRecordWrapper::Run() - \ + record wrapper found in frecency array during deletion")); + index->mFrecencyArray.RemoveRecord(mWrapper, lock); + } + } + + delete mWrapper; + return NS_OK; + } +}; + +void CacheIndexRecordWrapper::DispatchDeleteSelfToCurrentThread() { + // Dispatch during shutdown will not trigger DeleteCacheIndexRecordWrapper + nsCOMPtr event = new DeleteCacheIndexRecordWrapper(this); + MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(event)); +} + CacheIndexRecordWrapper::~CacheIndexRecordWrapper() { #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED CacheIndex::sLock.AssertCurrentThreadOwns(); diff --git a/netwerk/cache2/CacheIndex.h b/netwerk/cache2/CacheIndex.h index 684db9f96c15..0071025239ac 100644 --- a/netwerk/cache2/CacheIndex.h +++ b/netwerk/cache2/CacheIndex.h @@ -110,14 +110,17 @@ static_assert(sizeof(CacheIndexRecord::mHash) + class CacheIndexRecordWrapper final { public: - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CacheIndexRecordWrapper) + NS_INLINE_DECL_THREADSAFE_REFCOUNTING_WITH_DESTROY( + CacheIndexRecordWrapper, DispatchDeleteSelfToCurrentThread()); CacheIndexRecordWrapper() : mRec(MakeUnique()) {} CacheIndexRecord* Get() { return mRec.get(); } private: ~CacheIndexRecordWrapper(); + void DispatchDeleteSelfToCurrentThread(); UniquePtr mRec; + friend class DeleteCacheIndexRecordWrapper; }; class CacheIndexEntry : public PLDHashEntryHdr { @@ -813,6 +816,7 @@ class CacheIndex final : public CacheFileIOListener, public nsIRunnable { friend class FileOpenHelper; friend class CacheIndexIterator; friend class CacheIndexRecordWrapper; + friend class DeleteCacheIndexRecordWrapper; virtual ~CacheIndex();