Bug 1028415 - Cache thread gets stuck in CacheFileIOManager::OverLimitEvictionInternal loop, r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Wed, 16 Jul 2014 10:47:02 +0200
changeset 216271 a078bf8b709dafe3d9337a1fdbf550950a6b7135
parent 216224 f93457a01cf74de7595042a8f4e1af52a576edf0
child 216272 6f0a5bfa00a66e8be137eb39dbdc65210739e714
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1028415
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1028415 - Cache thread gets stuck in CacheFileIOManager::OverLimitEvictionInternal loop, r=honzab
netwerk/cache2/CacheFileIOManager.cpp
netwerk/cache2/CacheFileIOManager.h
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -876,17 +876,17 @@ protected:
 
 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) {
@@ -2156,20 +2156,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;
   }
@@ -2181,17 +2182,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));
@@ -2696,24 +2697,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();