зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1074832 - CacheIndex merges journal with pending updates incorrectly, r=honzab
This commit is contained in:
Michal Novotny
Родитель
7cc3798870
Коммит
bfb912d53e
|
|
@ -53,7 +53,7 @@ public:
|
|||
mIndex->AssertOwnsLock();
|
||||
|
||||
mHash = aHash;
|
||||
CacheIndexEntry *entry = FindEntry();
|
||||
const CacheIndexEntry *entry = FindEntry();
|
||||
mIndex->mIndexStats.BeforeChange(entry);
|
||||
if (entry && entry->IsInitialized() && !entry->IsRemoved()) {
|
||||
mOldRecord = entry->mRec;
|
||||
|
|
@ -66,7 +66,7 @@ public:
|
|||
{
|
||||
mIndex->AssertOwnsLock();
|
||||
|
||||
CacheIndexEntry *entry = FindEntry();
|
||||
const CacheIndexEntry *entry = FindEntry();
|
||||
mIndex->mIndexStats.AfterChange(entry);
|
||||
if (!entry || !entry->IsInitialized() || entry->IsRemoved()) {
|
||||
entry = nullptr;
|
||||
|
|
@ -125,9 +125,9 @@ public:
|
|||
void DoNotSearchInUpdates() { mDoNotSearchInUpdates = true; }
|
||||
|
||||
private:
|
||||
CacheIndexEntry * FindEntry()
|
||||
const CacheIndexEntry * FindEntry()
|
||||
{
|
||||
CacheIndexEntry *entry = nullptr;
|
||||
const CacheIndexEntry *entry = nullptr;
|
||||
|
||||
switch (mIndex->mState) {
|
||||
case CacheIndex::READING:
|
||||
|
|
@ -520,6 +520,7 @@ CacheIndex::AddEntry(const SHA1Sum::Hash *aHash)
|
|||
|
||||
CacheIndexEntry *entry = index->mIndex.GetEntry(*aHash);
|
||||
bool entryRemoved = entry && entry->IsRemoved();
|
||||
CacheIndexEntryUpdate *updated = nullptr;
|
||||
|
||||
if (index->mState == READY || index->mState == UPDATING ||
|
||||
index->mState == BUILDING) {
|
||||
|
|
@ -558,7 +559,7 @@ CacheIndex::AddEntry(const SHA1Sum::Hash *aHash)
|
|||
entry = index->mIndex.PutEntry(*aHash);
|
||||
}
|
||||
} else { // WRITING, READING
|
||||
CacheIndexEntry *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
bool updatedRemoved = updated && updated->IsRemoved();
|
||||
|
||||
if ((updated && !updatedRemoved) ||
|
||||
|
|
@ -578,12 +579,17 @@ CacheIndex::AddEntry(const SHA1Sum::Hash *aHash)
|
|||
}
|
||||
|
||||
updated = index->mPendingUpdates.PutEntry(*aHash);
|
||||
entry = updated;
|
||||
}
|
||||
|
||||
entry->InitNew();
|
||||
entry->MarkDirty();
|
||||
entry->MarkFresh();
|
||||
if (updated) {
|
||||
updated->InitNew();
|
||||
updated->MarkDirty();
|
||||
updated->MarkFresh();
|
||||
} else {
|
||||
entry->InitNew();
|
||||
entry->MarkDirty();
|
||||
entry->MarkFresh();
|
||||
}
|
||||
}
|
||||
|
||||
if (updateIfNonFreshEntriesExist &&
|
||||
|
|
@ -652,7 +658,7 @@ CacheIndex::EnsureEntryExists(const SHA1Sum::Hash *aHash)
|
|||
}
|
||||
entry->MarkFresh();
|
||||
} else { // WRITING, READING
|
||||
CacheIndexEntry *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
CacheIndexEntryUpdate *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
bool updatedRemoved = updated && updated->IsRemoved();
|
||||
|
||||
if (updatedRemoved ||
|
||||
|
|
@ -732,6 +738,7 @@ CacheIndex::InitEntry(const SHA1Sum::Hash *aHash,
|
|||
CacheIndexEntryAutoManage entryMng(aHash, index);
|
||||
|
||||
CacheIndexEntry *entry = index->mIndex.GetEntry(*aHash);
|
||||
CacheIndexEntryUpdate *updated = nullptr;
|
||||
bool reinitEntry = false;
|
||||
|
||||
if (entry && entry->IsRemoved()) {
|
||||
|
|
@ -753,7 +760,7 @@ CacheIndex::InitEntry(const SHA1Sum::Hash *aHash,
|
|||
}
|
||||
}
|
||||
} else {
|
||||
CacheIndexEntry *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
DebugOnly<bool> removed = updated && updated->IsRemoved();
|
||||
|
||||
MOZ_ASSERT(updated || !removed);
|
||||
|
|
@ -770,7 +777,6 @@ CacheIndex::InitEntry(const SHA1Sum::Hash *aHash,
|
|||
return NS_OK;
|
||||
}
|
||||
}
|
||||
entry = updated;
|
||||
} else {
|
||||
MOZ_ASSERT(entry->IsFresh());
|
||||
|
||||
|
|
@ -786,18 +792,28 @@ CacheIndex::InitEntry(const SHA1Sum::Hash *aHash,
|
|||
// make a copy of a read-only entry
|
||||
updated = index->mPendingUpdates.PutEntry(*aHash);
|
||||
*updated = *entry;
|
||||
entry = updated;
|
||||
}
|
||||
}
|
||||
|
||||
if (reinitEntry) {
|
||||
// There is a collision and we are going to rewrite this entry. Initialize
|
||||
// it as a new entry.
|
||||
entry->InitNew();
|
||||
entry->MarkFresh();
|
||||
if (updated) {
|
||||
updated->InitNew();
|
||||
updated->MarkFresh();
|
||||
} else {
|
||||
entry->InitNew();
|
||||
entry->MarkFresh();
|
||||
}
|
||||
}
|
||||
|
||||
if (updated) {
|
||||
updated->Init(aAppId, aAnonymous, aInBrowser);
|
||||
updated->MarkDirty();
|
||||
} else {
|
||||
entry->Init(aAppId, aAnonymous, aInBrowser);
|
||||
entry->MarkDirty();
|
||||
}
|
||||
entry->Init(aAppId, aAnonymous, aInBrowser);
|
||||
entry->MarkDirty();
|
||||
}
|
||||
|
||||
index->StartUpdatingIndexIfNeeded();
|
||||
|
|
@ -865,7 +881,7 @@ CacheIndex::RemoveEntry(const SHA1Sum::Hash *aHash)
|
|||
}
|
||||
}
|
||||
} else { // WRITING, READING
|
||||
CacheIndexEntry *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
CacheIndexEntryUpdate *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
bool updatedRemoved = updated && updated->IsRemoved();
|
||||
|
||||
if (updatedRemoved ||
|
||||
|
|
@ -945,42 +961,58 @@ CacheIndex::UpdateEntry(const SHA1Sum::Hash *aHash,
|
|||
if (!HasEntryChanged(entry, aFrecency, aExpirationTime, aSize)) {
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
MOZ_ASSERT(entry->IsFresh());
|
||||
MOZ_ASSERT(entry->IsInitialized());
|
||||
entry->MarkDirty();
|
||||
|
||||
if (aFrecency) {
|
||||
entry->SetFrecency(*aFrecency);
|
||||
}
|
||||
|
||||
if (aExpirationTime) {
|
||||
entry->SetExpirationTime(*aExpirationTime);
|
||||
}
|
||||
|
||||
if (aSize) {
|
||||
entry->SetFileSize(*aSize);
|
||||
}
|
||||
} else {
|
||||
CacheIndexEntry *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
CacheIndexEntryUpdate *updated = index->mPendingUpdates.GetEntry(*aHash);
|
||||
DebugOnly<bool> removed = updated && updated->IsRemoved();
|
||||
|
||||
MOZ_ASSERT(updated || !removed);
|
||||
MOZ_ASSERT(updated || entry);
|
||||
|
||||
if (!updated) {
|
||||
if (entry &&
|
||||
HasEntryChanged(entry, aFrecency, aExpirationTime, aSize)) {
|
||||
// make a copy of a read-only entry
|
||||
updated = index->mPendingUpdates.PutEntry(*aHash);
|
||||
*updated = *entry;
|
||||
entry = updated;
|
||||
} else {
|
||||
if (!entry) {
|
||||
LOG(("CacheIndex::UpdateEntry() - Entry was found neither in mIndex "
|
||||
"nor in mPendingUpdates!"));
|
||||
NS_WARNING(("CacheIndex::UpdateEntry() - Entry was found neither in "
|
||||
"mIndex nor in mPendingUpdates!"));
|
||||
return NS_ERROR_NOT_AVAILABLE;
|
||||
}
|
||||
} else {
|
||||
entry = updated;
|
||||
|
||||
// make a copy of a read-only entry
|
||||
updated = index->mPendingUpdates.PutEntry(*aHash);
|
||||
*updated = *entry;
|
||||
}
|
||||
}
|
||||
|
||||
MOZ_ASSERT(entry->IsFresh());
|
||||
MOZ_ASSERT(entry->IsInitialized());
|
||||
entry->MarkDirty();
|
||||
MOZ_ASSERT(updated->IsFresh());
|
||||
MOZ_ASSERT(updated->IsInitialized());
|
||||
updated->MarkDirty();
|
||||
|
||||
if (aFrecency) {
|
||||
entry->SetFrecency(*aFrecency);
|
||||
}
|
||||
if (aFrecency) {
|
||||
updated->SetFrecency(*aFrecency);
|
||||
}
|
||||
|
||||
if (aExpirationTime) {
|
||||
entry->SetExpirationTime(*aExpirationTime);
|
||||
}
|
||||
if (aExpirationTime) {
|
||||
updated->SetExpirationTime(*aExpirationTime);
|
||||
}
|
||||
|
||||
if (aSize) {
|
||||
entry->SetFileSize(*aSize);
|
||||
if (aSize) {
|
||||
updated->SetFileSize(*aSize);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1096,7 +1128,7 @@ CacheIndex::HasEntry(const nsACString &aKey, EntryStatus *_retval)
|
|||
sum.update(aKey.BeginReading(), aKey.Length());
|
||||
sum.finish(hash);
|
||||
|
||||
CacheIndexEntry *entry = nullptr;
|
||||
const CacheIndexEntry *entry = nullptr;
|
||||
|
||||
switch (index->mState) {
|
||||
case READING:
|
||||
|
|
@ -1481,7 +1513,7 @@ CacheIndex::ProcessPendingOperations()
|
|||
|
||||
// static
|
||||
PLDHashOperator
|
||||
CacheIndex::UpdateEntryInIndex(CacheIndexEntry *aEntry, void* aClosure)
|
||||
CacheIndex::UpdateEntryInIndex(CacheIndexEntryUpdate *aEntry, void* aClosure)
|
||||
{
|
||||
CacheIndex *index = static_cast<CacheIndex *>(aClosure);
|
||||
|
||||
|
|
@ -1516,8 +1548,16 @@ CacheIndex::UpdateEntryInIndex(CacheIndexEntry *aEntry, void* aClosure)
|
|||
return PL_DHASH_REMOVE;
|
||||
}
|
||||
|
||||
entry = index->mIndex.PutEntry(*aEntry->Hash());
|
||||
*entry = *aEntry;
|
||||
if (entry) {
|
||||
// Some information in mIndex can be newer than in mPendingUpdates (see bug
|
||||
// 1074832). This will copy just those values that were really updated.
|
||||
aEntry->ApplyUpdate(entry);
|
||||
} else {
|
||||
// There is no entry in mIndex, copy all information from mPendingUpdates
|
||||
// to mIndex.
|
||||
entry = index->mIndex.PutEntry(*aEntry->Hash());
|
||||
*entry = *aEntry;
|
||||
}
|
||||
|
||||
return PL_DHASH_REMOVE;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -166,32 +166,32 @@ public:
|
|||
}
|
||||
}
|
||||
|
||||
const SHA1Sum::Hash * Hash() { return &mRec->mHash; }
|
||||
const SHA1Sum::Hash * Hash() const { return &mRec->mHash; }
|
||||
|
||||
bool IsInitialized() { return !!(mRec->mFlags & kInitializedMask); }
|
||||
bool IsInitialized() const { return !!(mRec->mFlags & kInitializedMask); }
|
||||
|
||||
uint32_t AppId() { return mRec->mAppId; }
|
||||
bool Anonymous() { return !!(mRec->mFlags & kAnonymousMask); }
|
||||
bool InBrowser() { return !!(mRec->mFlags & kInBrowserMask); }
|
||||
uint32_t AppId() const { return mRec->mAppId; }
|
||||
bool Anonymous() const { return !!(mRec->mFlags & kAnonymousMask); }
|
||||
bool InBrowser() const { return !!(mRec->mFlags & kInBrowserMask); }
|
||||
|
||||
bool IsRemoved() { return !!(mRec->mFlags & kRemovedMask); }
|
||||
bool IsRemoved() const { return !!(mRec->mFlags & kRemovedMask); }
|
||||
void MarkRemoved() { mRec->mFlags |= kRemovedMask; }
|
||||
|
||||
bool IsDirty() { return !!(mRec->mFlags & kDirtyMask); }
|
||||
bool IsDirty() const { return !!(mRec->mFlags & kDirtyMask); }
|
||||
void MarkDirty() { mRec->mFlags |= kDirtyMask; }
|
||||
void ClearDirty() { mRec->mFlags &= ~kDirtyMask; }
|
||||
|
||||
bool IsFresh() { return !!(mRec->mFlags & kFreshMask); }
|
||||
bool IsFresh() const { return !!(mRec->mFlags & kFreshMask); }
|
||||
void MarkFresh() { mRec->mFlags |= kFreshMask; }
|
||||
|
||||
void SetFrecency(uint32_t aFrecency) { mRec->mFrecency = aFrecency; }
|
||||
uint32_t GetFrecency() { return mRec->mFrecency; }
|
||||
uint32_t GetFrecency() const { return mRec->mFrecency; }
|
||||
|
||||
void SetExpirationTime(uint32_t aExpirationTime)
|
||||
{
|
||||
mRec->mExpirationTime = aExpirationTime;
|
||||
}
|
||||
uint32_t GetExpirationTime() { return mRec->mExpirationTime; }
|
||||
uint32_t GetExpirationTime() const { return mRec->mExpirationTime; }
|
||||
|
||||
// Sets filesize in kilobytes.
|
||||
void SetFileSize(uint32_t aFileSize)
|
||||
|
|
@ -205,12 +205,12 @@ public:
|
|||
mRec->mFlags |= aFileSize;
|
||||
}
|
||||
// Returns filesize in kilobytes.
|
||||
uint32_t GetFileSize() { return GetFileSize(mRec); }
|
||||
uint32_t GetFileSize() const { return GetFileSize(mRec); }
|
||||
static uint32_t GetFileSize(CacheIndexRecord *aRec)
|
||||
{
|
||||
return aRec->mFlags & kFileSizeMask;
|
||||
}
|
||||
bool IsFileEmpty() { return GetFileSize() == 0; }
|
||||
bool IsFileEmpty() const { return GetFileSize() == 0; }
|
||||
|
||||
void WriteToBuf(void *aBuf)
|
||||
{
|
||||
|
|
@ -246,7 +246,7 @@ public:
|
|||
mRec->mFlags = NetworkEndian::readUint32(&src->mFlags);
|
||||
}
|
||||
|
||||
void Log() {
|
||||
void Log() const {
|
||||
LOG(("CacheIndexEntry::Log() [this=%p, hash=%08x%08x%08x%08x%08x, fresh=%u,"
|
||||
" initialized=%u, removed=%u, dirty=%u, anonymous=%u, inBrowser=%u, "
|
||||
"appId=%u, frecency=%u, expirationTime=%u, size=%u]",
|
||||
|
|
@ -280,6 +280,7 @@ public:
|
|||
}
|
||||
|
||||
private:
|
||||
friend class CacheIndexEntryUpdate;
|
||||
friend class CacheIndex;
|
||||
friend class CacheIndexEntryAutoManage;
|
||||
|
||||
|
|
@ -308,6 +309,82 @@ private:
|
|||
nsAutoPtr<CacheIndexRecord> mRec;
|
||||
};
|
||||
|
||||
class CacheIndexEntryUpdate : public CacheIndexEntry
|
||||
{
|
||||
public:
|
||||
explicit CacheIndexEntryUpdate(CacheIndexEntry::KeyTypePointer aKey)
|
||||
: CacheIndexEntry(aKey)
|
||||
, mUpdateFlags(0)
|
||||
{
|
||||
MOZ_COUNT_CTOR(CacheIndexEntryUpdate);
|
||||
LOG(("CacheIndexEntryUpdate::CacheIndexEntryUpdate()"));
|
||||
}
|
||||
~CacheIndexEntryUpdate()
|
||||
{
|
||||
MOZ_COUNT_DTOR(CacheIndexEntryUpdate);
|
||||
LOG(("CacheIndexEntryUpdate::~CacheIndexEntryUpdate()"));
|
||||
}
|
||||
|
||||
CacheIndexEntryUpdate& operator=(const CacheIndexEntry& aOther)
|
||||
{
|
||||
MOZ_ASSERT(memcmp(&mRec->mHash, &aOther.mRec->mHash,
|
||||
sizeof(SHA1Sum::Hash)) == 0);
|
||||
mUpdateFlags = 0;
|
||||
*(static_cast<CacheIndexEntry *>(this)) = aOther;
|
||||
return *this;
|
||||
}
|
||||
|
||||
void InitNew()
|
||||
{
|
||||
mUpdateFlags = 0;
|
||||
CacheIndexEntry::InitNew();
|
||||
}
|
||||
|
||||
void SetFrecency(uint32_t aFrecency)
|
||||
{
|
||||
mUpdateFlags |= kFrecencyUpdatedMask;
|
||||
CacheIndexEntry::SetFrecency(aFrecency);
|
||||
}
|
||||
|
||||
void SetExpirationTime(uint32_t aExpirationTime)
|
||||
{
|
||||
mUpdateFlags |= kExpirationUpdatedMask;
|
||||
CacheIndexEntry::SetExpirationTime(aExpirationTime);
|
||||
}
|
||||
|
||||
void SetFileSize(uint32_t aFileSize)
|
||||
{
|
||||
mUpdateFlags |= kFileSizeUpdatedMask;
|
||||
CacheIndexEntry::SetFileSize(aFileSize);
|
||||
}
|
||||
|
||||
void ApplyUpdate(CacheIndexEntry *aDst) {
|
||||
MOZ_ASSERT(memcmp(&mRec->mHash, &aDst->mRec->mHash,
|
||||
sizeof(SHA1Sum::Hash)) == 0);
|
||||
if (mUpdateFlags & kFrecencyUpdatedMask) {
|
||||
aDst->mRec->mFrecency = mRec->mFrecency;
|
||||
}
|
||||
if (mUpdateFlags & kExpirationUpdatedMask) {
|
||||
aDst->mRec->mExpirationTime = mRec->mExpirationTime;
|
||||
}
|
||||
aDst->mRec->mAppId = mRec->mAppId;
|
||||
if (mUpdateFlags & kFileSizeUpdatedMask) {
|
||||
aDst->mRec->mFlags = mRec->mFlags;
|
||||
} else {
|
||||
// Copy all flags except file size.
|
||||
aDst->mRec->mFlags &= kFileSizeMask;
|
||||
aDst->mRec->mFlags |= (mRec->mFlags & ~kFileSizeMask);
|
||||
}
|
||||
}
|
||||
|
||||
private:
|
||||
static const uint32_t kFrecencyUpdatedMask = 0x00000001;
|
||||
static const uint32_t kExpirationUpdatedMask = 0x00000002;
|
||||
static const uint32_t kFileSizeUpdatedMask = 0x00000004;
|
||||
|
||||
uint32_t mUpdateFlags;
|
||||
};
|
||||
|
||||
class CacheIndexStats
|
||||
{
|
||||
public:
|
||||
|
|
@ -397,7 +474,7 @@ public:
|
|||
return mSize;
|
||||
}
|
||||
|
||||
void BeforeChange(CacheIndexEntry *aEntry) {
|
||||
void BeforeChange(const CacheIndexEntry *aEntry) {
|
||||
#ifdef DEBUG_STATS
|
||||
if (!mDisableLogging) {
|
||||
LOG(("CacheIndexStats::BeforeChange()"));
|
||||
|
|
@ -441,7 +518,7 @@ public:
|
|||
}
|
||||
}
|
||||
|
||||
void AfterChange(CacheIndexEntry *aEntry) {
|
||||
void AfterChange(const CacheIndexEntry *aEntry) {
|
||||
MOZ_ASSERT(mStateLogged, "CacheIndexStats::AfterChange() - state not "
|
||||
"logged!");
|
||||
#ifdef DEBUG
|
||||
|
|
@ -642,7 +719,7 @@ private:
|
|||
|
||||
// Merge all pending operations from mPendingUpdates into mIndex.
|
||||
void ProcessPendingOperations();
|
||||
static PLDHashOperator UpdateEntryInIndex(CacheIndexEntry *aEntry,
|
||||
static PLDHashOperator UpdateEntryInIndex(CacheIndexEntryUpdate *aEntry,
|
||||
void* aClosure);
|
||||
|
||||
// Following methods perform writing of the index file.
|
||||
|
|
@ -929,7 +1006,7 @@ private:
|
|||
|
||||
// We cannot add, remove or change any entry in mIndex in states READING and
|
||||
// WRITING. We track all changes in mPendingUpdates during these states.
|
||||
nsTHashtable<CacheIndexEntry> mPendingUpdates;
|
||||
nsTHashtable<CacheIndexEntryUpdate> mPendingUpdates;
|
||||
|
||||
// Contains information statistics for mIndex + mPendingUpdates.
|
||||
CacheIndexStats mIndexStats;
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче