Fix for smoketest blocker bug 81799 "Infinite loop in disk cache", sr=darin.

netwerk/cache/src/nsDiskCacheBinding.cpp

@@ -290,6 +290,7 @@ nsDiskCacheBindery::AddBinding(nsDiskCacheBinding * binding)
             PR_INSERT_BEFORE(binding, p);
             if (hashEntry->mBinding == p)
                 hashEntry->mBinding = binding;
+            break;
         }
         
         if (binding->mGeneration == p->mGeneration) {
@@ -309,6 +310,7 @@ nsDiskCacheBindery::AddBinding(nsDiskCacheBinding * binding)
                 return NS_ERROR_UNEXPECTED;
             }
             PR_INSERT_BEFORE(binding, hashEntry->mBinding);
+            break;
         }
     }
     return NS_OK;

netwerk/cache/src/nsDiskCacheDevice.cpp

@@ -614,7 +614,7 @@ nsDiskCacheDevice::FindEntry(nsCString * key)
     
     nsDiskCacheEntry * diskEntry;
     rv = mCacheMap->ReadDiskCacheEntry(&record, &diskEntry);
-    if (NS_FAILED(rv))  goto error_exit;
+    if (NS_FAILED(rv))  return nsnull;
     
     // compare key to be sure
     if (nsCRT::strcmp(diskEntry->mKeyStart, key->get()) == 0) {
@@ -622,16 +622,16 @@ nsDiskCacheDevice::FindEntry(nsCString * key)
     }
     delete diskEntry;
     
+    // If we had a hash collision or CreateCacheEntry failed, return nsnull
+    if (!entry)  return nsnull;
+    
     binding = mBindery.CreateBinding(entry, &record);
     if (!binding) {
-        goto error_exit;
+        delete entry;
+        return nsnull;
     }
     
     return entry;
-    
-error_exit:
-    delete entry;
-    return nsnull;
 }
 
 
@@ -684,7 +684,7 @@ nsDiskCacheDevice::BindEntry(nsCacheEntry * entry)
     
     // create a new record for this entry
     record.SetHashNumber(nsDiskCache::Hash(entry->Key()->get()));
