Bug 1028415 - Cache thread gets stuck in CacheFileIOManager::OverLimitEvictionInternal loop. r=honzab, a=sledru
authorMichal Novotny <michal.novotny@gmail.com>
Wed, 16 Jul 2014 10:47:02 +0200
changeset 208057 434889d480f8ab935f88e7fc74a36a95da601ec9
parent 208056 4f934dd2701009dbf1edb77ee769e361d6b3fa4b
child 208058 6e60c41431acd47a5a1efeabd74bcb9cd7b54692
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, sledru
bugs1028415
milestone32.0a2
Bug 1028415 - Cache thread gets stuck in CacheFileIOManager::OverLimitEvictionInternal loop. r=honzab, a=sledru
netwerk/cache2/CacheFileIOManager.cpp
netwerk/cache2/CacheFileIOManager.h
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -864,17 +864,17 @@ public:
   }
 
   NS_IMETHOD Run()
   {
     if (mTarget) {
       if (!mIOMan) {
         mRV = NS_ERROR_NOT_INITIALIZED;
       } else {
-        mRV = mIOMan->DoomFileByKeyInternal(&mHash);
+        mRV = mIOMan->DoomFileByKeyInternal(&mHash, false);
         mIOMan = nullptr;
       }
 
       nsCOMPtr<nsIEventTarget> target;
       mTarget.swap(target);
       target->Dispatch(this, nsIEventTarget::DISPATCH_NORMAL);
     } else {
       if (mCallback) {
@@ -2117,20 +2117,21 @@ CacheFileIOManager::DoomFileByKey(const 
   nsRefPtr<DoomFileByKeyEvent> ev = new DoomFileByKeyEvent(aKey, aCallback);
   rv = ioMan->mIOThread->DispatchAfterPendingOpens(ev);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 nsresult
-CacheFileIOManager::DoomFileByKeyInternal(const SHA1Sum::Hash *aHash)
+CacheFileIOManager::DoomFileByKeyInternal(const SHA1Sum::Hash *aHash,
+                                          bool aFailIfAlreadyDoomed)
 {
-  LOG(("CacheFileIOManager::DoomFileByKeyInternal() [hash=%08x%08x%08x%08x%08x]"
-       , LOGSHA1(aHash)));
+  LOG(("CacheFileIOManager::DoomFileByKeyInternal() [hash=%08x%08x%08x%08x%08x,"
+        " failIfAlreadyDoomed=%d]", LOGSHA1(aHash), aFailIfAlreadyDoomed));
 
   MOZ_ASSERT(CacheFileIOManager::IsOnIOThreadOrCeased());
 
   nsresult rv;
 
   if (mShuttingDown) {
     return NS_ERROR_NOT_INITIALIZED;
   }
@@ -2142,17 +2143,17 @@ CacheFileIOManager::DoomFileByKeyInterna
   // Find active handle
   nsRefPtr<CacheFileHandle> handle;
   mHandles.GetHandle(aHash, true, getter_AddRefs(handle));
 
   if (handle) {
     handle->Log();
 
     if (handle->IsDoomed()) {
-      return NS_OK;
+      return aFailIfAlreadyDoomed ? NS_ERROR_NOT_AVAILABLE : NS_OK;
     }
 
     return DoomFileInternal(handle);
   }
 
   // There is no handle for this file, delete the file if exists
   nsCOMPtr<nsIFile> file;
   rv = GetFile(aHash, getter_AddRefs(file));
@@ -2626,24 +2627,34 @@ CacheFileIOManager::OverLimitEvictionInt
     }
 
     SHA1Sum::Hash hash;
     uint32_t cnt;
     static uint32_t consecutiveFailures = 0;
     rv = CacheIndex::GetEntryForEviction(&hash, &cnt);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = DoomFileByKeyInternal(&hash);
+    rv = DoomFileByKeyInternal(&hash, true);
     if (NS_SUCCEEDED(rv)) {
       consecutiveFailures = 0;
     } else if (rv == NS_ERROR_NOT_AVAILABLE) {
       LOG(("CacheFileIOManager::OverLimitEvictionInternal() - "
            "DoomFileByKeyInternal() failed. [rv=0x%08x]", rv));
       // TODO index is outdated, start update
 
+#ifdef DEBUG
+      // Dooming should never fail due to already doomed handle, but bug 1028415
+      // shows that this unexpected state can happen. Assert in debug build so
+      // we can find the cause if we ever find a way to reproduce it with NSPR
+      // logging enabled.
+      nsRefPtr<CacheFileHandle> handle;
+      mHandles.GetHandle(&hash, true, getter_AddRefs(handle));
+      MOZ_ASSERT(!handle || !handle->IsDoomed());
+#endif
+
       // Make sure index won't return the same entry again
       CacheIndex::RemoveEntry(&hash);
       consecutiveFailures = 0;
     } else {
       // This shouldn't normally happen, but the eviction must not fail
       // completely if we ever encounter this problem.
       NS_WARNING("CacheFileIOManager::OverLimitEvictionInternal() - Unexpected "
                  "failure of DoomFileByKeyInternal()");
--- a/netwerk/cache2/CacheFileIOManager.h
+++ b/netwerk/cache2/CacheFileIOManager.h
@@ -326,17 +326,18 @@ private:
                                    uint32_t aFlags,
                                    CacheFileHandle **_retval);
   nsresult CloseHandleInternal(CacheFileHandle *aHandle);
   nsresult ReadInternal(CacheFileHandle *aHandle, int64_t aOffset,
                         char *aBuf, int32_t aCount);
   nsresult WriteInternal(CacheFileHandle *aHandle, int64_t aOffset,
                          const char *aBuf, int32_t aCount, bool aValidate);
   nsresult DoomFileInternal(CacheFileHandle *aHandle);
-  nsresult DoomFileByKeyInternal(const SHA1Sum::Hash *aHash);
+  nsresult DoomFileByKeyInternal(const SHA1Sum::Hash *aHash,
+                                 bool aFailIfAlreadyDoomed);
   nsresult ReleaseNSPRHandleInternal(CacheFileHandle *aHandle);
   nsresult TruncateSeekSetEOFInternal(CacheFileHandle *aHandle,
                                       int64_t aTruncatePos, int64_t aEOFPos);
   nsresult RenameFileInternal(CacheFileHandle *aHandle,
                               const nsACString &aNewName);
   nsresult EvictIfOverLimitInternal();
   nsresult OverLimitEvictionInternal();
   nsresult EvictAllInternal();