Bug 1277826 - Crash in shutdownhang | mozilla::net::CacheIndex::WriteLogToDisk, r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Sun, 04 Sep 2016 02:39:49 +0200
changeset 312636 8225b084f18f282820d9d3e0b81c5351496da437
parent 312622 29f26ec3baed14a0c761fbb7f5d184fcfd7b13a9
child 312637 9c8d5aaeb00e5174f5c5caaf5003048ddb3745f9
push id30651
push userryanvm@gmail.com
push dateSun, 04 Sep 2016 17:37:08 +0000
treeherdermozilla-central@dbe4b47941c7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1277826
milestone51.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 1277826 - Crash in shutdownhang | mozilla::net::CacheIndex::WriteLogToDisk, r=honzab
netwerk/cache2/CacheIndex.cpp
netwerk/cache2/CacheIndex.h
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -436,35 +436,35 @@ CacheIndex::Shutdown()
 
   switch (oldState) {
     case WRITING:
       index->FinishWrite(false);
       MOZ_FALLTHROUGH;
     case READY:
       if (index->mIndexOnDiskIsValid && !index->mDontMarkIndexClean) {
         if (!sanitize && NS_FAILED(index->WriteLogToDisk())) {
-          index->RemoveIndexFromDisk();
+          index->RemoveJournalAndTempFile();
         }
       } else {
-        index->RemoveIndexFromDisk();
+        index->RemoveJournalAndTempFile();
       }
       break;
     case READING:
       index->FinishRead(false);
       break;
     case BUILDING:
     case UPDATING:
       index->FinishUpdate(false);
       break;
     default:
       MOZ_ASSERT(false, "Unexpected state!");
   }
 
   if (sanitize) {
-    index->RemoveIndexFromDisk();
+    index->RemoveAllIndexFiles();
   }
 
   return NS_OK;
 }
 
 // static
 nsresult
 CacheIndex::AddEntry(const SHA1Sum::Hash *aHash)
@@ -1834,31 +1834,36 @@ CacheIndex::RemoveFile(const nsACString 
       return rv;
     }
   }
 
   return NS_OK;
 }
 
 void