-    record.SetEvictionRank(ULONG_MAX - entry->ExpirationTime());
+    record.SetEvictionRank(ULONG_MAX - SecondsFromPRTime(PR_Now()));
 
     if (!entry->IsDoomed()) {
         // if entry isn't doomed, add it to the cache map
@@ -776,12 +776,11 @@ nsDiskCacheDevice::GetTransportForEntry(nsCacheEntry * entry,
     } else {
         binding->mRecord.SetDataFileGeneration(binding->mGeneration);
         binding->mRecord.SetDataFileSize(0);    // 1k minimum
-    }
-
-    if (!binding->mDoomed) {
-        // record stored in cache map, so update it
-        rv = mCacheMap->UpdateRecord(&binding->mRecord);
-        if (NS_FAILED(rv))  return rv;
+        if (!binding->mDoomed) {
+            // record stored in cache map, so update it
+            rv = mCacheMap->UpdateRecord(&binding->mRecord);
+            if (NS_FAILED(rv))  return rv;
+        }
     }
 
     // generate the name of the cache entry from the hash code of its key,
@@ -826,12 +825,11 @@ nsDiskCacheDevice::GetFileForEntry(nsCacheEntry *    entry,
     } else {
         binding->mRecord.SetDataFileGeneration(binding->mGeneration);
         binding->mRecord.SetDataFileSize(0);    // 1k minimum
-    }
-        
-     if (!binding->mDoomed) {
-        // record stored in cache map, so update it
-       rv = mCacheMap->UpdateRecord(&binding->mRecord);
-        if (NS_FAILED(rv))  return rv;
+        if (!binding->mDoomed) {
+            // record stored in cache map, so update it
+            rv = mCacheMap->UpdateRecord(&binding->mRecord);
+            if (NS_FAILED(rv))  return rv;
+        }
     }
     
     nsCOMPtr<nsIFile>  file;

netwerk/cache/src/nsDiskCacheMap.cpp

@@ -26,6 +26,8 @@
 #include "nsDiskCacheBinding.h"
 #include "nsDiskCacheEntry.h"
 
+#include "nsCache.h"
+
 #include "nsCRT.h"
 
 #include <string.h>
@@ -80,12 +82,13 @@ nsDiskCacheBucket::CountRecords()
 
 
 PRUint32
-nsDiskCacheBucket::EvictionRank()
+nsDiskCacheBucket::EvictionRank(PRUint32 targetRank)
 {
     PRUint32  rank = 0;
     for (int i = CountRecords() - 1; i >= 0; --i) {
-        if (rank < mRecords[i].EvictionRank())
-            rank = mRecords[i].EvictionRank();
+        if ((rank < mRecords[i].EvictionRank()) &&
+            ((targetRank == 0) || (mRecords[i].EvictionRank() < targetRank)))
+                rank = mRecords[i].EvictionRank();
     }
     return rank;
 }
@@ -100,7 +103,7 @@ nsDiskCacheBucket::VisitEachRecord(nsDiskCacheRecordVisitor *  visitor,
     PRInt32  rv = kVisitNextRecord;
     PRInt32  last  = CountRecords() - 1;
     
-    // call visitor for each entry (equal or greater than evictionRank)
+    // call visitor for each entry (matching any eviction rank)
     for (int i = last; i >= 0; i--) {
         if (evictionRank > mRecords[i].EvictionRank())  continue;
     
@@ -329,7 +332,8 @@ nsDiskCacheMap::AddRecord( nsDiskCacheRecord *  mapRecord,
             if (mHeader.mEvictionRank[bucketIndex] < mapRecord->EvictionRank())
                 mHeader.mEvictionRank[bucketIndex] = mapRecord->EvictionRank();
 
-NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(), "whao!");
+            NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(0),
+                         "eviction rank out of sync");
             return NS_OK;
         }
         
@@ -344,10 +348,11 @@ NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(), "whao
     if ((oldRecord->HashNumber() != 0) ||
          (mapRecord->EvictionRank() > mHeader.mEvictionRank[bucketIndex])) {
          
-        mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank();
+        mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank(0);
     }
 
-NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(), "whao!");    
+NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(0),
+             "eviction rank out of sync");    
     return NS_OK;
 }
 
@@ -372,9 +377,10 @@ nsDiskCacheMap::UpdateRecord( nsDiskCacheRecord *  mapRecord)
             if (mHeader.mEvictionRank[bucketIndex] < mapRecord->EvictionRank())
                 mHeader.mEvictionRank[bucketIndex] = mapRecord->EvictionRank();
             else if (mHeader.mEvictionRank[bucketIndex] == oldRank)
-                mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank();
+                mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank(0);
 
-NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(), "whao!");
+NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(0),
+             "eviction rank out of sync");
             return NS_OK;
         }
     }
@@ -423,10 +429,11 @@ nsDiskCacheMap::DeleteRecord( nsDiskCacheRecord *  mapRecord)
             // update eviction rank
             PRUint32  bucketIndex = GetBucketIndex(mapRecord->HashNumber());
             if (mHeader.mEvictionRank[bucketIndex] <= evictionRank) {
-                mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank();
+                mHeader.mEvictionRank[bucketIndex] = bucket->EvictionRank(0);
             }
 
-NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(), "whao!");
+            NS_ASSERTION(mHeader.mEvictionRank[bucketIndex] == bucket->EvictionRank(0),
+                         "eviction rank out of sync");
             return NS_OK;
         }
     }
@@ -448,11 +455,12 @@ nsDiskCacheMap::VisitRecords( nsDiskCacheRecordVisitor *  visitor)
         PRBool continueFlag = mBuckets[i].VisitEachRecord(visitor, 0, &recordsDeleted);
         if (recordsDeleted) {
             // recalc eviction rank
-            mHeader.mEvictionRank[i] = mBuckets[i].EvictionRank();
+            mHeader.mEvictionRank[i] = mBuckets[i].EvictionRank(0);
             mHeader.mEntryCount -= recordsDeleted;
             // XXX write bucket
         }
