diff options
20 files changed, 38 insertions, 192 deletions
diff --git a/net/data/cache_tests/dirty_entry/contents.txt b/net/data/cache_tests/dirty_entry/contents.txt deleted file mode 100644 index 6cece3e..0000000 --- a/net/data/cache_tests/dirty_entry/contents.txt +++ /dev/null @@ -1,67 +0,0 @@ -Index header: -num_entries: 2 -num_bytes: 27 -this_id: 1 -table_len: 64k - -head: 0x90000001 -tail: 0x90000000 - -Address: 0xa0010002 -Address: 0xa0010003 - -------------------------------- - -entry: -Address: 0xa0010002 -hash: 0x687d1422 -next: 0 -rankings_node: 0x90000000 -key_len: 13 -long_key: 0 -data_size: 0's -data_addr: 0's -key: "the first key" - -rankings: -Address: 0x90000000 -next: 0x90000000 -prev: 0x90000001 -contents: 0xa0010002 -dirty: 0 -pointer: 0 - -------------------------------- - -entry: -Address: 0xa0010003 -hash: 0x63909ecb -next: 0 -rankings_node: 0x90000001 -key_len: 14 -long_key: 0 -data_size: 0's -data_addr: 0's -key: "some other key" - -rankings: -Address: 0x90000001 -next: 0x90000000 -prev: 0x90000001 -contents: 0xa0010003 -dirty: 0 -pointer: 0x0169dc48 <- Invalid. - -================================ - -Generated with: - -disk_cache::Entry *entry; -ASSERT_TRUE(cache_->CreateEntry("the first key", &entry)); -entry->Close(); - -ASSERT_TRUE(cache_->CreateEntry("some other key", &entry)); -entry->Close(); <---- Edit value* - -* Edit the value with the debugger before it is saved to disk (break on -the destructor of EntryImpl and skip the line that clears "pointer")
\ No newline at end of file diff --git a/net/data/cache_tests/dirty_entry/data_0 b/net/data/cache_tests/dirty_entry/data_0 Binary files differdeleted file mode 100644 index 1d60495..0000000 --- a/net/data/cache_tests/dirty_entry/data_0 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry/data_1 b/net/data/cache_tests/dirty_entry/data_1 Binary files differdeleted file mode 100644 index 446ac45..0000000 --- a/net/data/cache_tests/dirty_entry/data_1 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry/data_2 b/net/data/cache_tests/dirty_entry/data_2 Binary files differdeleted file mode 100644 index c7e2eb9..0000000 --- a/net/data/cache_tests/dirty_entry/data_2 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry/data_3 b/net/data/cache_tests/dirty_entry/data_3 Binary files differdeleted file mode 100644 index 5eec973..0000000 --- a/net/data/cache_tests/dirty_entry/data_3 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry/index b/net/data/cache_tests/dirty_entry/index Binary files differdeleted file mode 100644 index ba2b6ca..0000000 --- a/net/data/cache_tests/dirty_entry/index +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry2/contents.txt b/net/data/cache_tests/dirty_entry2/contents.txt deleted file mode 100644 index cde34ce..0000000 --- a/net/data/cache_tests/dirty_entry2/contents.txt +++ /dev/null @@ -1,67 +0,0 @@ -Index header: -num_entries: 2 -num_bytes: 27 -this_id: 1 -table_len: 64k - -head: 0x90000001 -tail: 0x90000000 - -Address: 0xa0010002 -Address: 0xa0010003 - -------------------------------- - -entry: -Address: 0xa0010002 -hash: 0x687d1422 -next: 0 -rankings_node: 0x90000000 -key_len: 13 -long_key: 0 -data_size: 0's -data_addr: 0's -key: "the first key" - -rankings: -Address: 0x90000000 -next: 0x90000000 -prev: 0x90000001 -contents: 0xa0010002 -dirty: 0 -pointer: 0 - -------------------------------- - -entry: -Address: 0xa0010003 -hash: 0x63909ecb -next: 0 -rankings_node: 0x90000001 -key_len: 14 -long_key: 0 -data_size: 0's -data_addr: 0's -key: "some other key" - -rankings: -Address: 0x90000001 -next: 0x90000000 -prev: 0x90000001 -contents: 0xa0010003 -dirty: 2 <- Next id!. -pointer: 0x0169dc48 <- Invalid. - -================================ - -Generated with: - -disk_cache::Entry *entry; -ASSERT_TRUE(cache_->CreateEntry("the first key", &entry)); -entry->Close(); - -ASSERT_TRUE(cache_->CreateEntry("some other key", &entry)); -entry->Close(); <---- Edit value* - -* Edit the value with the debugger before it is saved to disk (break on -the destructor of EntryImpl and skip the line that clears "pointer")
\ No newline at end of file diff --git a/net/data/cache_tests/dirty_entry2/data_0 b/net/data/cache_tests/dirty_entry2/data_0 Binary files differdeleted file mode 100644 index 5ba0deb..0000000 --- a/net/data/cache_tests/dirty_entry2/data_0 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry2/data_1 b/net/data/cache_tests/dirty_entry2/data_1 Binary files differdeleted file mode 100644 index 446ac45..0000000 --- a/net/data/cache_tests/dirty_entry2/data_1 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry2/data_2 b/net/data/cache_tests/dirty_entry2/data_2 Binary files differdeleted file mode 100644 index c7e2eb9..0000000 --- a/net/data/cache_tests/dirty_entry2/data_2 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry2/data_3 b/net/data/cache_tests/dirty_entry2/data_3 Binary files differdeleted file mode 100644 index 5eec973..0000000 --- a/net/data/cache_tests/dirty_entry2/data_3 +++ /dev/null diff --git a/net/data/cache_tests/dirty_entry2/index b/net/data/cache_tests/dirty_entry2/index Binary files differdeleted file mode 100644 index ba2b6ca..0000000 --- a/net/data/cache_tests/dirty_entry2/index +++ /dev/null diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc index d482386..14a2722 100644 --- a/net/disk_cache/backend_impl.cc +++ b/net/disk_cache/backend_impl.cc @@ -819,8 +819,8 @@ EntryImpl* BackendImpl::CreateEntryImpl(const std::string& key) { open_entries_[entry_address.value()] = cache_entry; // Save the entry. - block_files_.GetFile(entry_address)->Store(cache_entry->entry()); - block_files_.GetFile(node_address)->Store(cache_entry->rankings()); + cache_entry->entry()->Store(); + cache_entry->rankings()->Store(); IncreaseNumEntries(); entry_count_++; @@ -1643,9 +1643,6 @@ int BackendImpl::NewEntry(Addr address, EntryImpl** entry) { if (!cache_entry->LoadNodeAddress()) return ERR_READ_FAILURE; - // Prevent overwriting the dirty flag on the destructor. - cache_entry->SetDirtyFlag(GetCurrentEntryId()); - if (!rankings_.SanityCheck(cache_entry->rankings(), false)) { STRESS_NOTREACHED(); cache_entry->SetDirtyFlag(0); @@ -1665,6 +1662,9 @@ int BackendImpl::NewEntry(Addr address, EntryImpl** entry) { cache_entry->FixForDelete(); } + // Prevent overwriting the dirty flag on the destructor. + cache_entry->SetDirtyFlag(GetCurrentEntryId()); + if (cache_entry->dirty()) { Trace("Dirty entry 0x%p 0x%x", reinterpret_cast<void*>(cache_entry.get()), address.value()); @@ -2219,8 +2219,7 @@ bool BackendImpl::CheckEntry(EntryImpl* cache_entry) { } } - RankingsNode* rankings = cache_entry->rankings()->Data(); - return ok && !rankings->dummy; + return ok && cache_entry->rankings()->VerifyHash(); } int BackendImpl::MaxBuffersSize() { diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc index b102193..c19ca68 100644 --- a/net/disk_cache/backend_unittest.cc +++ b/net/disk_cache/backend_unittest.cc @@ -58,7 +58,6 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache { void BackendInvalidEntry10(bool eviction); void BackendInvalidEntry11(bool eviction); void BackendTrimInvalidEntry12(); - void BackendNotMarkedButDirty(const std::string& name); void BackendDoomAll(); void BackendDoomAll2(); void BackendInvalidRankings(); @@ -699,7 +698,7 @@ void DiskCacheBackendTest::BackendInvalidEntryWithLoad() { for (int i = 0; i < kNumEntries / 2; i++) { disk_cache::Entry* entry; - EXPECT_EQ(net::OK, OpenEntry(keys[i], &entry)); + ASSERT_EQ(net::OK, OpenEntry(keys[i], &entry)); entry->Close(); } @@ -1843,36 +1842,6 @@ TEST_F(DiskCacheBackendTest, NewEvictionTrimInvalidEntry12) { BackendTrimInvalidEntry12(); } -// We want to be able to deal with abnormal dirty entries. -void DiskCacheBackendTest::BackendNotMarkedButDirty(const std::string& name) { - ASSERT_TRUE(CopyTestCache(name)); - DisableFirstCleanup(); - InitCache(); - - disk_cache::Entry *entry1, *entry2; - ASSERT_EQ(net::OK, OpenEntry("the first key", &entry1)); - EXPECT_NE(net::OK, OpenEntry("some other key", &entry2)); - entry1->Close(); -} - -TEST_F(DiskCacheBackendTest, NotMarkedButDirty) { - BackendNotMarkedButDirty("dirty_entry"); -} - -TEST_F(DiskCacheBackendTest, NewEvictionNotMarkedButDirty) { - SetNewEviction(); - BackendNotMarkedButDirty("dirty_entry"); -} - -TEST_F(DiskCacheBackendTest, NotMarkedButDirty2) { - BackendNotMarkedButDirty("dirty_entry2"); -} - -TEST_F(DiskCacheBackendTest, NewEvictionNotMarkedButDirty2) { - SetNewEviction(); - BackendNotMarkedButDirty("dirty_entry2"); -} - // We want to be able to deal with messed up entries on disk. void DiskCacheBackendTest::BackendInvalidRankings2() { ASSERT_TRUE(CopyTestCache("bad_rankings")); diff --git a/net/disk_cache/disk_format.h b/net/disk_cache/disk_format.h index fd36d34..35b7030 100644 --- a/net/disk_cache/disk_format.h +++ b/net/disk_cache/disk_format.h @@ -123,7 +123,8 @@ struct EntryStore { int32 data_size[4]; // We can store up to 4 data streams for each CacheAddr data_addr[4]; // entry. uint32 flags; // Any combination of EntryFlags. - int32 pad[5]; + int32 pad[4]; + uint32 self_hash; // The hash of EntryStore up to this point. char key[256 - 24 * 4]; // null terminated }; @@ -153,7 +154,7 @@ struct RankingsNode { CacheAddr prev; // LRU list. CacheAddr contents; // Address of the EntryStore. int32 dirty; // The entry is being modifyied. - int32 dummy; // Old files may have a pointer here. + uint32 self_hash; // RankingsNode's hash. }; #pragma pack(pop) diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc index 0a6a2d2..7e13029 100644 --- a/net/disk_cache/entry_impl.cc +++ b/net/disk_cache/entry_impl.cc @@ -544,11 +544,6 @@ bool EntryImpl::Update() { void EntryImpl::SetDirtyFlag(int32 current_id) { DCHECK(node_.HasData()); - // We are checking if the entry is valid or not. If there is a pointer here, - // we should not be checking the entry. - if (node_.Data()->dummy) - dirty_ = true; - if (node_.Data()->dirty && current_id != node_.Data()->dirty) dirty_ = true; @@ -558,7 +553,6 @@ void EntryImpl::SetDirtyFlag(int32 current_id) { void EntryImpl::SetPointerForInvalidEntry(int32 new_id) { node_.Data()->dirty = new_id; - node_.Data()->dummy = 0; node_.Store(); } @@ -571,6 +565,9 @@ bool EntryImpl::LeaveRankingsBehind() { // Basically, even if there is something wrong with this entry, we want to see // if it is possible to load the rankings node and delete them together. bool EntryImpl::SanityCheck() { + if (!entry_.VerifyHash()) + return false; + EntryStore* stored = entry_.Data(); if (!stored->rankings_node || stored->key_len <= 0) return false; diff --git a/net/disk_cache/rankings.cc b/net/disk_cache/rankings.cc index d478d92..4d936f3 100644 --- a/net/disk_cache/rankings.cc +++ b/net/disk_cache/rankings.cc @@ -494,6 +494,9 @@ int Rankings::SelfCheck() { } bool Rankings::SanityCheck(CacheRankingsBlock* node, bool from_list) const { + if (!node->VerifyHash()) + return false; + const RankingsNode* data = node->Data(); if ((!data->next && data->prev) || (data->next && !data->prev)) @@ -572,17 +575,14 @@ bool Rankings::GetRanking(CacheRankingsBlock* rankings) { backend_->OnEvent(Stats::OPEN_RANKINGS); - // "dummy" is the old "pointer" value, so it has to be 0. - if (!rankings->Data()->dirty && !rankings->Data()->dummy) + if (!rankings->Data()->dirty) return true; EntryImpl* entry = backend_->GetOpenEntry(rankings); if (!entry) { // We cannot trust this entry, but we cannot initiate a cleanup from this - // point (we may be in the middle of a cleanup already). Just get rid of - // the invalid pointer and continue; the entry will be deleted when detected - // from a regular open/create path. - rankings->Data()->dummy = 0; + // point (we may be in the middle of a cleanup already). The entry will be + // deleted when detected from a regular open/create path. rankings->Data()->dirty = backend_->GetCurrentEntryId() - 1; if (!rankings->Data()->dirty) rankings->Data()->dirty--; @@ -623,7 +623,6 @@ void Rankings::CompleteTransaction() { if (!node.Load()) return; - node.Data()->dummy = 0; node.Store(); Addr& my_head = heads_[control_data_->operation_list]; @@ -720,7 +719,7 @@ void Rankings::RevertRemove(CacheRankingsBlock* node) { } bool Rankings::CheckEntry(CacheRankingsBlock* rankings) { - if (!rankings->Data()->dummy) + if (rankings->VerifyHash()) return true; // If this entry is not dirty, it is a serious problem. diff --git a/net/disk_cache/storage_block-inl.h b/net/disk_cache/storage_block-inl.h index e2f4b3c..614b143 100644 --- a/net/disk_cache/storage_block-inl.h +++ b/net/disk_cache/storage_block-inl.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -9,6 +9,7 @@ #include "net/disk_cache/storage_block.h" #include "base/logging.h" +#include "net/disk_cache/hash.h" #include "net/disk_cache/trace.h" namespace disk_cache { @@ -98,6 +99,11 @@ template<typename T> bool StorageBlock<T>::HasData() const { return (NULL != data_); } +template<typename T> bool StorageBlock<T>::VerifyHash() const { + uint32 hash = CalculateHash(); + return (!data_->self_hash || data_->self_hash == hash); +} + template<typename T> bool StorageBlock<T>::own_data() const { return own_data_; } @@ -123,6 +129,7 @@ template<typename T> bool StorageBlock<T>::Load() { template<typename T> bool StorageBlock<T>::Store() { if (file_ && data_) { + data_->self_hash = CalculateHash(); if (file_->Store(this)) { modified_ = false; return true; @@ -156,6 +163,10 @@ template<typename T> void StorageBlock<T>::DeleteData() { } } +template<typename T> uint32 StorageBlock<T>::CalculateHash() const { + return Hash(reinterpret_cast<char*>(data_), offsetof(T, self_hash)); +} + } // namespace disk_cache #endif // NET_DISK_CACHE_STORAGE_BLOCK_INL_H_ diff --git a/net/disk_cache/storage_block.h b/net/disk_cache/storage_block.h index 49694c6..2cf3c3a 100644 --- a/net/disk_cache/storage_block.h +++ b/net/disk_cache/storage_block.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -61,6 +61,9 @@ class StorageBlock : public FileBlock { // Returns true if there is data associated with this object. bool HasData() const; + // Returns true if the internal hash is correct. + bool VerifyHash() const; + // Returns true if this object owns the data buffer, false if it is shared. bool own_data() const; @@ -73,6 +76,7 @@ class StorageBlock : public FileBlock { private: void AllocateData(); void DeleteData(); + uint32 CalculateHash() const; T* data_; MappedFile* file_; diff --git a/net/tools/dump_cache/dump_files.cc b/net/tools/dump_cache/dump_files.cc index f8913d8..5d2975a 100644 --- a/net/tools/dump_cache/dump_files.cc +++ b/net/tools/dump_cache/dump_files.cc @@ -257,7 +257,7 @@ void DumpRankings(const disk_cache::RankingsNode& rankings) { printf("prev: 0x%x\n", rankings.prev); printf("entry: 0x%x\n", rankings.contents); printf("dirty: %d\n", rankings.dirty); - printf("pointer: 0x%x\n", rankings.dummy); + printf("hash: 0x%x\n", rankings.self_hash); printf("----------\n\n"); } |