From dc8b936deddb16343b09df857bbdd75f11f9408d Mon Sep 17 00:00:00 2001 From: "rvargas@google.com" Date: Thu, 28 May 2009 01:32:25 +0000 Subject: Disk cache: Reset mask_ when the disk cache is being restarted. It is possible that the size for the index table is reduced when the cache is restarted (the available disk space could be lower than what it was when the cache was originally created). If that is the case, not resetting mask_ to 0 will prevent the mask to be set correctly for the new table size, resulting in a crash. BUG=12378 TEST=unittests Review URL: http://codereview.chromium.org/113930 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17052 0039d316-1c4b-4281-b951-d872f2087c98 --- net/data/cache_tests/bad_rankings2/contents.txt | 66 ++++++++++++++++++++++++ net/data/cache_tests/bad_rankings2/data_0 | Bin 0 -> 45056 bytes net/data/cache_tests/bad_rankings2/data_1 | Bin 0 -> 270336 bytes net/data/cache_tests/bad_rankings2/data_2 | Bin 0 -> 8192 bytes net/data/cache_tests/bad_rankings2/data_3 | Bin 0 -> 8192 bytes net/data/cache_tests/bad_rankings2/index | Bin 0 -> 524656 bytes net/disk_cache/backend_impl.cc | 8 +++ net/disk_cache/backend_unittest.cc | 63 ++++++++++++++++++++++ 8 files changed, 137 insertions(+) create mode 100644 net/data/cache_tests/bad_rankings2/contents.txt create mode 100644 net/data/cache_tests/bad_rankings2/data_0 create mode 100644 net/data/cache_tests/bad_rankings2/data_1 create mode 100644 net/data/cache_tests/bad_rankings2/data_2 create mode 100644 net/data/cache_tests/bad_rankings2/data_3 create mode 100644 net/data/cache_tests/bad_rankings2/index diff --git a/net/data/cache_tests/bad_rankings2/contents.txt b/net/data/cache_tests/bad_rankings2/contents.txt new file mode 100644 index 0000000..a949728 --- /dev/null +++ b/net/data/cache_tests/bad_rankings2/contents.txt @@ -0,0 +1,66 @@ +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: 0 <------ wrong +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: 0 + +================================ + +Generated with: + +disk_cache::Entry *entry; +ASSERT_TRUE(cache_->CreateEntry("the first key", &entry)); +entry->Close(); + +ASSERT_TRUE(cache_->CreateEntry("some other key", &entry)); <---- Edit value* +entry->Close(); + +* Edit the value with the debugger before it is saved to disk. \ No newline at end of file diff --git a/net/data/cache_tests/bad_rankings2/data_0 b/net/data/cache_tests/bad_rankings2/data_0 new file mode 100644 index 0000000..5b32215 Binary files /dev/null and b/net/data/cache_tests/bad_rankings2/data_0 differ diff --git a/net/data/cache_tests/bad_rankings2/data_1 b/net/data/cache_tests/bad_rankings2/data_1 new file mode 100644 index 0000000..7241f13 Binary files /dev/null and b/net/data/cache_tests/bad_rankings2/data_1 differ diff --git a/net/data/cache_tests/bad_rankings2/data_2 b/net/data/cache_tests/bad_rankings2/data_2 new file mode 100644 index 0000000..c7e2eb9 Binary files /dev/null and b/net/data/cache_tests/bad_rankings2/data_2 differ diff --git a/net/data/cache_tests/bad_rankings2/data_3 b/net/data/cache_tests/bad_rankings2/data_3 new file mode 100644 index 0000000..5eec973 Binary files /dev/null and b/net/data/cache_tests/bad_rankings2/data_3 differ diff --git a/net/data/cache_tests/bad_rankings2/index b/net/data/cache_tests/bad_rankings2/index new file mode 100644 index 0000000..602dfba Binary files /dev/null and b/net/data/cache_tests/bad_rankings2/index differ diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc index e997b90..5c64d01 100644 --- a/net/disk_cache/backend_impl.cc +++ b/net/disk_cache/backend_impl.cc @@ -788,6 +788,10 @@ void BackendImpl::CriticalError(int error) { LogStats(); ReportError(error); + // Reset the mask_ if it was not given by the user. + if (mask_ == data_->header.table_len - 1) + mask_ = 0; + // Setting the index table length to an invalid value will force re-creation // of the cache files. data_->header.table_len = 1; @@ -983,6 +987,10 @@ void BackendImpl::RestartCache() { } void BackendImpl::PrepareForRestart() { + // Reset the mask_ if it was not given by the user. + if (mask_ == data_->header.table_len - 1) + mask_ = 0; + data_->header.crash = 0; index_ = NULL; data_ = NULL; diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc index dd33d28..2fbcfa9 100644 --- a/net/disk_cache/backend_unittest.cc +++ b/net/disk_cache/backend_unittest.cc @@ -59,10 +59,12 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache { void BackendInvalidEntry2(); void BackendNotMarkedButDirty(); void BackendDoomAll(); + void BackendDoomAll2(); void BackendInvalidRankings(); void BackendInvalidRankings2(); void BackendDisable(); void BackendDisable2(); + void BackendDisable3(); }; void DiskCacheBackendTest::BackendBasics() { @@ -1169,6 +1171,38 @@ TEST_F(DiskCacheBackendTest, NewEvictionDisableFailure2) { BackendDisable2(); } +// If the index size changes when we disable the cache, we should not crash. +void DiskCacheBackendTest::BackendDisable3() { + disk_cache::Entry *entry1, *entry2; + void* iter = NULL; + EXPECT_EQ(2, cache_->GetEntryCount()); + ASSERT_TRUE(cache_->OpenNextEntry(&iter, &entry1)); + entry1->Close(); + + EXPECT_FALSE(cache_->OpenNextEntry(&iter, &entry2)); + ASSERT_TRUE(cache_->CreateEntry("Something new", &entry2)); + entry2->Close(); + + EXPECT_EQ(1, cache_->GetEntryCount()); +} + +TEST_F(DiskCacheBackendTest, DisableSuccess3) { + ASSERT_TRUE(CopyTestCache(L"bad_rankings2")); + DisableFirstCleanup(); + SetMaxSize(20 * 1024 * 1024); + InitCache(); + BackendDisable3(); +} + +TEST_F(DiskCacheBackendTest, NewEvictionDisableSuccess3) { + ASSERT_TRUE(CopyTestCache(L"bad_rankings2")); + DisableFirstCleanup(); + SetMaxSize(20 * 1024 * 1024); + SetNewEviction(); + InitCache(); + BackendDisable3(); +} + TEST_F(DiskCacheTest, Backend_UsageStats) { MessageLoopHelper helper; @@ -1240,6 +1274,35 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyDoomAll) { BackendDoomAll(); } +// If the index size changes when we doom the cache, we should not crash. +void DiskCacheBackendTest::BackendDoomAll2() { + EXPECT_EQ(2, cache_->GetEntryCount()); + EXPECT_TRUE(cache_->DoomAllEntries()); + + disk_cache::Entry* entry; + ASSERT_TRUE(cache_->CreateEntry("Something new", &entry)); + entry->Close(); + + EXPECT_EQ(1, cache_->GetEntryCount()); +} + +TEST_F(DiskCacheBackendTest, DoomAll2) { + ASSERT_TRUE(CopyTestCache(L"bad_rankings2")); + DisableFirstCleanup(); + SetMaxSize(20 * 1024 * 1024); + InitCache(); + BackendDoomAll2(); +} + +TEST_F(DiskCacheBackendTest, NewEvictionDoomAll2) { + ASSERT_TRUE(CopyTestCache(L"bad_rankings2")); + DisableFirstCleanup(); + SetMaxSize(20 * 1024 * 1024); + SetNewEviction(); + InitCache(); + BackendDoomAll2(); +} + // We should be able to create the same entry on multiple simultaneous instances // of the cache. TEST_F(DiskCacheTest, MultipleInstances) { -- cgit v1.1