Bug 1052266 - Potential deadlock detected: CacheEntry, CacheIndex.mLock, CacheStorageService at CacheEntry. r=honzab, a=lmandel
authorMichal Novotny <michal.novotny@gmail.com>
Wed, 10 Sep 2014 14:43:50 +0200
changeset 224862 82df151dc46ae18234722c1add491d8a61234b71
parent 224861 785d8cb2da69f0a336cea1f714a55e803a6a48ee
child 224863 44d46e903286f2341f72c196bd87234ea6dd0a14
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, lmandel
bugs1052266
milestone34.0a2
Bug 1052266 - Potential deadlock detected: CacheEntry, CacheIndex.mLock, CacheStorageService at CacheEntry. r=honzab, a=lmandel
netwerk/cache2/CacheIndex.cpp
netwerk/cache2/CacheStorageService.cpp
netwerk/cache2/CacheStorageService.h
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -236,17 +236,17 @@ NS_IMPL_RELEASE(CacheIndex)
 
 NS_INTERFACE_MAP_BEGIN(CacheIndex)
   NS_INTERFACE_MAP_ENTRY(mozilla::net::CacheFileIOListener)
   NS_INTERFACE_MAP_ENTRY(nsIRunnable)
 NS_INTERFACE_MAP_END_THREADSAFE
 
 
 CacheIndex::CacheIndex()
-  : mLock("CacheFile.mLock")
+  : mLock("CacheIndex.mLock")
   , mState(INITIAL)
   , mShuttingDown(false)
   , mIndexNeedsUpdate(false)
   , mRemovingAll(false)
   , mIndexOnDiskIsValid(false)
   , mDontMarkIndexClean(false)
   , mIndexTimeStamp(0)
   , mUpdateEventPending(false)
--- a/netwerk/cache2/CacheStorageService.cpp
+++ b/netwerk/cache2/CacheStorageService.cpp
@@ -104,17 +104,18 @@ CacheStorageService::MemoryPool::Limit()
 NS_IMPL_ISUPPORTS(CacheStorageService,
                   nsICacheStorageService,
                   nsIMemoryReporter,
                   nsITimerCallback)
 
 CacheStorageService* CacheStorageService::sSelf = nullptr;
 
 CacheStorageService::CacheStorageService()
-: mLock("CacheStorageService")
+: mLock("CacheStorageService.mLock")
+, mForcedValidEntriesLock("CacheStorageService.mForcedValidEntriesLock")
 , mShutdown(false)
 , mDiskPool(MemoryPool::DISK)
 , mMemoryPool(MemoryPool::MEMORY)
 {
   CacheFileIOManager::Init();
 
   MOZ_ASSERT(!sSelf);
 
@@ -964,17 +965,17 @@ CacheStorageService::RemoveEntry(CacheEn
   }
 
   if (aOnlyUnreferenced) {
     if (aEntry->IsReferenced()) {
       LOG(("  still referenced, not removing"));
       return false;
     }
 
-    if (!aEntry->IsUsingDisk() && IsForcedValidEntryInternal(entryKey)) {
+    if (!aEntry->IsUsingDisk() && IsForcedValidEntry(entryKey)) {
       LOG(("  forced valid, not removing"));
       return false;
     }
   }
 
   CacheEntryTable* entries;
   if (sGlobalEntryTables->Get(aEntry->GetStorageID(), &entries))
     RemoveExactEntry(entries, entryKey, aEntry, false /* don't overwrite */);
@@ -1035,29 +1036,22 @@ CacheStorageService::RecordMemoryOnlyEnt
   if (aOnlyInMemory) {
     AddExactEntry(entries, entryKey, aEntry, aOverwrite);
   }
   else {
     RemoveExactEntry(entries, entryKey, aEntry, aOverwrite);
   }
 }
 
-// Acquires the mutex lock for CacheStorageService and calls through to
-// IsForcedValidInternal (bug 1044233)
+// Checks if a cache entry is forced valid (will be loaded directly from cache
+// without further validation) - see nsICacheEntry.idl for further details
 bool CacheStorageService::IsForcedValidEntry(nsACString &aCacheEntryKey)
 {
-  mozilla::MutexAutoLock lock(mLock);
-
-  return IsForcedValidEntryInternal(aCacheEntryKey);
-}
+  mozilla::MutexAutoLock lock(mForcedValidEntriesLock);
 
-// Checks if a cache entry is forced valid (will be loaded directly from cache
-// without further validation) - see nsICacheEntry.idl for further details
-bool CacheStorageService::IsForcedValidEntryInternal(nsACString &aCacheEntryKey)
-{
   TimeStamp validUntil;
 
   if (!mForcedValidEntries.Get(aCacheEntryKey, &validUntil)) {
     return false;
   }
 
   if (validUntil.IsNull()) {
     return false;
@@ -1073,17 +1067,17 @@ bool CacheStorageService::IsForcedValidE
   return false;
 }
 
 // Allows a cache entry to be loaded directly from cache without further
 // validation - see nsICacheEntry.idl for further details
 void CacheStorageService::ForceEntryValidFor(nsACString &aCacheEntryKey,
                                              uint32_t aSecondsToTheFuture)
 {
-  mozilla::MutexAutoLock lock(mLock);
+  mozilla::MutexAutoLock lock(mForcedValidEntriesLock);
 
   TimeStamp now = TimeStamp::NowLoRes();
   ForcedValidEntriesPrune(now);
 
   // This will be the timeout
   TimeStamp validUntil = now + TimeDuration::FromSeconds(aSecondsToTheFuture);
 
   mForcedValidEntries.Put(aCacheEntryKey, validUntil);
--- a/netwerk/cache2/CacheStorageService.h
+++ b/netwerk/cache2/CacheStorageService.h
@@ -157,29 +157,23 @@ private:
    */
   void ForceEntryValidFor(nsACString &aCacheEntryKey,
                           uint32_t aSecondsToTheFuture);
 
 private:
   friend class CacheIndex;
 
   /**
-   * Gets the mutex lock for CacheStorageService then calls through to
-   * IsForcedValidEntryInternal. See below for details.
-   */
-  bool IsForcedValidEntry(nsACString &aCacheEntryKey);
-
-  /**
    * Retrieves the status of the cache entry to see if it has been forced valid
    * (so it will loaded directly from cache without further validation)
    * CacheIndex uses this to prevent a cache entry from being prememptively
    * thrown away when forced valid
    * See nsICacheEntry.idl for more details
    */
-  bool IsForcedValidEntryInternal(nsACString &aCacheEntryKey);
+  bool IsForcedValidEntry(nsACString &aCacheEntryKey);
 
 private:
   // These are helpers for telemetry monitorying of the memory pools.
   void TelemetryPrune(TimeStamp &now);
   void TelemetryRecordEntryCreation(CacheEntry const* entry);
   void TelemetryRecordEntryRemoval(CacheEntry const* entry);
 
 private:
@@ -288,16 +282,17 @@ private:
                            bool aWriteToDisk,
                            bool aCreateIfNotExist,
                            bool aReplace,
                            CacheEntryHandle** aResult);
 
   static CacheStorageService* sSelf;
 
   mozilla::Mutex mLock;
+  mozilla::Mutex mForcedValidEntriesLock;
 
   bool mShutdown;
 
   // Accessible only on the service thread
   class MemoryPool
   {
   public:
     enum EType