Disk Cache: Improve handling of dirty entries.

net/disk_cache/backend_impl.cc

 // 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.
 
 #include "net/disk_cache/backend_impl.h"
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/metrics/field_trial.h"
@@ skipping 742 matching lines @@
     DCHECK(!error);
     if (parent_entry) {
       parent.swap(&parent_entry);
     } else if (data_->table[hash & mask_]) {
       // We should have corrected the problem.
       NOTREACHED();
       return NULL;
     }
   }
 
+  // The general flow is to allocate disk space and initialize the entry data,
+  // followed by saving that to disk, then linking the entry though the index
+  // and finally thorugh the lists. If there is a crash in this process, we may

[gavinp, 2011/09/29 13:25:34]

typo: thorugh -> through

+  // end up with:
+  // a. Used, unreferenced empty blocks on disk (basically just garbage).
+  // b. Used, unreferenced but meaningful data on disk (more garbage).
+  // c. A fully formed entry, reachable only through the index.
+  // d. A fully formed entry, also reachable through the lists, but still dirty.
+  //
+  // Anything after (b) can be automatically cleaned up. We may consider saving
+  // the current operation (as we do while manipulating the lists) so that we
+  // can detect and cleanup (a) and (b).
+
   int num_blocks = EntryImpl::NumBlocksForEntry(key.size());
   if (!block_files_.CreateBlock(BLOCK_256, num_blocks, &entry_address)) {
     LOG(ERROR) << "Create entry failed " << key.c_str();
     stats_.OnEvent(Stats::CREATE_ERROR);
     return NULL;
   }
 
   Addr node_address(0);
   if (!block_files_.CreateBlock(RANKINGS, 1, &node_address)) {
     block_files_.DeleteBlock(entry_address, false);
@@ skipping 12 matching lines @@
     LOG(ERROR) << "Create entry failed " << key.c_str();
     stats_.OnEvent(Stats::CREATE_ERROR);
     return NULL;
   }
 
   cache_entry->BeginLogging(net_log_, true);
 
   // We are not failing the operation; let's add this to the map.
   open_entries_[entry_address.value()] = cache_entry;
 
-  if (parent.get())
-    parent->SetNextAddress(entry_address);
-
+  // Save the entry.
   block_files_.GetFile(entry_address)->Store(cache_entry->entry());
   block_files_.GetFile(node_address)->Store(cache_entry->rankings());
+  IncreaseNumEntries();
+  entry_count_++;
 
-  IncreaseNumEntries();
+  // Link this entry through the index.
+  if (parent.get()) {
+    parent->SetNextAddress(entry_address);
+  } else {
+    data_->table[hash & mask_] = entry_address.value();
+  }
+
+  // Link this entry through the lists.
   eviction_.OnCreateEntry(cache_entry);
-  entry_count_++;
-  if (!parent.get())
-    data_->table[hash & mask_] = entry_address.value();
 
   CACHE_UMA(AGE_MS, "CreateTime", GetSizeGroup(), start);
   stats_.OnEvent(Stats::CREATE_HIT);
   SIMPLE_STATS_COUNTER("disk_cache.miss");
   Trace("create entry hit ");
   return cache_entry.release();
 }
 
 EntryImpl* BackendImpl::OpenNextEntryImpl(void** iter) {
   return OpenFollowingEntry(true, iter);
@@ skipping 739 matching lines @@
     LOG(WARNING) << "Messed up entry found.";
     return ERR_INVALID_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))
-    return ERR_INVALID_LINKS;
+  if (!rankings_.SanityCheck(cache_entry->rankings(), false)) {
+    cache_entry->SetDirtyFlag(0);
+    // Don't remove this from the list (it is not linked properly). Instead,
+    // break the link back to the entry because it is going away, and leave the
+    // rankings node to be deleted if we find it through a list.
+    rankings_.SetContents(cache_entry->rankings(), 0);
+  } else if (!rankings_.DataSanityCheck(cache_entry->rankings(), false)) {
+    cache_entry->SetDirtyFlag(0);
+    rankings_.SetContents(cache_entry->rankings(), address.value());
+  }
+
+  if (!cache_entry->DataSanityCheck()) {
+    LOG(WARNING) << "Messed up entry found.";
+    cache_entry->SetDirtyFlag(0);
+    cache_entry->FixForDelete();
+  }
 
   if (cache_entry->dirty()) {
     Trace("Dirty entry 0x%p 0x%x", reinterpret_cast<void*>(cache_entry.get()),
           address.value());
   }
 
   open_entries_[address.value()] = cache_entry;
 
   cache_entry->BeginLogging(net_log_, false);
   cache_entry.swap(entry);
@@ skipping 129 matching lines @@
       return NULL;
   } else {
     // Get the next entry from the last list, and the actual entries for the
     // elements on the other lists.
     for (int i = 0; i < kListsToSearch; i++) {
       EntryImpl* temp = NULL;
       if (iterator->list == i) {
           OpenFollowingEntryFromList(forward, iterator->list,
                                      &iterator->nodes[i], &temp);
       } else {
-        temp = GetEnumeratedEntry(iterator->nodes[i]);
+        temp = GetEnumeratedEntry(iterator->nodes[i],
+                                  static_cast<Rankings::List>(i));
       }
 
       entries[i].swap(&temp);  // The entry was already addref'd.
     }
   }
 
   int newest = -1;
   int oldest = -1;
   Time access_times[kListsToSearch];
   for (int i = 0; i < kListsToSearch; i++) {
@@ skipping 36 matching lines @@
   if (!new_eviction_ && Rankings::NO_USE != list)
     return false;
 
   Rankings::ScopedRankingsBlock rankings(&rankings_, *from_entry);
   CacheRankingsBlock* next_block = forward ?
       rankings_.GetNext(rankings.get(), list) :
       rankings_.GetPrev(rankings.get(), list);
   Rankings::ScopedRankingsBlock next(&rankings_, next_block);
   *from_entry = NULL;
 
-  *next_entry = GetEnumeratedEntry(next.get());
+  *next_entry = GetEnumeratedEntry(next.get(), list);
   if (!*next_entry)
     return false;
 
   *from_entry = next.release();
   return true;
 }
 
-EntryImpl* BackendImpl::GetEnumeratedEntry(CacheRankingsBlock* next) {
+EntryImpl* BackendImpl::GetEnumeratedEntry(CacheRankingsBlock* next,
+                                           Rankings::List list) {
   if (!next || disabled_)
     return NULL;
 
   EntryImpl* entry;
-  if (NewEntry(Addr(next->Data()->contents), &entry)) {
-    // TODO(rvargas) bug 73102: We should remove this node from the list, and
-    // maybe do a better cleanup.
+  int rv = NewEntry(Addr(next->Data()->contents), &entry);
+  if (rv) {
+    rankings_.Remove(next, list, false);
+    if (rv == ERR_INVALID_ADDRESS) {
+      // There is nothing linked from the index. Delete the rankings node.
+      DeleteBlock(next->address(), true);
+    }
     return NULL;
   }
 
   if (entry->dirty()) {
     // We cannot trust this entry.
     InternalDoomEntry(entry);
     entry->Release();
     return NULL;
   }
 
@@ skipping 321 matching lines @@
     if (total_memory > kMaxBuffersSize || total_memory <= 0)
       total_memory = kMaxBuffersSize;
 
     done = true;
   }
 
   return static_cast<int>(total_memory);
 }
 
 }  // namespace disk_cache