commit 7e3bc66813a09440a6f77b8e4c27b7f31e8d87ab
parent 7dd24a8c10ba43dc07c7f7b4f6b3601e8e10a37b
Author: Randell Jesup <rjesup@mozilla.com>
Date: Tue, 7 Oct 2025 17:56:02 +0000
Bug 1988076: Shutdown cleanup issues for Compression Dictionaries r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D264524
Diffstat:
6 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/netwerk/cache2/CacheFileIOManager.cpp b/netwerk/cache2/CacheFileIOManager.cpp
@@ -1314,6 +1314,7 @@ nsresult CacheFileIOManager::Shutdown() {
}
CacheIndex::Shutdown();
+ DictionaryCache::Shutdown();
if (CacheObserver::ClearCacheOnShutdown()) {
auto totalTimer =
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -41,7 +41,7 @@ namespace mozilla::net {
// static
GlobalEntryTables* CacheStorageService::sGlobalEntryTables = nullptr;
-mozilla::Mutex CacheStorageService::sLock{"CacheStorageService.sLock"};
+StaticMutex CacheStorageService::sLock;
namespace {
@@ -122,7 +122,7 @@ CacheStorageService::~CacheStorageService() {
}
void CacheStorageService::Shutdown() {
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) return;
@@ -150,7 +150,7 @@ void CacheStorageService::ShutdownBackground() {
MOZ_ASSERT(IsOnManagementThread());
{
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
// Cancel purge timer to avoid leaking.
if (mPurgeTimer) {
@@ -226,7 +226,7 @@ class WalkMemoryCacheRunnable : public WalkCacheRunnable {
LOG(("WalkMemoryCacheRunnable::Run - collecting [this=%p]", this));
// First, walk, count and grab all entries from the storage
- mozilla::MutexAutoLock lock(CacheStorageService::sLock);
+ StaticMutexAutoLock lock(CacheStorageService::sLock);
if (!CacheStorageService::IsRunning()) return NS_ERROR_NOT_INITIALIZED;
@@ -522,7 +522,7 @@ class WalkDiskCacheRunnable : public WalkCacheRunnable {
} // namespace CacheStorageServiceInternal
void CacheStorageService::DropPrivateBrowsingEntries() {
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) return;
@@ -664,7 +664,7 @@ NS_IMETHODIMP CacheStorageService::Clear() {
// when all the context have been removed from disk.
CacheIndex::OnAsyncEviction(true);
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
{
mozilla::MutexAutoLock forcedValidEntriesLock(mForcedValidEntriesLock);
@@ -753,7 +753,7 @@ NS_IMETHODIMP CacheStorageService::ClearBaseDomain(
const nsAString& aBaseDomain) {
LOG(("CacheStorageService::ClearBaseDomain %s",
NS_ConvertUTF16toUTF8(aBaseDomain).get()));
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (sGlobalEntryTables) {
if (mShutdown) return NS_ERROR_NOT_AVAILABLE;
@@ -842,7 +842,7 @@ nsresult CacheStorageService::ClearOriginInternal(
return NS_ERROR_FAILURE;
}
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (sGlobalEntryTables) {
for (const auto& globalEntry : *sGlobalEntryTables) {
@@ -1052,7 +1052,7 @@ bool CacheStorageService::RemoveEntry(CacheEntry* aEntry,
return false;
}
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) {
LOG((" after shutdown"));
@@ -1302,7 +1302,7 @@ bool CacheStorageService::MemoryPool::OnMemoryConsumptionChange(
void CacheStorageService::SchedulePurgeOverMemoryLimit() {
LOG(("CacheStorageService::SchedulePurgeOverMemoryLimit"));
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) {
LOG((" past shutdown"));
@@ -1329,7 +1329,7 @@ NS_IMETHODIMP
CacheStorageService::Notify(nsITimer* aTimer) {
LOG(("CacheStorageService::Notify"));
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (aTimer == mPurgeTimer) {
#ifdef MOZ_TSAN
@@ -1500,7 +1500,7 @@ Result<size_t, nsresult> CacheStorageService::MemoryPool::PurgeByFrecency(
return Err(NS_ERROR_OUT_OF_MEMORY);
}
{
- mozilla::MutexAutoLock lock(CacheStorageService::Self()->Lock());
+ StaticMutexAutoLock lock(CacheStorageService::Self()->Lock());
for (const auto& entry : mManagedEntries) {
// Referenced items cannot be purged and we deliberately want to not look
@@ -1603,7 +1603,7 @@ nsresult CacheStorageService::AddStorageEntry(
RefPtr<CacheEntryHandle> handle;
{
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1708,7 +1708,7 @@ nsresult CacheStorageService::CheckStorageEntry(CacheStorage const* aStorage,
aURI.BeginReading(), aIdExtension.BeginReading(), contextKey.get()));
{
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1870,7 +1870,7 @@ nsresult CacheStorageService::DoomStorageEntry(
RefPtr<CacheEntry> entry;
{
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1957,7 +1957,7 @@ nsresult CacheStorageService::DoomStorageEntries(
nsAutoCString contextKey;
CacheFileUtils::AppendKeyPrefix(aStorage->LoadInfo(), contextKey);
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
return DoomStorageEntries(contextKey, aStorage->LoadInfo(),
aStorage->WriteToDisk(), aStorage->Pinning(),
@@ -2097,7 +2097,7 @@ void CacheStorageService::CacheFileDoomed(const nsACString& aKey,
nsAutoCString entryKey;
CacheEntry::HashingKey(""_ns, aIdExtension, aURISpec, entryKey);
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) {
return;
@@ -2134,7 +2134,7 @@ bool CacheStorageService::GetCacheEntryInfo(
RefPtr<CacheEntry> entry;
{
- mozilla::MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
if (mShutdown) {
return false;
@@ -2316,7 +2316,7 @@ size_t CacheStorageService::SizeOfIncludingThis(
NS_IMETHODIMP
CacheStorageService::CollectReports(nsIHandleReportCallback* aHandleReport,
nsISupports* aData, bool aAnonymize) {
- MutexAutoLock lock(sLock);
+ StaticMutexAutoLock lock(sLock);
MOZ_COLLECT_REPORT("explicit/network/cache2/io", KIND_HEAP, UNITS_BYTES,
CacheFileIOManager::SizeOfIncludingThis(MallocSizeOf),
"Memory used by the cache IO manager.");
diff --git a/netwerk/cache2/CacheStorageService.h b/netwerk/cache2/CacheStorageService.h
@@ -19,6 +19,7 @@
#include "nsProxyRelease.h"
#include "mozilla/Monitor.h"
#include "mozilla/Mutex.h"
+#include "mozilla/StaticMutex.h"
#include "mozilla/AtomicBitfields.h"
#include "mozilla/Atomics.h"
#include "mozilla/TimeStamp.h"
@@ -111,7 +112,7 @@ class CacheStorageService final : public nsICacheStorageService,
static bool IsRunning() { return sSelf && !sSelf->mShutdown; }
static bool IsOnManagementThread();
already_AddRefed<nsIEventTarget> Thread() const;
- mozilla::Mutex& Lock() { return sLock; }
+ StaticMutex& Lock() { return sLock; }
// Tracks entries that may be forced valid in a pruned hashtable.
struct ForcedValidData {
@@ -346,7 +347,7 @@ class CacheStorageService final : public nsICacheStorageService,
static CacheStorageService* sSelf;
- static mozilla::Mutex sLock;
+ static StaticMutex sLock;
mozilla::Mutex mForcedValidEntriesLock{
"CacheStorageService.mForcedValidEntriesLock"};
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -80,7 +80,7 @@ LazyLogModule gDictionaryLog("CompressionDictionaries");
* Reference to the DictionaryCache singleton. May be null.
*/
StaticRefPtr<DictionaryCache> gDictionaryCache;
-nsCOMPtr<nsICacheStorage> DictionaryCache::sCacheStorage;
+StaticRefPtr<nsICacheStorage> DictionaryCache::sCacheStorage;
// about:cache gets upset about entries that don't fit URL specs, so we need
// to add the trailing '/' to GetPrePath()
@@ -758,16 +758,24 @@ nsresult DictionaryCache::Init() {
if (!cacheStorageService) {
return NS_ERROR_FAILURE;
}
+ nsCOMPtr<nsICacheStorage> temp;
nsresult rv = cacheStorageService->DiskCacheStorage(
- nullptr, getter_AddRefs(sCacheStorage)); // Don't need a load context
+ nullptr, getter_AddRefs(temp)); // Don't need a load context
if (NS_FAILED(rv)) {
return rv;
}
+ sCacheStorage = temp;
}
DICTIONARY_LOG(("Inited DictionaryCache %p", sCacheStorage.get()));
return NS_OK;
}
+// static
+void DictionaryCache::Shutdown() {
+ gDictionaryCache = nullptr;
+ sCacheStorage = nullptr;
+}
+
nsresult DictionaryCache::AddEntry(nsIURI* aURI, const nsACString& aKey,
const nsACString& aPattern,
nsTArray<nsCString>& aMatchDest,
@@ -829,7 +837,7 @@ already_AddRefed<DictionaryCacheEntry> DictionaryCache::AddEntry(
// which cause allocations
// Write the dirty entry we couldn't write before once
// we get the hash
- aDictEntry->WriteOnHash();
+ entry->WriteOnHash();
return NS_OK;
});
// Since this is read asynchronously, we need to either add the entry
diff --git a/netwerk/cache2/Dictionary.h b/netwerk/cache2/Dictionary.h
@@ -59,7 +59,7 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
NS_DECL_NSIREQUESTOBSERVER
NS_DECL_NSISTREAMLISTENER
- DictionaryCacheEntry(const char* aKey);
+ explicit DictionaryCacheEntry(const char* aKey);
DictionaryCacheEntry(const nsACString& aKey, const nsACString& aPattern,
nsTArray<nsCString>& aMatchDest, const nsACString& aId,
uint32_t aExpiration = 0,
@@ -322,6 +322,7 @@ class DictionaryCache final {
static already_AddRefed<DictionaryCache> GetInstance();
nsresult Init();
+ static void Shutdown();
nsresult AddEntry(nsIURI* aURI, const nsACString& aKey,
const nsACString& aPattern, nsTArray<nsCString>& aMatchDest,
@@ -356,7 +357,7 @@ class DictionaryCache final {
}
private:
- static nsCOMPtr<nsICacheStorage> sCacheStorage;
+ static StaticRefPtr<nsICacheStorage> sCacheStorage;
// In-memory cache of dictionary entries. HashMap, keyed by origin, of
// Linked list (LRU order) of valid dictionaries for the origin.
diff --git a/netwerk/docs/cache2/doc.rst b/netwerk/docs/cache2/doc.rst
@@ -628,7 +628,6 @@ Future optimizations:
- Compressing dictionary-encoded files with zstd in the cache
-- Trades CPU use for hitrate
-- Perhaps only above some size
-- Preemptively reading dict:<origin> entries into memory in the background
- at startup
+- Preemptively reading dict:<origin> entries into memory in the background at startup
-- Up to some limit
- LRU-ing dict:<origin> entries and dropping old ones from memory
