Disk cache: Fix handling of invalid entries that are detected

when doing evictions or enumerations.

This cl fixes an issue with dirty entries being deleted
twice from disk, and improves the whole thing so that
different flavors or corrupt entries are removed
gracefully. We started deleting things twice when we
created the map of currently-open entries, because
we don't want to have dirty entries on that map, so it is
possible to have multiple EntryImpl objects that refer
to the same entry (for corrupt entries).

BUG=17474
TEST=Unittests, stress_cache.exe

Review URL: http://codereview.chromium.org/155951

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21360 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 200 insertions, 80 deletions

diff --git a/net/data/purify/net_unittests.exe.gtest.txt b/net/data/purify/net_unittests.exe.gtest.txt
index 4e2a700..890bc5e 100644
--- a/net/data/purify/net_unittests.exe.gtest.txt
+++ b/net/data/purify/net_unittests.exe.gtest.txt
@@ -7,9 +7,11 @@ DiskCacheBackendTest.InvalidEntry
 DiskCacheBackendTest.InvalidEntryRead
 DiskCacheBackendTest.InvalidEntryWithLoad
 DiskCacheBackendTest.TrimInvalidEntry
+DiskCacheBackendTest.TrimInvalidEntry2
 DiskCacheBackendTest.InvalidEntryEnumeration
 DiskCacheBackendTest.NewEvictionInvalidEntry
 DiskCacheBackendTest.NewEvictionInvalidEntryRead
 DiskCacheBackendTest.NewEvictionInvalidEntryWithLoad
 DiskCacheBackendTest.NewEvictionTrimInvalidEntry
+DiskCacheBackendTest.NewEvictionTrimInvalidEntry2
 DiskCacheBackendTest.NewEvictionInvalidEntryEnumeration
diff --git a/net/data/valgrind/net_unittests.gtest.txt b/net/data/valgrind/net_unittests.gtest.txt
index 45117d8..458ddef 100644
--- a/net/data/valgrind/net_unittests.gtest.txt
+++ b/net/data/valgrind/net_unittests.gtest.txt
@@ -6,9 +6,11 @@ DiskCacheBackendTest.InvalidEntry
 DiskCacheBackendTest.InvalidEntryRead
 DiskCacheBackendTest.InvalidEntryWithLoad
 DiskCacheBackendTest.TrimInvalidEntry
+DiskCacheBackendTest.TrimInvalidEntry2
 DiskCacheBackendTest.InvalidEntryEnumeration
 DiskCacheBackendTest.NewEvictionInvalidEntry
 DiskCacheBackendTest.NewEvictionInvalidEntryRead
 DiskCacheBackendTest.NewEvictionInvalidEntryWithLoad
 DiskCacheBackendTest.NewEvictionTrimInvalidEntry
+DiskCacheBackendTest.NewEvictionTrimInvalidEntry2
 DiskCacheBackendTest.NewEvictionInvalidEntryEnumeration
diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index ecbbcc2..5806822 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -147,6 +147,12 @@ bool InitExperiment(int* current_group) {
   }
 
   if (*current_group <= 5) {
+#if defined(UNIT_TEST)
+    // The unit test controls directly what to test.
+    *current_group = 0;
+    return true;
+#endif
+
     // Re-load the two groups.
     int option = base::RandInt(0, 9);
 
@@ -1177,7 +1183,7 @@ EntryImpl* BackendImpl::MatchEntry(const std::string& key, uint32 hash,
       if (!error) {
         // It is important to call DestroyInvalidEntry after removing this
         // entry from the table.
-        DestroyInvalidEntry(address, cache_entry);
+        DestroyInvalidEntry(cache_entry);
         cache_entry = NULL;
       } else {
         Trace("NewEntry failed on MatchEntry 0x%x", address.value());
@@ -1252,7 +1258,7 @@ bool BackendImpl::OpenFollowingEntry(bool forward, void** iter,
           OpenFollowingEntryFromList(forward, iterator->list,
                                      &iterator->nodes[i], &temp);
       } else {
-        temp = GetEnumeratedEntry(iterator->nodes[i]);
+        temp = GetEnumeratedEntry(iterator->nodes[i], false);
       }
 
       entries[i].swap(&temp);  // The entry was already addref'd.
@@ -1308,7 +1314,7 @@ bool BackendImpl::OpenFollowingEntryFromList(bool forward, Rankings::List list,
   Rankings::ScopedRankingsBlock next(&rankings_, next_block);
   *from_entry = NULL;
 
-  *next_entry = GetEnumeratedEntry(next.get());
+  *next_entry = GetEnumeratedEntry(next.get(), false);
   if (!*next_entry)
     return false;
 
@@ -1316,41 +1322,29 @@ bool BackendImpl::OpenFollowingEntryFromList(bool forward, Rankings::List list,
   return true;
 }
 
-EntryImpl* BackendImpl::GetEnumeratedEntry(CacheRankingsBlock* next) {
+EntryImpl* BackendImpl::GetEnumeratedEntry(CacheRankingsBlock* next,
+                                           bool to_evict) {
   if (!next)
     return NULL;
 
-  scoped_refptr<EntryImpl> entry;
-  EntryImpl* temp = NULL;
-
-  if (next->Data()->pointer) {
-    entry = reinterpret_cast<EntryImpl*>(next->Data()->pointer);
-    entry.swap(&temp);
-    return temp;
-  }
-
+  EntryImpl* entry;
   bool dirty;
-  if (NewEntry(Addr(next->Data()->contents), &temp, &dirty))
+  if (NewEntry(Addr(next->Data()->contents), &entry, &dirty))
     return NULL;
-  entry.swap(&temp);
 
   if (dirty) {
-    // We cannot trust this entry. Call MatchEntry to go through the regular
-    // path and take the appropriate action.
-    std::string key = entry->GetKey();
-    uint32 hash = entry->GetHash();
-    entry = NULL;  // Release the entry.
-    temp = MatchEntry(key, hash, false);
-    if (temp)
-      temp->Release();
-
+    // We cannot trust this entry. This code also releases the reference.
+    DestroyInvalidEntryFromEnumeration(entry);
     return NULL;
   }
-  if (!entry->Update())
+
+  // There is no need to store the entry to disk if we want to delete it.
+  if (!to_evict && !entry->Update()) {
+    entry->Release();
     return NULL;
+  }
 
-  entry.swap(&temp);
-  return temp;
+  return entry;
 }
 
 bool BackendImpl::ResurrectEntry(EntryImpl* deleted_entry, Entry** entry) {
@@ -1372,7 +1366,7 @@ bool BackendImpl::ResurrectEntry(EntryImpl* deleted_entry, Entry** entry) {
   return true;
 }
 
-void BackendImpl::DestroyInvalidEntry(Addr address, EntryImpl* entry) {
+void BackendImpl::DestroyInvalidEntry(EntryImpl* entry) {
   LOG(WARNING) << "Destroying invalid entry.";
   Trace("Destroying invalid entry 0x%p", entry);
 
@@ -1386,6 +1380,42 @@ void BackendImpl::DestroyInvalidEntry(Addr address, EntryImpl* entry) {
   stats_.OnEvent(Stats::INVALID_ENTRY);
 }
 
+// This is kind of ugly. The entry may or may not be part of the cache index
+// table, and it may even have corrupt fields. If we just doom it, we may end up
+// deleting it twice (if all fields are right, and when looking up the parent of
+// chained entries wee see this one... and we delete it because it is dirty). If
+// we ignore it, we may leave it here forever. So we're going to attempt to
+// delete it through the provided object, without touching the index table
+// (because we cannot jus call MatchEntry()), and also attempt to delete it from
+// the table through the key: this may find a new entry (too bad), or an entry
+// that was just deleted and consider it a very corrupt entry.
+void BackendImpl::DestroyInvalidEntryFromEnumeration(EntryImpl* entry) {
+  std::string key = entry->GetKey();
+  entry->SetPointerForInvalidEntry(GetCurrentEntryId());
+  CacheAddr next_entry = entry->entry()->Data()->next;
+  if (!next_entry) {
+    DestroyInvalidEntry(entry);
+    entry->Release();
+  }
+  DoomEntry(key);
+
+  if (!next_entry)
+    return;
+
+  // We have a chained entry so instead of destroying first this entry and then
+  // anything with this key, we just called DoomEntry() first. If that call
+  // deleted everything, |entry| has invalid data. Let's see if there is
+  // something else to do. We started with just a rankings node (we come from
+  // an enumeration), so that one may still be there.
+  CacheRankingsBlock* rankings = entry->rankings();
+  rankings->Load();
+  if (rankings->Data()->contents) {
+    // We still have something. Clean this up.
+    DestroyInvalidEntry(entry);
+  }
+  entry->Release();
+}
+
 void BackendImpl::AddStorageSize(int32 bytes) {
   data_->header.num_bytes += bytes;
   DCHECK(data_->header.num_bytes >= 0);
diff --git a/net/disk_cache/backend_impl.h b/net/disk_cache/backend_impl.h
index e53150e..ef282779 100644
--- a/net/disk_cache/backend_impl.h
+++ b/net/disk_cache/backend_impl.h
@@ -208,13 +208,15 @@ class BackendImpl : public Backend {
                                   CacheRankingsBlock** from_entry,
                                   EntryImpl** next_entry);
 
-  // Returns the entry that is pointed by |next|.
-  EntryImpl* GetEnumeratedEntry(CacheRankingsBlock* next);
+  // Returns the entry that is pointed by |next|. If we are trimming the cache,
+  // |to_evict| should be true so that we don't perform extra disk writes.
+  EntryImpl* GetEnumeratedEntry(CacheRankingsBlock* next, bool to_evict);
 
   // Re-opens an entry that was previously deleted.
   bool ResurrectEntry(EntryImpl* deleted_entry, Entry** entry);
 
-  void DestroyInvalidEntry(Addr address, EntryImpl* entry);
+  void DestroyInvalidEntry(EntryImpl* entry);
+  void DestroyInvalidEntryFromEnumeration(EntryImpl* entry);
 
   // Handles the used storage count.
   void AddStorageSize(int32 bytes);
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 8364f5e..a1127a6 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -48,6 +48,7 @@ class DiskCacheBackendTest : public DiskCacheTestWithCache {
   void BackendInvalidEntryRead();
   void BackendInvalidEntryWithLoad();
   void BackendTrimInvalidEntry();
+  void BackendTrimInvalidEntry2();
   void BackendEnumerations();
   void BackendInvalidEntryEnumeration();
   void BackendFixEnumerators();
@@ -493,8 +494,8 @@ void DiskCacheBackendTest::BackendTrimInvalidEntry() {
   // Use the implementation directly... we need to simulate a crash.
   SetDirectMode();
 
-  const int cache_size = 0x4000;  // 16 kB
-  SetMaxSize(cache_size * 10);
+  const int kSize = 0x3000;  // 12 kB
+  SetMaxSize(kSize * 10);
   InitCache();
 
   std::string first("some key");
@@ -502,21 +503,26 @@ void DiskCacheBackendTest::BackendTrimInvalidEntry() {
   disk_cache::Entry* entry;
   ASSERT_TRUE(cache_->CreateEntry(first, &entry));
 
-  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(cache_size);
-  memset(buffer->data(), 0, cache_size);
-  EXPECT_EQ(cache_size * 19 / 20, entry->WriteData(0, 0, buffer,
-                cache_size * 19 / 20, NULL, false));
+  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(kSize);
+  memset(buffer->data(), 0, kSize);
+  EXPECT_EQ(kSize, entry->WriteData(0, 0, buffer, kSize, NULL, false));
 
   // Simulate a crash.
   SimulateCrash();
 
   ASSERT_TRUE(cache_->CreateEntry(second, &entry));
-  EXPECT_EQ(cache_size / 10, entry->WriteData(0, 0, buffer, cache_size / 10,
-                                              NULL, false)) << "trim the cache";
-  entry->Close();
+  EXPECT_EQ(kSize, entry->WriteData(0, 0, buffer, kSize, NULL, false));
 
+  EXPECT_EQ(2, cache_->GetEntryCount());
+  SetMaxSize(kSize);
+  entry->Close();  // Trim the cache.
+
+  // If we evicted the entry in less than 20mS, we have one entry in the cache;
+  // if it took more than that, we posted a task and we'll delete the second
+  // entry too.
+  MessageLoop::current()->RunAllPending();
+  EXPECT_GE(1, cache_->GetEntryCount());
   EXPECT_FALSE(cache_->OpenEntry(first, &entry));
-  EXPECT_EQ(1, cache_->GetEntryCount());
 }
 
 // We'll be leaking memory from this test.
@@ -530,6 +536,62 @@ TEST_F(DiskCacheBackendTest, NewEvictionTrimInvalidEntry) {
   BackendTrimInvalidEntry();
 }
 
+// We'll be leaking memory from this test.
+void DiskCacheBackendTest::BackendTrimInvalidEntry2() {
+  // Use the implementation directly... we need to simulate a crash.
+  SetDirectMode();
+  SetMask(0xf);  // 16-entry table.
+
+  const int kSize = 0x3000;  // 12 kB
+  SetMaxSize(kSize * 40);
+  InitCache();
+
+  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(kSize);
+  memset(buffer->data(), 0, kSize);
+  disk_cache::Entry* entry;
+
+  // Writing 32 entries to this cache chains most of them.
+  for (int i = 0; i < 32; i++) {
+    std::string key(StringPrintf("some key %d", i));
+    ASSERT_TRUE(cache_->CreateEntry(key, &entry));
+    EXPECT_EQ(kSize, entry->WriteData(0, 0, buffer, kSize, NULL, false));
+    entry->Close();
+    ASSERT_TRUE(cache_->OpenEntry(key, &entry));
+    // Note that we are not closing the entries.
+  }
+
+  // Simulate a crash.
+  SimulateCrash();
+
+  ASSERT_TRUE(cache_->CreateEntry("Something else", &entry));
+  EXPECT_EQ(kSize, entry->WriteData(0, 0, buffer, kSize, NULL, false));
+
+  EXPECT_EQ(33, cache_->GetEntryCount());
+  SetMaxSize(kSize);
+
+  // For the new eviction code, all corrupt entries are on the second list so
+  // they are not going away that easy.
+  if (new_eviction_)
+    cache_->DoomAllEntries();
+
+  entry->Close();  // Trim the cache.
+
+  // We may abort the eviction before cleaning up everything.
+  MessageLoop::current()->RunAllPending();
+  EXPECT_GE(30, cache_->GetEntryCount());
+}
+
+// We'll be leaking memory from this test.
+TEST_F(DiskCacheBackendTest, TrimInvalidEntry2) {
+  BackendTrimInvalidEntry2();
+}
+
+// We'll be leaking memory from this test.
+TEST_F(DiskCacheBackendTest, NewEvictionTrimInvalidEntry2) {
+  SetNewEviction();
+  BackendTrimInvalidEntry2();
+}
+
 void DiskCacheBackendTest::BackendEnumerations() {
   InitCache();
   Time initial = Time::Now();
diff --git a/net/disk_cache/eviction.cc b/net/disk_cache/eviction.cc
index 8fb1e1c..b6d4be4 100644
--- a/net/disk_cache/eviction.cc
+++ b/net/disk_cache/eviction.cc
@@ -73,25 +73,29 @@ void Eviction::TrimCache(bool empty) {
   if (new_eviction_)
     return TrimCacheV2(empty);
 
-  Trace("*** Trim Cache ***");
   if (backend_->disabled_ || trimming_)
     return;
 
   if (!empty && backend_->IsLoaded())
     return PostDelayedTrim();
 
+  Trace("*** Trim Cache ***");
   trimming_ = true;
   Time start = Time::Now();
   Rankings::ScopedRankingsBlock node(rankings_);
   Rankings::ScopedRankingsBlock next(rankings_,
       rankings_->GetPrev(node.get(), Rankings::NO_USE));
-  DCHECK(next.get());
   int target_size = empty ? 0 : max_size_;
   while (header_->num_bytes > target_size && next.get()) {
+    // The iterator could be invalidated within EvictEntry().
+    if (!next->HasData())
+      break;
     node.reset(next.release());
     next.reset(rankings_->GetPrev(node.get(), Rankings::NO_USE));
     if (!node->Data()->pointer || empty) {
       // This entry is not being used by anybody.
+      // Do NOT use node as an iterator after this point.
+      rankings_->TrackRankingsBlock(node.get(), false);
       if (!EvictEntry(node.get(), empty))
         continue;
 
@@ -191,17 +195,12 @@ Rankings::List Eviction::GetListForEntry(EntryImpl* entry) {
 }
 
 bool Eviction::EvictEntry(CacheRankingsBlock* node, bool empty) {
-  EntryImpl* entry;
-  bool dirty;
-  if (backend_->NewEntry(Addr(node->Data()->contents), &entry, &dirty)) {
+  EntryImpl* entry = backend_->GetEnumeratedEntry(node, true);
+  if (!entry) {
     Trace("NewEntry failed on Trim 0x%x", node->address().value());
     return false;
   }
 
-  if (node->Data()->pointer) {
-    // We ignore the failure; we're removing the entry anyway.
-    entry->Update();
-  }
   ReportTrimTimes(entry);
   if (empty || !new_eviction_) {
     entry->Doom();
@@ -224,13 +223,13 @@ bool Eviction::EvictEntry(CacheRankingsBlock* node, bool empty) {
 // -----------------------------------------------------------------------
 
 void Eviction::TrimCacheV2(bool empty) {
-  Trace("*** Trim Cache ***");
   if (backend_->disabled_ || trimming_)
     return;
 
   if (!empty && backend_->IsLoaded())
     return PostDelayedTrim();
 
+  Trace("*** Trim Cache ***");
   trimming_ = true;
   Time start = Time::Now();
 
@@ -270,11 +269,16 @@ void Eviction::TrimCacheV2(bool empty) {
   int target_size = empty ? 0 : max_size_;
   for (; list < kListsToSearch; list++) {
     while (header_->num_bytes > target_size && next[list].get()) {
+      // The iterator could be invalidated within EvictEntry().
+      if (!next[list]->HasData())
+        break;
       node.reset(next[list].release());
       next[list].reset(rankings_->GetPrev(node.get(),
                                           static_cast<Rankings::List>(list)));
       if (!node->Data()->pointer || empty) {
         // This entry is not being used by anybody.
+        // Do NOT use node as an iterator after this point.
+        rankings_->TrackRankingsBlock(node.get(), false);
         if (!EvictEntry(node.get(), empty))
           continue;
 
@@ -419,6 +423,8 @@ bool Eviction::RemoveDeletedNode(CacheRankingsBlock* node) {
     return false;
   }
 
+  // TODO(rvargas): figure out how to deal with corruption at this point (dirty
+  // entries that live in this list).
   if (node->Data()->pointer) {
     // We ignore the failure; we're removing the entry anyway.
     entry->Update();
diff --git a/net/disk_cache/rankings.cc b/net/disk_cache/rankings.cc
index a88792f..b36ee2e 100644
--- a/net/disk_cache/rankings.cc
+++ b/net/disk_cache/rankings.cc
@@ -320,6 +320,7 @@ void Rankings::Remove(CacheRankingsBlock* node, List list) {
   Trace("Remove 0x%x (0x%x 0x%x)", node->address().value(), node->Data()->next,
         node->Data()->prev);
   DCHECK(node->HasData());
+  InvalidateIterators(node);
   Addr next_addr(node->Data()->next);
   Addr prev_addr(node->Data()->prev);
   if (!next_addr.is_initialized() || next_addr.is_separate_file() ||
@@ -394,7 +395,6 @@ void Rankings::Remove(CacheRankingsBlock* node, List list) {
 // entry. Otherwise we'll need reentrant transactions, which is an overkill.
 void Rankings::UpdateRank(CacheRankingsBlock* node, bool modified, List list) {
   Time start = Time::Now();
-  NotAnIterator(node);
   Remove(node, list);
   Insert(node, modified, list);
   CACHE_UMA(AGE_MS, "UpdateRank", 0, start);
@@ -574,6 +574,19 @@ void Rankings::FreeRankingsBlock(CacheRankingsBlock* node) {
   TrackRankingsBlock(node, false);
 }
 
+void Rankings::TrackRankingsBlock(CacheRankingsBlock* node,
+                                  bool start_tracking) {
+  if (!node)
+    return;
+
+  IteratorPair current(node->address().value(), node);
+
+  if (start_tracking)
+    iterators_.push_back(current);
+  else
+    iterators_.remove(current);
+}
+
 int Rankings::SelfCheck() {
   int total = 0;
   for (int i = 0; i < LAST_ELEMENT; i++) {
@@ -727,19 +740,6 @@ bool Rankings::IsTail(CacheAddr addr) {
   return false;
 }
 
-void Rankings::TrackRankingsBlock(CacheRankingsBlock* node,
-                                  bool start_tracking) {
-  if (!node)
-    return;
-
-  IteratorPair current(node->address().value(), node);
-
-  if (start_tracking)
-    iterators_.push_back(current);
-  else
-    iterators_.remove(current);
-}
-
 // We expect to have just a few iterators at any given time, maybe two or three,
 // But we could have more than one pointing at the same mode. We walk the list
 // of cache iterators and update all that are pointing to the given node.
@@ -747,7 +747,7 @@ void Rankings::UpdateIterators(CacheRankingsBlock* node) {
   CacheAddr address = node->address().value();
   for (IteratorList::iterator it = iterators_.begin(); it != iterators_.end();
        ++it) {
-    if (it->first == address) {
+    if (it->first == address && it->second->HasData()) {
       CacheRankingsBlock* other = it->second;
       other->Data()->next = node->Data()->next;
       other->Data()->prev = node->Data()->prev;
@@ -755,16 +755,14 @@ void Rankings::UpdateIterators(CacheRankingsBlock* node) {
   }
 }
 
-void Rankings::NotAnIterator(CacheRankingsBlock* node) {
-#ifdef NDEBUG
-  // This method is only enabled for debug builds.
-  return;
-#endif
+void Rankings::InvalidateIterators(CacheRankingsBlock* node) {
   CacheAddr address = node->address().value();
   for (IteratorList::iterator it = iterators_.begin(); it != iterators_.end();
        ++it) {
-    if (it->first == address)
-      NOTREACHED();
+    if (it->first == address) {
+      LOG(WARNING) << "Invalidating iterator at 0x" << std::hex << address;
+      it->second->Discard();
+    }
   }
 }
 
diff --git a/net/disk_cache/rankings.h b/net/disk_cache/rankings.h
index 0a8b113..4e94237 100644
--- a/net/disk_cache/rankings.h
+++ b/net/disk_cache/rankings.h
@@ -127,6 +127,9 @@ class Rankings {
   CacheRankingsBlock* GetPrev(CacheRankingsBlock* node, List list);
   void FreeRankingsBlock(CacheRankingsBlock* node);
 
+  // Controls tracking of nodes used for enumerations.
+  void TrackRankingsBlock(CacheRankingsBlock* node, bool start_tracking);
+
   // Peforms a simple self-check of the lists, and returns the number of items
   // or an error code (negative value).
   int SelfCheck();
@@ -171,14 +174,11 @@ class Rankings {
   bool IsHead(CacheAddr addr);
   bool IsTail(CacheAddr addr);
 
-  // Controls tracking of nodes used for enumerations.
-  void TrackRankingsBlock(CacheRankingsBlock* node, bool start_tracking);
-
   // Updates the iterators whenever node is being changed.
   void UpdateIterators(CacheRankingsBlock* node);
 
-  // Verifies that no iterator gets invalidated by changing a node.
-  void NotAnIterator(CacheRankingsBlock* node);
+  // Invalidates the iterators pointing to this node.
+  void InvalidateIterators(CacheRankingsBlock* node);
 
   // Keeps track of the number of entries on a list.
   void IncrementCounter(List list);
diff --git a/net/disk_cache/storage_block-inl.h b/net/disk_cache/storage_block-inl.h
index 9ec9fa0..909b717 100644
--- a/net/disk_cache/storage_block-inl.h
+++ b/net/disk_cache/storage_block-inl.h
@@ -62,6 +62,20 @@ template<typename T> void StorageBlock<T>::SetData(T* other) {
   data_ = other;
 }
 
+template<typename T> void  StorageBlock<T>::Discard() {
+  if (!data_)
+    return;
+  if (!own_data_) {
+    NOTREACHED();
+    return;
+  }
+  DeleteData();
+  data_ = NULL;
+  modified_ = false;
+  own_data_ = false;
+  extended_ = false;
+}
+
 template<typename T> void StorageBlock<T>::set_modified() {
   DCHECK(data_);
   modified_ = true;
@@ -97,7 +111,7 @@ template<typename T> bool StorageBlock<T>::Load() {
 }
 
 template<typename T> bool StorageBlock<T>::Store() {
-  if (file_) {
+  if (file_ && data_) {
     if (file_->Store(this)) {
       modified_ = false;
       return true;
diff --git a/net/disk_cache/storage_block.h b/net/disk_cache/storage_block.h
index 1c02756..78d51db 100644
--- a/net/disk_cache/storage_block.h
+++ b/net/disk_cache/storage_block.h
@@ -43,9 +43,13 @@ class StorageBlock : public FileBlock {
   // Allows the overide of dummy values passed on the constructor.
   bool LazyInit(MappedFile* file, Addr address);
 
-  // Sets the internal storage to share the momory provided by other instance.
+  // Sets the internal storage to share the memory provided by other instance.
   void SetData(T* other);
 
+  // Deletes the data, even if it was modified and not saved. This object must
+  // own the memory buffer (it cannot be shared).
+  void Discard();
+
   // Sets the object to lazily save the in-memory data on destruction.
   void set_modified();