-NS_ASSERTION(mHeader.mEvictionRank[i] == mBuckets[i].EvictionRank(), "whao!");
+        NS_ASSERTION(mHeader.mEvictionRank[i] == mBuckets[i].EvictionRank(0),
+                     "eviction rank out of sync");
         if (!continueFlag)  break;
     }
     
@@ -468,34 +476,41 @@ NS_ASSERTION(mHeader.mEvictionRank[i] == mBuckets[i].EvictionRank(), "whao!");
 nsresult
 nsDiskCacheMap::EvictRecords( nsDiskCacheRecordVisitor * visitor)
 {
+    PRUint32  tempRank[kBucketsPerTable];
+    int       i;
+    
+    // copy eviction rank array
+    for (i = 0; i < kBucketsPerTable; ++i)
+        tempRank[i] = mHeader.mEvictionRank[i];
+    
     while (1) {
     
         // find bucket with highest eviction rank
         PRUint32    rank  = 0;
         PRUint32    index = 0;
-        for (int i = 0; i < kBucketsPerTable; ++i) {
-            if (rank < mHeader.mEvictionRank[i]) {
-                rank = mHeader.mEvictionRank[i];
+        for (i = 0; i < kBucketsPerTable; ++i) {
+            if (rank < tempRank[i]) {
+                rank = tempRank[i];
                 index = i;
             }
         }
         
-NS_ASSERTION(mHeader.mEvictionRank[index] == mBuckets[index].EvictionRank(),
-    "header eviction rank out of sync");
+NS_ASSERTION(mHeader.mEvictionRank[index] == mBuckets[index].EvictionRank(0),
+             "header eviction rank out of sync");
             
         // visit records in bucket with eviction ranks >= target eviction rank
         PRUint32 recordsDeleted;
-        PRBool continueFlag = mBuckets[index].VisitEachRecord(visitor, rank, &recordsDeleted);
+        PRInt32  continueResult = mBuckets[index].VisitEachRecord(visitor, rank, &recordsDeleted);
         if (recordsDeleted) {
             // recalc eviction rank
-            mHeader.mEvictionRank[index] = mBuckets[index].EvictionRank();
+            mHeader.mEvictionRank[index] = mBuckets[index].EvictionRank(0);
             mHeader.mEntryCount -= recordsDeleted;
             // XXX write bucket
         }
-        if (!continueFlag)  break;
-        
-        // break if visitor returned stop
+        if (continueResult == kStopVisitingRecords)  break;
         
+        // find greatest rank less than 'rank'
+        tempRank[index] = mBuckets[index].EvictionRank(rank);
     }
     return NS_OK;
 }
@@ -644,6 +659,8 @@ nsDiskCacheMap::WriteDiskCacheEntry(nsDiskCacheBinding *  binding)
             if (NS_FAILED(rv))  return rv;
         }
     }
+
+    binding->mRecord.SetEvictionRank(ULONG_MAX - SecondsFromPRTime(PR_Now()));
         
     if (fileIndex == 0) {
         // Write entry data to separate file

netwerk/cache/src/nsDiskCacheMap.h

@@ -114,7 +114,7 @@ public:
 
     // EvictionRank accessors
     PRUint32  EvictionRank() const              { return mEvictionRank; }
-    void      SetEvictionRank( PRUint32 rank)   { mEvictionRank = rank; }
+    void      SetEvictionRank( PRUint32 rank)   { mEvictionRank = rank ? rank : 1; }
 
     // DataLocation accessors
     PRBool    DataLocationInitialized() { return mDataLocation & eLocationInitializedMask; }
@@ -311,7 +311,7 @@ struct nsDiskCacheBucket {
     void      Swap();
     void      Unswap();
     PRUint32  CountRecords();
-    PRUint32  EvictionRank();   // return largest rank in bucket
+    PRUint32  EvictionRank(PRUint32  targetRank);   // return largest rank in bucket < targetRank
     PRInt32   VisitEachRecord( nsDiskCacheRecordVisitor *  visitor,
                                PRUint32                    evictionRank,
                                PRUint32 *                  recordsDeleted);