commit c7325cad6fa282a437cac15a8c4f1f1e7e1ab8fc
parent 6c6b0579a99701df58aa0d415327ecaedee46628
Author: Cristian Tuns <ctuns@mozilla.com>
Date: Thu, 2 Oct 2025 07:40:09 -0400
Revert "Bug 1975024: Cleanup Cache code and locks in prep for Compression Dictionaries #necko-reviewers r=necko-reviewers,kershaw" for causing build bustages in CacheStorageService.cpp
This reverts commit 9c189563abd35ca9d8943ee866750f291fe3d8b3.
Diffstat:
3 files changed, 48 insertions(+), 69 deletions(-)
diff --git a/netwerk/cache2/CacheEntry.cpp b/netwerk/cache2/CacheEntry.cpp
@@ -266,12 +266,6 @@ nsresult CacheEntry::HashingKey(const nsACString& aStorageID,
return HashingKey(aStorageID, aEnhanceID, spec, aResult);
}
-// The hash key (which is also the filename) is:
-// A[~B]:C
-// Where A is the storage ID ([O<oa>,][a,][p,]), B is the optional 'id',
-// and C is the URI 'oa' are the OriginAttributes in suffix form
-// (i.e. |^key=value&key2=value2|)
-
// static
nsresult CacheEntry::HashingKey(const nsACString& aStorageID,
const nsACString& aEnhanceID,
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -39,10 +39,6 @@
namespace mozilla::net {
-// static
-GlobalEntryTables* CacheStorageService::sGlobalEntryTables = nullptr;
-mozilla::Mutex CacheStorageService::sLock{"CacheStorageService.sLock"};
-
namespace {
void AppendMemoryStorageTag(nsAutoCString& key) {
@@ -54,6 +50,21 @@ void AppendMemoryStorageTag(nsAutoCString& key) {
} // namespace
+// Not defining as static or class member of CacheStorageService since
+// it would otherwise need to include CacheEntry.h and that then would
+// need to be exported to make nsNetModule.cpp compilable.
+using GlobalEntryTables = nsClassHashtable<nsCStringHashKey, CacheEntryTable>;
+
+/**
+ * Keeps tables of entries. There is one entries table for each distinct load
+ * context type. The distinction is based on following load context info
+ * states: <isPrivate|isAnon|inIsolatedMozBrowser> which builds a mapping
+ * key.
+ *
+ * Thread-safe to access, protected by the service mutex.
+ */
+static GlobalEntryTables* sGlobalEntryTables;
+
CacheMemoryConsumer::CacheMemoryConsumer(uint32_t aFlags) {
StoreFlags(aFlags);
}
@@ -122,7 +133,7 @@ CacheStorageService::~CacheStorageService() {
}
void CacheStorageService::Shutdown() {
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) return;
@@ -150,7 +161,7 @@ void CacheStorageService::ShutdownBackground() {
MOZ_ASSERT(IsOnManagementThread());
{
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
// Cancel purge timer to avoid leaking.
if (mPurgeTimer) {
@@ -169,7 +180,7 @@ void CacheStorageService::ShutdownBackground() {
// Internal management methods
-namespace CacheStorageServiceInternal {
+namespace {
// WalkCacheRunnable
// Base class for particular storage entries visiting
@@ -226,14 +237,13 @@ 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);
+ mozilla::MutexAutoLock lock(CacheStorageService::Self()->Lock());
if (!CacheStorageService::IsRunning()) return NS_ERROR_NOT_INITIALIZED;
// Count the entries to allocate the array memory all at once.
size_t numEntries = 0;
- for (const auto& entries :
- CacheStorageService::sGlobalEntryTables->Values()) {
+ for (const auto& entries : sGlobalEntryTables->Values()) {
if (entries->Type() != CacheEntryTable::MEMORY_ONLY) {
continue;
}
@@ -242,8 +252,7 @@ class WalkMemoryCacheRunnable : public WalkCacheRunnable {
mEntryArray.SetCapacity(numEntries);
// Collect the entries.
- for (const auto& entries :
- CacheStorageService::sGlobalEntryTables->Values()) {
+ for (const auto& entries : sGlobalEntryTables->Values()) {
if (entries->Type() != CacheEntryTable::MEMORY_ONLY) {
continue;
}
@@ -519,10 +528,10 @@ class WalkDiskCacheRunnable : public WalkCacheRunnable {
uint32_t mCount;
};
-} // namespace CacheStorageServiceInternal
+} // namespace
void CacheStorageService::DropPrivateBrowsingEntries() {
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) return;
@@ -664,7 +673,7 @@ NS_IMETHODIMP CacheStorageService::Clear() {
// when all the context have been removed from disk.
CacheIndex::OnAsyncEviction(true);
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
{
mozilla::MutexAutoLock forcedValidEntriesLock(mForcedValidEntriesLock);
@@ -747,8 +756,9 @@ static bool RemoveExactEntry(CacheEntryTable* aEntries, nsACString const& aKey,
NS_IMETHODIMP CacheStorageService::ClearBaseDomain(
const nsAString& aBaseDomain) {
- mozilla::MutexAutoLock lock(sLock);
if (sGlobalEntryTables) {
+ mozilla::MutexAutoLock lock(mLock);
+
if (mShutdown) return NS_ERROR_NOT_AVAILABLE;
nsCString cBaseDomain = NS_ConvertUTF16toUTF8(aBaseDomain);
@@ -835,7 +845,7 @@ nsresult CacheStorageService::ClearOriginInternal(
return NS_ERROR_FAILURE;
}
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (sGlobalEntryTables) {
for (const auto& globalEntry : *sGlobalEntryTables) {
@@ -968,9 +978,8 @@ NS_IMETHODIMP CacheStorageService::AsyncVisitAllStorages(
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
// Walking the disk cache also walks the memory cache.
- RefPtr<CacheStorageServiceInternal::WalkDiskCacheRunnable> event =
- new CacheStorageServiceInternal::WalkDiskCacheRunnable(
- nullptr, aVisitEntries, aVisitor);
+ RefPtr<WalkDiskCacheRunnable> event =
+ new WalkDiskCacheRunnable(nullptr, aVisitEntries, aVisitor);
return event->Walk();
}
@@ -1032,7 +1041,7 @@ bool CacheStorageService::RemoveEntry(CacheEntry* aEntry,
return false;
}
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) {
LOG((" after shutdown"));
@@ -1080,7 +1089,7 @@ void CacheStorageService::RecordMemoryOnlyEntry(CacheEntry* aEntry,
// not is always recorded in the storage master hash table, the one identified
// by CacheEntry.StorageID().
- sLock.AssertCurrentThreadOwns();
+ mLock.AssertCurrentThreadOwns();
if (mShutdown) {
LOG((" after shutdown"));
@@ -1282,7 +1291,7 @@ bool CacheStorageService::MemoryPool::OnMemoryConsumptionChange(
void CacheStorageService::SchedulePurgeOverMemoryLimit() {
LOG(("CacheStorageService::SchedulePurgeOverMemoryLimit"));
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) {
LOG((" past shutdown"));
@@ -1309,7 +1318,7 @@ NS_IMETHODIMP
CacheStorageService::Notify(nsITimer* aTimer) {
LOG(("CacheStorageService::Notify"));
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (aTimer == mPurgeTimer) {
#ifdef MOZ_TSAN
@@ -1583,7 +1592,7 @@ nsresult CacheStorageService::AddStorageEntry(
RefPtr<CacheEntryHandle> handle;
{
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1679,7 +1688,7 @@ nsresult CacheStorageService::CheckStorageEntry(CacheStorage const* aStorage,
aURI.BeginReading(), aIdExtension.BeginReading(), contextKey.get()));
{
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1840,7 +1849,7 @@ nsresult CacheStorageService::DoomStorageEntry(
RefPtr<CacheEntry> entry;
{
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
NS_ENSURE_FALSE(mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -1927,7 +1936,7 @@ nsresult CacheStorageService::DoomStorageEntries(
nsAutoCString contextKey;
CacheFileUtils::AppendKeyPrefix(aStorage->LoadInfo(), contextKey);
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
return DoomStorageEntries(contextKey, aStorage->LoadInfo(),
aStorage->WriteToDisk(), aStorage->Pinning(),
@@ -1940,7 +1949,7 @@ nsresult CacheStorageService::DoomStorageEntries(
LOG(("CacheStorageService::DoomStorageEntries [context=%s]",
aContextKey.BeginReading()));
- sLock.AssertCurrentThreadOwns();
+ mLock.AssertCurrentThreadOwns();
NS_ENSURE_TRUE(!mShutdown, NS_ERROR_NOT_INITIALIZED);
@@ -2045,15 +2054,13 @@ nsresult CacheStorageService::WalkStorageEntries(
NS_ENSURE_ARG(aStorage);
if (aStorage->WriteToDisk()) {
- RefPtr<CacheStorageServiceInternal::WalkDiskCacheRunnable> event =
- new CacheStorageServiceInternal::WalkDiskCacheRunnable(
- aStorage->LoadInfo(), aVisitEntries, aVisitor);
+ RefPtr<WalkDiskCacheRunnable> event = new WalkDiskCacheRunnable(
+ aStorage->LoadInfo(), aVisitEntries, aVisitor);
return event->Walk();
}
- RefPtr<CacheStorageServiceInternal::WalkMemoryCacheRunnable> event =
- new CacheStorageServiceInternal::WalkMemoryCacheRunnable(
- aStorage->LoadInfo(), aVisitEntries, aVisitor);
+ RefPtr<WalkMemoryCacheRunnable> event = new WalkMemoryCacheRunnable(
+ aStorage->LoadInfo(), aVisitEntries, aVisitor);
return event->Walk();
}
@@ -2066,7 +2073,7 @@ void CacheStorageService::CacheFileDoomed(nsILoadContextInfo* aLoadContextInfo,
nsAutoCString entryKey;
CacheEntry::HashingKey(""_ns, aIdExtension, aURISpec, entryKey);
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) {
return;
@@ -2103,7 +2110,7 @@ bool CacheStorageService::GetCacheEntryInfo(
RefPtr<CacheEntry> entry;
{
- mozilla::MutexAutoLock lock(sLock);
+ mozilla::MutexAutoLock lock(mLock);
if (mShutdown) {
return false;
@@ -2262,7 +2269,7 @@ void CacheStorageService::TelemetryRecordEntryRemoval(CacheEntry* entry) {
size_t CacheStorageService::SizeOfExcludingThis(
mozilla::MallocSizeOf mallocSizeOf) const {
- sLock.AssertCurrentThreadOwns();
+ CacheStorageService::Self()->Lock().AssertCurrentThreadOwns();
size_t n = 0;
// The elemets are referenced by sGlobalEntryTables and are reported from
@@ -2285,7 +2292,7 @@ size_t CacheStorageService::SizeOfIncludingThis(
NS_IMETHODIMP
CacheStorageService::CollectReports(nsIHandleReportCallback* aHandleReport,
nsISupports* aData, bool aAnonymize) {
- MutexAutoLock lock(sLock);
+ MutexAutoLock lock(mLock);
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
@@ -41,7 +41,6 @@ class CacheStorageService;
class CacheStorage;
class CacheEntry;
class CacheEntryHandle;
-class CacheEntryTable;
class CacheMemoryConsumer {
private:
@@ -74,22 +73,11 @@ class CacheMemoryConsumer {
void DoMemoryReport(uint32_t aCurrentSize);
};
-using GlobalEntryTables = nsClassHashtable<nsCStringHashKey, CacheEntryTable>;
-class WalkMemoryCacheRunnable;
-
-namespace CacheStorageServiceInternal {
-class WalkMemoryCacheRunnable;
-class WalkDiskCacheRunnable;
-} // namespace CacheStorageServiceInternal
-
class CacheStorageService final : public nsICacheStorageService,
public nsIMemoryReporter,
public nsITimerCallback,
public nsICacheTesting,
public nsINamed {
- friend class CacheStorageServiceInternal::WalkMemoryCacheRunnable;
- friend class CacheStorageServiceInternal::WalkDiskCacheRunnable;
-
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSICACHESTORAGESERVICE
@@ -111,7 +99,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; }
+ mozilla::Mutex& Lock() { return mLock; }
// Tracks entries that may be forced valid in a pruned hashtable.
struct ForcedValidData {
@@ -157,16 +145,6 @@ class CacheStorageService final : public nsICacheStorageService,
virtual ~CacheStorageService();
void ShutdownBackground();
- /**
- * Keeps tables of entries. There is one entries table for each distinct load
- * context type. The distinction is based on following load context info
- * states: <isPrivate|isAnon|inIsolatedMozBrowser> which builds a mapping
- * key.
- *
- * Thread-safe to access, protected by the service mutex.
- */
- static GlobalEntryTables* sGlobalEntryTables MOZ_GUARDED_BY(sLock);
-
private:
// The following methods may only be called on the management
// thread.
@@ -344,7 +322,7 @@ class CacheStorageService final : public nsICacheStorageService,
static CacheStorageService* sSelf;
- static mozilla::Mutex sLock;
+ mozilla::Mutex mLock MOZ_UNANNOTATED{"CacheStorageService.mLock"};
mozilla::Mutex mForcedValidEntriesLock{
"CacheStorageService.mForcedValidEntriesLock"};
