Backed out changeset 12b059c5afc2 (bug 1055369) for frequent asserts across all test suites.

netwerk/cache2/CacheFileContextEvictor.cpp

@@ -551,7 +551,7 @@ CacheFileContextEvictor::EvictEntries()
          mEntries[0]->mIterator.get(), mEntries[0]->mInfo.get()));
 
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
                                                       getter_AddRefs(handle));
     if (handle) {
       // We doom any active handle in CacheFileIOManager::EvictByContext(), so

netwerk/cache2/CacheFileIOManager.cpp

@@ -310,6 +310,7 @@ CacheFileHandles::~CacheFileHandles()
 
 nsresult
 CacheFileHandles::GetHandle(const SHA1Sum::Hash *aHash,
+                            bool aReturnDoomed,
                             CacheFileHandle **_retval)
 {
   MOZ_ASSERT(CacheFileIOManager::IsOnIOThreadOrCeased());
@@ -343,11 +344,15 @@ CacheFileHandles::GetHandle(const SHA1Sum::Hash *aHash,
   if (handle->IsDoomed()) {
     LOG(("CacheFileHandles::GetHandle() hash=%08x%08x%08x%08x%08x "
          "found doomed 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));
+    // 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));
+  }
 
   handle.forget(_retval);
   return NS_OK;
@@ -805,7 +810,7 @@ public:
     if (!mIOMan) {
       rv = NS_ERROR_NOT_INITIALIZED;
     } else {
-      rv = mIOMan->DoomFileByKeyInternal(&mHash);
+      rv = mIOMan->DoomFileByKeyInternal(&mHash, false);
       mIOMan = nullptr;
     }
 
@@ -1570,7 +1575,7 @@ CacheFileIOManager::OpenFileInternal(const SHA1Sum::Hash *aHash,
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsRefPtr<CacheFileHandle> handle;
-  mHandles.GetHandle(aHash, getter_AddRefs(handle));
+  mHandles.GetHandle(aHash, false, getter_AddRefs(handle));
 
   if ((aFlags & (OPEN | CREATE | CREATE_NEW)) == CREATE_NEW) {
     if (handle) {
@@ -2069,10 +2074,11 @@ CacheFileIOManager::DoomFileByKey(const nsACString &aKey,
 }
 
 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());
 
@@ -2088,11 +2094,15 @@ CacheFileIOManager::DoomFileByKeyInternal(const SHA1Sum::Hash *aHash)
 
   // Find active handle
   nsRefPtr<CacheFileHandle> handle;
-  mHandles.GetHandle(aHash, getter_AddRefs(handle));
+  mHandles.GetHandle(aHash, true, getter_AddRefs(handle));
 
   if (handle) {
     handle->Log();
 
+    if (handle->IsDoomed()) {
+      return aFailIfAlreadyDoomed ? NS_ERROR_NOT_AVAILABLE : NS_OK;
+    }
+
     return DoomFileInternal(handle);
   }
 
@@ -2234,7 +2244,7 @@ CacheFileIOManager::GetEntryInfo(const SHA1Sum::Hash *aHash,
   nsAutoCString uriSpec;
 
   nsRefPtr<CacheFileHandle> handle;
-  ioMan->mHandles.GetHandle(aHash, getter_AddRefs(handle));
+  ioMan->mHandles.GetHandle(aHash, false, getter_AddRefs(handle));
   if (handle) {
     nsRefPtr<nsILoadContextInfo> info =
       CacheFileUtils::ParseKey(handle->Key(), &enhanceId, &uriSpec);
@@ -2605,7 +2615,7 @@ CacheFileIOManager::OverLimitEvictionInternal()
     rv = CacheIndex::GetEntryForEviction(false, &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) {
@@ -2613,6 +2623,16 @@ 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;
@@ -3630,7 +3650,7 @@ CacheFileIOManager::OpenNSPRHandle(CacheFileHandle *aHandle, bool aCreate)
 
       rv = CacheIndex::GetEntryForEviction(true, &hash, &cnt);
       if (NS_SUCCEEDED(rv)) {
-        rv = DoomFileByKeyInternal(&hash);
+        rv = DoomFileByKeyInternal(&hash, true);
       }
       if (NS_SUCCEEDED(rv)) {
         rv = aHandle->mFile->OpenNSPRFileDesc(

netwerk/cache2/CacheFileIOManager.h

@@ -90,7 +90,7 @@ public:
   CacheFileHandles();
   ~CacheFileHandles();
 
-  nsresult GetHandle(const SHA1Sum::Hash *aHash, CacheFileHandle **_retval);
+  nsresult GetHandle(const SHA1Sum::Hash *aHash, bool aReturnDoomed, CacheFileHandle **_retval);
   nsresult NewHandle(const SHA1Sum::Hash *aHash, bool aPriority, CacheFileHandle **_retval);
   void     RemoveHandle(CacheFileHandle *aHandlle);
   void     GetAllHandles(nsTArray<nsRefPtr<CacheFileHandle> > *_retval);
@@ -330,7 +330,8 @@ private:
   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);

netwerk/cache2/CacheIndex.cpp

@@ -1277,7 +1277,7 @@ bool CacheIndex::IsForcedValidEntry(const SHA1Sum::Hash *aHash)
   nsRefPtr<CacheFileHandle> handle;
 
   CacheFileIOManager::gInstance->mHandles.GetHandle(
-    aHash, getter_AddRefs(handle));
+    aHash, false, getter_AddRefs(handle));
 
   if (!handle)
     return false;
@@ -2775,7 +2775,7 @@ CacheIndex::BuildIndex()
 
 #ifdef DEBUG
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
                                                       getter_AddRefs(handle));
 #endif
 
@@ -2988,7 +2988,7 @@ CacheIndex::UpdateIndex()
 
 #ifdef DEBUG
     nsRefPtr<CacheFileHandle> handle;
-    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash,
+    CacheFileIOManager::gInstance->mHandles.GetHandle(&hash, false,
                                                       getter_AddRefs(handle));
 #endif