From 8c99402512b6acfbd7b27e137da95730c8961a9e Mon Sep 17 00:00:00 2001 From: "gordon%netscape.com" Date: Mon, 21 May 2001 22:05:10 +0000 Subject: [PATCH] Fix for smoketest blocker bug 81799 "Infinite loop in disk cache", sr=darin. --- netwerk/cache/src/nsDiskCacheBinding.cpp | 2 + netwerk/cache/src/nsDiskCacheDevice.cpp | 36 +++++++------- netwerk/cache/src/nsDiskCacheMap.cpp | 63 +++++++++++++++--------- netwerk/cache/src/nsDiskCacheMap.h | 4 +- 4 files changed, 61 insertions(+), 44 deletions(-) diff --git a/netwerk/cache/src/nsDiskCacheBinding.cpp b/netwerk/cache/src/nsDiskCacheBinding.cpp index 6449d170190..6ee3898c699 100644 --- a/netwerk/cache/src/nsDiskCacheBinding.cpp +++ b/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; diff --git a/netwerk/cache/src/nsDiskCacheDevice.cpp b/netwerk/cache/src/nsDiskCacheDevice.cpp index a3d24a9bd2b..c5b89cdf4c7 100644 --- a/netwerk/cache/src/nsDiskCacheDevice.cpp +++ b/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 file; diff --git a/netwerk/cache/src/nsDiskCacheMap.cpp b/netwerk/cache/src/nsDiskCacheMap.cpp index 180d2dcc77d..28514c71a9a 100644 --- a/netwerk/cache/src/nsDiskCacheMap.cpp +++ b/netwerk/cache/src/nsDiskCacheMap.cpp @@ -26,6 +26,8 @@ #include "nsDiskCacheBinding.h" #include "nsDiskCacheEntry.h" +#include "nsCache.h" + #include "nsCRT.h" #include @@ -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 diff --git a/netwerk/cache/src/nsDiskCacheMap.h b/netwerk/cache/src/nsDiskCacheMap.h index 5dbdd3f833a..aad883aa254 100644 --- a/netwerk/cache/src/nsDiskCacheMap.h +++ b/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);