Bug 673543 - crash nsDiskCacheMap::DeleteRecord r=michal.novotny

netwerk/cache/nsCacheEntry.cpp

@@ -250,10 +250,17 @@ bool
 nsCacheEntry::RemoveDescriptor(nsCacheEntryDescriptor * descriptor)
 {
     NS_ASSERTION(descriptor->CacheEntry() == this, "### Wrong cache entry!!");
-    descriptor->CloseOutput();
+    nsresult rv = descriptor->CloseOutput();
+    if (rv == NS_BASE_STREAM_WOULD_BLOCK)
+        return true;
+
     descriptor->ClearCacheEntry();
     PR_REMOVE_AND_INIT_LINK(descriptor);
 
+    // Doom entry if something bad happens while closing. See bug #673543
+    if (NS_FAILED(rv))
+        nsCacheService::DoomEntry(this);
+
     if (!PR_CLIST_IS_EMPTY(&mDescriptorQ))
         return true;  // stay active if we still have open descriptors
 
@@ -273,8 +280,15 @@ nsCacheEntry::DetachDescriptors(void)
     while (descriptor != &mDescriptorQ) {
         nsCacheEntryDescriptor * nextDescriptor =
             (nsCacheEntryDescriptor *)PR_NEXT_LINK(descriptor);
-        
-        descriptor->CloseOutput();
+
+        // Doom entry if something bad happens while closing. See bug #673543
+        // Errors are handled different from RemoveDescriptor because this
+        // method is only called from ClearDoomList (in which case the entry is
+        // doomed anyway) and ClearActiveEntries (in which case we are shutting
+        // down and really want to get rid of the entry immediately)
+        if (NS_FAILED(descriptor->CloseOutput()))
+            nsCacheService::DoomEntry(this);
+
         descriptor->ClearCacheEntry();
         PR_REMOVE_AND_INIT_LINK(descriptor);
         descriptor = nextDescriptor;

netwerk/cache/nsCacheEntryDescriptor.h

@@ -76,22 +76,24 @@ public:
     nsCacheEntry * CacheEntry(void)      { return mCacheEntry; }
     void           ClearCacheEntry(void) { mCacheEntry = nsnull; }
 
-    void           CloseOutput(void)
+    nsresult       CloseOutput(void)
     {
-      InternalCleanup(mOutput);
+      nsresult rv = InternalCleanup(mOutput);
       mOutput = nsnull;
+      return rv;
     }
 
 private:
-    void           InternalCleanup(nsIOutputStream *stream)
+    nsresult       InternalCleanup(nsIOutputStream *stream)
     {
       if (stream) {
         nsCOMPtr<nsIDiskCacheStreamInternal> tmp (do_QueryInterface(stream));
         if (tmp)
-          tmp->CloseInternal();
+          return tmp->CloseInternal();
         else
-          stream->Close();
+          return stream->Close();
       }
+      return NS_OK;
     }
 
 

netwerk/cache/nsDiskCacheBinding.cpp

@@ -43,6 +43,7 @@
 
 #include "nscore.h"
 #include "nsDiskCacheBinding.h"
+#include "nsCacheService.h"
 
 
 
@@ -124,7 +125,8 @@ nsDiskCacheBinding::~nsDiskCacheBinding()
     
     // sever streamIO/binding link
     if (mStreamIO) {
-        mStreamIO->ClearBinding();
+        if (NS_FAILED(mStreamIO->ClearBinding()))
+            nsCacheService::DoomEntry(mCacheEntry);
         NS_RELEASE(mStreamIO);
     }
 }

netwerk/cache/nsDiskCacheStreams.cpp

@@ -234,12 +234,13 @@ nsDiskCacheOutputStream::~nsDiskCacheOutputStream()
 NS_IMETHODIMP
 nsDiskCacheOutputStream::Close()
 {
+    nsresult rv = NS_OK;
     mozilla::TimeStamp start = mozilla::TimeStamp::Now();
 
     if (!mClosed) {
         mClosed = true;
         // tell parent streamIO we are closing
-        mStreamIO->CloseOutputStream(this);
+        rv = mStreamIO->CloseOutputStream(this);
     }
 
     mozilla::Telemetry::ID id;
@@ -250,18 +251,19 @@ nsDiskCacheOutputStream::Close()
 
     mozilla::Telemetry::AccumulateTimeDelta(id, start);
 
-    return NS_OK;
+    return rv;
 }
 
 NS_IMETHODIMP
 nsDiskCacheOutputStream::CloseInternal()
 {
+    nsresult rv = NS_OK;
     mozilla::TimeStamp start = mozilla::TimeStamp::Now();
 
     if (!mClosed) {
         mClosed = true;
         // tell parent streamIO we are closing
-        mStreamIO->CloseOutputStreamInternal(this);
+        rv = mStreamIO->CloseOutputStreamInternal(this);
     }
 
     mozilla::Telemetry::ID id;
@@ -272,7 +274,7 @@ nsDiskCacheOutputStream::CloseInternal()
 
     mozilla::Telemetry::AccumulateTimeDelta(id, start);
 
-    return NS_OK;
+    return rv;
 }
 
 NS_IMETHODIMP
@@ -459,12 +461,14 @@ nsDiskCacheStreamIO::GetOutputStream(PRUint32 offset, nsIOutputStream ** outputS
     return NS_OK;
 }
 
-void
+nsresult
 nsDiskCacheStreamIO::ClearBinding()
 {
+    nsresult rv = NS_OK;
     if (mBinding && mOutStream)
-        Flush();
+        rv = Flush();
     mBinding = nsnull;
+    return rv;
 }
 
 nsresult
@@ -535,7 +539,6 @@ nsDiskCacheStreamIO::Flush()
             rv = cacheMap->DeleteStorage(record, nsDiskCache::kData);
             if (NS_FAILED(rv)) {
                 NS_WARNING("cacheMap->DeleteStorage() failed.");
-                cacheMap->DeleteRecord(record);
                 return rv;
             }
         }

netwerk/cache/nsDiskCacheStreams.h

@@ -76,7 +76,7 @@ public:
     nsresult    Tell(PRUint32 * position);    
     nsresult    SetEOF();
 
-    void        ClearBinding();
+    nsresult    ClearBinding();
     
     void        IncrementInputStreamCount() { PR_ATOMIC_INCREMENT(&mInStreamCount); }
     void        DecrementInputStreamCount()