-CacheIndex::RemoveIndexFromDisk()
+CacheIndex::RemoveAllIndexFiles()
 {
-  LOG(("CacheIndex::RemoveIndexFromDisk()"));
-
+  LOG(("CacheIndex::RemoveAllIndexFiles()"));
   RemoveFile(NS_LITERAL_CSTRING(INDEX_NAME));
+  RemoveJournalAndTempFile();
+}
+
+void
+CacheIndex::RemoveJournalAndTempFile()
+{
+  LOG(("CacheIndex::RemoveJournalAndTempFile()"));
   RemoveFile(NS_LITERAL_CSTRING(TEMP_INDEX_NAME));
   RemoveFile(NS_LITERAL_CSTRING(JOURNAL_NAME));
 }
 
 class WriteLogHelper
 {
 public:
   explicit WriteLogHelper(PRFileDesc *aFD)
-    : mStatus(NS_OK)
-    , mFD(aFD)
+    : mFD(aFD)
     , mBufSize(kMaxBufSize)
     , mBufPos(0)
   {
     mHash = new CacheHash();
     mBuf = static_cast<char *>(moz_xmalloc(mBufSize));
   }
 
   ~WriteLogHelper() {
@@ -1867,83 +1872,70 @@ public:
 
   nsresult AddEntry(CacheIndexEntry *aEntry);
   nsresult Finish();
 
 private:
 
   nsresult FlushBuffer();
 
-  nsresult            mStatus;
-  PRFileDesc         *mFD;
-  char               *mBuf;
-  uint32_t            mBufSize;
-  int32_t             mBufPos;
+  PRFileDesc       *mFD;
+  char             *mBuf;
+  uint32_t          mBufSize;
+  int32_t           mBufPos;
   RefPtr<CacheHash> mHash;
 };
 
 nsresult
 WriteLogHelper::AddEntry(CacheIndexEntry *aEntry)
 {
   nsresult rv;
 
-  if (NS_FAILED(mStatus)) {
-    return mStatus;
-  }
-
   if (mBufPos + sizeof(CacheIndexRecord) > mBufSize) {
     mHash->Update(mBuf, mBufPos);
 
     rv = FlushBuffer();
-    if (NS_FAILED(rv)) {
-      mStatus = rv;
-      return rv;
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
     MOZ_ASSERT(mBufPos + sizeof(CacheIndexRecord) <= mBufSize);
   }
 
   aEntry->WriteToBuf(mBuf + mBufPos);
   mBufPos += sizeof(CacheIndexRecord);
 
   return NS_OK;
 }
 
 nsresult
 WriteLogHelper::Finish()
 {
   nsresult rv;
 
-  if (NS_FAILED(mStatus)) {
-    return mStatus;
-  }
-
   mHash->Update(mBuf, mBufPos);
   if (mBufPos + sizeof(CacheHash::Hash32_t) > mBufSize) {
     rv = FlushBuffer();
-    if (NS_FAILED(rv)) {
-      mStatus = rv;
-      return rv;
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
     MOZ_ASSERT(mBufPos + sizeof(CacheHash::Hash32_t) <= mBufSize);
   }
 
   NetworkEndian::writeUint32(mBuf + mBufPos, mHash->GetHash());
   mBufPos += sizeof(CacheHash::Hash32_t);
 
   rv = FlushBuffer();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  mStatus = NS_ERROR_UNEXPECTED; // Don't allow any other operation
   return NS_OK;
 }
 
 nsresult
 WriteLogHelper::FlushBuffer()
 {
-  MOZ_ASSERT(NS_SUCCEEDED(mStatus));
+  if (CacheObserver::IsPastShutdownIOLag()) {
+    LOG(("WriteLogHelper::FlushBuffer() - Interrupting writing journal."));
+    return NS_ERROR_FAILURE;
+  }
 
   int32_t bytesWritten = PR_Write(mFD, mBuf, mBufPos);
 
   if (bytesWritten != mBufPos) {
     return NS_ERROR_FAILURE;
   }
 
   mBufPos = 0;
@@ -1955,16 +1947,21 @@ CacheIndex::WriteLogToDisk()
 {
   LOG(("CacheIndex::WriteLogToDisk()"));
 
   nsresult rv;
 
   MOZ_ASSERT(mPendingUpdates.Count() == 0);
   MOZ_ASSERT(mState == SHUTDOWN);
 
+  if (CacheObserver::IsPastShutdownIOLag()) {
+    LOG(("CacheIndex::WriteLogToDisk() - Skipping writing journal."));
+    return NS_ERROR_FAILURE;
+  }
+
   RemoveFile(NS_LITERAL_CSTRING(TEMP_INDEX_NAME));
 
   nsCOMPtr<nsIFile> indexFile;
   rv = GetFile(NS_LITERAL_CSTRING(INDEX_NAME), getter_AddRefs(indexFile));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIFile> logFile;
   rv = GetFile(NS_LITERAL_CSTRING(JOURNAL_NAME), getter_AddRefs(logFile));
@@ -1976,19 +1973,21 @@ CacheIndex::WriteLogToDisk()
   rv = logFile->OpenNSPRFileDesc(PR_RDWR | PR_CREATE_FILE | PR_TRUNCATE,
                                  0600, &fd);
   NS_ENSURE_SUCCESS(rv, rv);
 
   WriteLogHelper wlh(fd);
   for (auto iter = mIndex.Iter(); !iter.Done(); iter.Next()) {
     CacheIndexEntry* entry = iter.Get();
     if (entry->IsRemoved() || entry->IsDirty()) {
-      wlh.AddEntry(entry);
+      rv = wlh.AddEntry(entry);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
     }
-    iter.Remove();
   }
 
   rv = wlh.Finish();
   PR_Close(fd);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = indexFile->OpenNSPRFileDesc(PR_RDWR, 0600, &fd);
   NS_ENSURE_SUCCESS(rv, rv);
--- a/netwerk/cache2/CacheIndex.h
+++ b/netwerk/cache2/CacheIndex.h
@@ -767,17 +767,18 @@ private:
   // methods must be called only during shutdown since they write/delete files
   // directly on the main thread instead of using CacheFileIOManager that does
   // it asynchronously on IO thread. Journal contains only entries that are
   // dirty, i.e. changes that are not present in the index file on the disk.
   // When the log is written successfully, the dirty flag in index file is
   // cleared.
   nsresult GetFile(const nsACString &aName, nsIFile **_retval);
   nsresult RemoveFile(const nsACString &aName);
-  void     RemoveIndexFromDisk();
+  void     RemoveAllIndexFiles();
+  void     RemoveJournalAndTempFile();
   // Writes journal to the disk and clears dirty flag in index header.
   nsresult WriteLogToDisk();
 
   // Following methods perform reading of the index from the disk.
   //
   // Index is read at startup just after initializing the CacheIndex. There are
   // 3 files used when manipulating with index: index file, journal file and
   // a temporary file. All files contain the hash of the data, so we can check