Bug 1055369 - Assertion failure: !handle || !handle->IsDoomed(). r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Tue, 24 Feb 2015 11:49:46 -0500
changeset 260834 d94287364c3d04308c3e27a044be6dc96634e069
parent 260833 7717689f4a7fadd610a73d51205f4434e651e516
child 260835 ae87225300271a4250b54e19cde9501e5db30cc7
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1055369
milestone39.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 1055369 - Assertion failure: !handle || !handle->IsDoomed(). r=honzab CLOSED TREE DONTBUILD
netwerk/cache2/CacheFileContextEvictor.cpp
netwerk/cache2/CacheFileIOManager.cpp
netwerk/cache2/CacheFileIOManager.h
netwerk/cache2/CacheIndex.cpp
--- a/netwerk/cache2/CacheFileContextEvictor.cpp
+++ b/netwerk/cache2/CacheFileContextEvictor.cpp
@@ -546,17 +546,17 @@ CacheFileContextEvictor::EvictEntries()
       continue;
     }
 
     LOG(("CacheFileContextEvictor::EvictEntries() - Processing hash. "
          "[hash=%08x%08x%08x%08x%08x, iterator=%p, info=%p]", LOGSHA1(&hash),
          mEntries[0]->mIterator.get(), mEntries[0]->mInfo.get()));
 
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
                                                       getter_AddRefs(handle));
     if (handle) {
       // We doom any active handle in CacheFileIOManager::EvictByContext(), so
       // this must be a new one. Skip it.
       LOG(("CacheFileContextEvictor::EvictEntries() - Skipping entry since we "
            "found an active handle. [handle=%p]", handle.get()));
       continue;
     }
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -305,17 +305,16 @@ CacheFileHandles::CacheFileHandles()
 CacheFileHandles::~CacheFileHandles()
 {
   LOG(("CacheFileHandles::~CacheFileHandles() [this=%p]", this));
   MOZ_COUNT_DTOR(CacheFileHandles);
 }
 
 nsresult
 CacheFileHandles::GetHandle(const SHA1Sum::Hash *aHash,
-                            bool aReturnDoomed,
                             CacheFileHandle **_retval)
 {
   MOZ_ASSERT(CacheFileIOManager::IsOnIOThreadOrCeased());
   MOZ_ASSERT(aHash);
 
 #ifdef DEBUG_HANDLES
   LOG(("CacheFileHandles::GetHandle() [hash=%08x%08x%08x%08x%08x]",
        LOGSHA1(aHash)));
@@ -339,26 +338,22 @@ CacheFileHandles::GetHandle(const SHA1Su
     LOG(("CacheFileHandles::GetHandle() hash=%08x%08x%08x%08x%08x "
          "no handle found %p, entry %p", LOGSHA1(aHash), handle.get(), entry));
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   if (handle->IsDoomed()) {
     LOG(("CacheFileHandles::GetHandle() hash=%08x%08x%08x%08x%08x "
          "found doomed handle %p, entry %p", LOGSHA1(aHash), handle.get(), entry));
-
-    // If the consumer doesn't want doomed handles, exit with NOT_AVAIL.
-    if (!aReturnDoomed) {
-      return NS_ERROR_NOT_AVAILABLE;
-    }
-  } else {
-    LOG(("CacheFileHandles::GetHandle() hash=%08x%08x%08x%08x%08x "
-         "found handle %p, entry %p", LOGSHA1(aHash), handle.get(), entry));
+    return NS_ERROR_NOT_AVAILABLE;
   }
 
+  LOG(("CacheFileHandles::GetHandle() hash=%08x%08x%08x%08x%08x "
+       "found handle %p, entry %p", LOGSHA1(aHash), handle.get(), entry));
+
   handle.forget(_retval);
   return NS_OK;
 }
 
 
 nsresult
 CacheFileHandles::NewHandle(const SHA1Sum::Hash *aHash,
                             bool aPriority,
@@ -805,17 +800,17 @@ protected:
 public:
   NS_IMETHOD Run()
   {
     nsresult rv;
 
     if (!mIOMan) {
       rv = NS_ERROR_NOT_INITIALIZED;
     } else {
-      rv = mIOMan->DoomFileByKeyInternal(&mHash, false);
+      rv = mIOMan->DoomFileByKeyInternal(&mHash);
       mIOMan = nullptr;
     }
 
     if (mCallback) {
       mCallback->OnFileDoomed(nullptr, rv);
     }
 
     return NS_OK;
@@ -1570,17 +1565,17 @@ CacheFileIOManager::OpenFileInternal(con
     if (NS_FAILED(rv)) return rv;
   }
 
   nsCOMPtr<nsIFile> file;
   rv = GetFile(aHash, getter_AddRefs(file));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsRefPtr<CacheFileHandle> handle;
-  mHandles.GetHandle(aHash, false, getter_AddRefs(handle));
+  mHandles.GetHandle(aHash, getter_AddRefs(handle));
 
   if ((aFlags & (OPEN | CREATE | CREATE_NEW)) == CREATE_NEW) {
     if (handle) {
       rv = DoomFileInternal(handle);
       NS_ENSURE_SUCCESS(rv, rv);
       handle = nullptr;
     }
 
@@ -2069,45 +2064,40 @@ 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,
-                                          bool aFailIfAlreadyDoomed)
+CacheFileIOManager::DoomFileByKeyInternal(const SHA1Sum::Hash *aHash)
 {
-  LOG(("CacheFileIOManager::DoomFileByKeyInternal() [hash=%08x%08x%08x%08x%08x,"
-        " failIfAlreadyDoomed=%d]", LOGSHA1(aHash), aFailIfAlreadyDoomed));
+  LOG(("CacheFileIOManager::DoomFileByKeyInternal() [hash=%08x%08x%08x%08x%08x]"
+       , LOGSHA1(aHash)));
 
   MOZ_ASSERT(CacheFileIOManager::IsOnIOThreadOrCeased());
 
   nsresult rv;
 
   if (mShuttingDown) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   if (!mCacheDirectory) {
     return NS_ERROR_FILE_INVALID_PATH;
   }
 
   // Find active handle
   nsRefPtr<CacheFileHandle> handle;
-  mHandles.GetHandle(aHash, true, getter_AddRefs(handle));
+  mHandles.GetHandle(aHash, getter_AddRefs(handle));
 
   if (handle) {
     handle->Log();
 
-    if (handle->IsDoomed()) {
-      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));
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -2239,17 +2229,17 @@ CacheFileIOManager::GetEntryInfo(const S
   if (!ioMan) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   nsAutoCString enhanceId;
   nsAutoCString uriSpec;
 
   nsRefPtr<CacheFileHandle> handle;
-  ioMan->mHandles.GetHandle(aHash, false, getter_AddRefs(handle));
+  ioMan->mHandles.GetHandle(aHash, getter_AddRefs(handle));
   if (handle) {
     nsRefPtr<nsILoadContextInfo> info =
       CacheFileUtils::ParseKey(handle->Key(), &enhanceId, &uriSpec);
 
     MOZ_ASSERT(info);
     if (!info) {
       return NS_OK; // ignore
     }
@@ -2610,34 +2600,24 @@ CacheFileIOManager::OverLimitEvictionInt
     }
 
     SHA1Sum::Hash hash;
     uint32_t cnt;
     static uint32_t consecutiveFailures = 0;
     rv = CacheIndex::GetEntryForEviction(false, &hash, &cnt);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = DoomFileByKeyInternal(&hash, true);
+    rv = DoomFileByKeyInternal(&hash);
     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()");
@@ -3645,17 +3625,17 @@ CacheFileIOManager::OpenNSPRHandle(Cache
            " might reached a limit on FAT32. Will evict a single entry and try "
            "again. [hash=%08x%08x%08x%08x%08x]", LOGSHA1(aHandle->Hash())));
 
       SHA1Sum::Hash hash;
       uint32_t cnt;
 
       rv = CacheIndex::GetEntryForEviction(true, &hash, &cnt);
       if (NS_SUCCEEDED(rv)) {
-        rv = DoomFileByKeyInternal(&hash, true);
+        rv = DoomFileByKeyInternal(&hash);
       }
       if (NS_SUCCEEDED(rv)) {
         rv = aHandle->mFile->OpenNSPRFileDesc(
                PR_RDWR | PR_CREATE_FILE | PR_TRUNCATE, 0600, &aHandle->mFD);
         LOG(("CacheFileIOManager::OpenNSPRHandle() - Successfully evicted entry"
              " with hash %08x%08x%08x%08x%08x. %s to create the new file.",
              LOGSHA1(&hash), NS_SUCCEEDED(rv) ? "Succeeded" : "Failed"));
 
--- a/netwerk/cache2/CacheFileIOManager.h
+++ b/netwerk/cache2/CacheFileIOManager.h
@@ -85,17 +85,17 @@ private:
   nsCString            mKey;
 };
 
 class CacheFileHandles {
 public:
   CacheFileHandles();
   ~CacheFileHandles();
 
-  nsresult GetHandle(const SHA1Sum::Hash *aHash, bool aReturnDoomed, CacheFileHandle **_retval);
+  nsresult GetHandle(const SHA1Sum::Hash *aHash, CacheFileHandle **_retval);
   nsresult NewHandle(const SHA1Sum::Hash *aHash, bool aPriority, CacheFileHandle **_retval);
   void     RemoveHandle(CacheFileHandle *aHandlle);
   void     GetAllHandles(nsTArray<nsRefPtr<CacheFileHandle> > *_retval);
   void     GetActiveHandles(nsTArray<nsRefPtr<CacheFileHandle> > *_retval);
   void     ClearAll();
   uint32_t HandleCount();
 
 #ifdef DEBUG_HANDLES
@@ -325,18 +325,17 @@ 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,
-                                 bool aFailIfAlreadyDoomed);
+  nsresult DoomFileByKeyInternal(const SHA1Sum::Hash *aHash);
   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();
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -1272,17 +1272,17 @@ CacheIndex::GetEntryForEviction(bool aIg
 
 
 // static
 bool CacheIndex::IsForcedValidEntry(const SHA1Sum::Hash *aHash)
 {
   nsRefPtr<CacheFileHandle> handle;
 
   CacheFileIOManager::gInstance->mHandles.GetHandle(
-    aHash, false, getter_AddRefs(handle));
+    aHash, getter_AddRefs(handle));
 
   if (!handle)
     return false;
 
   nsCString hashKey = handle->Key();
   return CacheStorageService::Self()->IsForcedValidEntry(hashKey);
 }
 
@@ -2770,17 +2770,17 @@ CacheIndex::BuildIndex()
            "[name=%s]", leaf.get()));
       entry->Log();
       MOZ_ASSERT(entry->IsFresh());
       entry = nullptr;
     }
 
 #ifdef DEBUG
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
                                                       getter_AddRefs(handle));
 #endif
 
     if (entry) {
       // the entry is up to date
       LOG(("CacheIndex::BuildIndex() - Skipping file because the entry is up to"
            " date. [name=%s]", leaf.get()));
       entry->Log();
@@ -2983,17 +2983,17 @@ CacheIndex::UpdateIndex()
              "[name=%s]", leaf.get()));
         entry->Log();
       }
       entry = nullptr;
     }
 
 #ifdef DEBUG
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
                                                       getter_AddRefs(handle));
 #endif
 
     if (entry && entry->IsFresh()) {
       // the entry is up to date
       LOG(("CacheIndex::UpdateIndex() - Skipping file because the entry is up "
            " to date. [name=%s]", leaf.get()));
       entry->Log();