Fixes the cache lock issue.

net/http/http_cache.cc

Index: net/http/http_cache.cc
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index b02ddf02adf0a12669426d13131ce0e33839fe58..0e63f1d07eeaa3990f02bc22e1be9b73015a5461 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -35,6 +35,7 @@
 #include "net/base/upload_data_stream.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/http/disk_cache_based_quic_server_info.h"
+#include "net/http/http_cache_shared_writers.h"
 #include "net/http/http_cache_transaction.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_network_session.h"
@@ -341,7 +342,8 @@ HttpCache::~HttpCache() {
     entry->will_process_pending_queue = false;
     entry->pending_queue.clear();
     entry->readers.clear();
-    entry->writer = NULL;
+    entry->writer = nullptr;
+    entry->shared_writers.reset();

[jkarlin, 2017/02/03 18:26:19]

Is this necessary?

[shivanisha, 2017/02/06 21:14:10]

DeactivateEntry has multiple dchecks to assert that entry does not contain any
non null transaction pointers (including shared_writers) since DeactivateEntry
is invoked at multiple places, not just in the destructor.

     DeactivateEntry(entry);
   }
 
@@ -453,12 +455,12 @@ int HttpCache::CreateTransaction(RequestPriority priority,
     CreateBackend(NULL, CompletionCallback());
   }
 
-   HttpCache::Transaction* transaction =
+  HttpCache::Transaction* transaction =
       new HttpCache::Transaction(priority, this);
-   if (bypass_lock_for_test_)
+  if (bypass_lock_for_test_)
     transaction->BypassLockForTest();
-   if (fail_conditionalization_for_test_)
-     transaction->FailConditionalizationForTest();
+  if (fail_conditionalization_for_test_)
+    transaction->FailConditionalizationForTest();
 
   trans->reset(transaction);
   return OK;
@@ -584,8 +586,8 @@ int HttpCache::DoomEntry(const std::string& key, Transaction* trans) {
   entry_ptr->disk_entry->Doom();
   entry_ptr->doomed = true;
 
-  DCHECK(entry_ptr->writer || !entry_ptr->readers.empty() ||
-         entry_ptr->will_process_pending_queue);
+  DCHECK(entry_ptr->writer || entry_ptr->shared_writers ||
+         !entry_ptr->readers.empty() || entry_ptr->will_process_pending_queue);
   return OK;
 }
 
@@ -632,7 +634,7 @@ void HttpCache::DoomMainEntryForUrl(const GURL& url) {
 
 void HttpCache::FinalizeDoomedEntry(ActiveEntry* entry) {
   DCHECK(entry->doomed);
-  DCHECK(!entry->writer);
+  DCHECK(!entry->writer && !entry->shared_writers);
   DCHECK(entry->readers.empty());
   DCHECK(entry->pending_queue.empty());
 
@@ -657,7 +659,7 @@ HttpCache::ActiveEntry* HttpCache::ActivateEntry(
 void HttpCache::DeactivateEntry(ActiveEntry* entry) {
   DCHECK(!entry->will_process_pending_queue);
   DCHECK(!entry->doomed);
-  DCHECK(!entry->writer);
+  DCHECK(!entry->writer && !entry->shared_writers);
   DCHECK(entry->disk_entry);
   DCHECK(entry->readers.empty());
   DCHECK(entry->pending_queue.empty());
@@ -791,20 +793,35 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
   DCHECK(entry);
   DCHECK(entry->disk_entry);
 
-  // We implement a basic reader/writer lock for the disk cache entry.  If
-  // there is already a writer, then everyone has to wait for the writer to
-  // finish before they can access the cache entry.  There can be multiple
-  // readers.
+  // If a writer is in the validation stage, all other transactions need to wait
+  // before they can access the cache entry. Once the validation stage is over
+  // and if the writer creates a SharedWriters object, other eligible
+  // transactions will become equally privileged to be able to read from the
+  // network and write to the cache.
   //
   // NOTE: If the transaction can only write, then the entry should not be in
   // use (since any existing entry should have already been doomed).
-
   if (entry->writer || entry->will_process_pending_queue) {
     entry->pending_queue.push_back(trans);
     return ERR_IO_PENDING;
   }
 
-  if (trans->mode() & Transaction::WRITE) {
+  if (entry->shared_writers) {
+    DCHECK(!entry->writer);
+    if (!trans->IsEligibleForSharedWriting() ||
+        !entry->shared_writers->CanAddNewTransaction()) {
+      entry->pending_queue.push_back(trans);
+      return ERR_IO_PENDING;
+    } else {
+      if (entry->shared_writers->AddTransaction(trans)) {
+        return OK;
+      } else {
+        // Another transaction is in the process of validation, wait for it and
+        // any other transactions already in the queue to complete.
+        return ERR_IO_PENDING;
+      }
+    }
+  } else if (trans->mode() & Transaction::WRITE) {  // No SharedWriters.
     // transaction needs exclusive access to the entry
     if (entry->readers.empty()) {
       entry->writer = trans;
@@ -814,7 +831,7 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
     }
   } else {
     // transaction needs read access to the entry
-    entry->readers.push_back(trans);
+    entry->readers.insert(trans);
   }
 
   // We do this before calling EntryAvailable to force any further calls to
@@ -828,6 +845,9 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
 
 void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
                               bool cancel) {
+  // This is not expected to be called for shared writing transactions.

[jkarlin, 2017/02/03 18:26:19]

Comment unnecessary

[shivanisha, 2017/02/06 21:14:10]

Removed

+  DCHECK(!trans->shared());
+
   // If we already posted a task to move on to the next transaction and this was
   // the writer, there is nothing to cancel.
   if (entry->will_process_pending_queue && entry->readers.empty())
@@ -835,7 +855,6 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
 
   if (entry->writer) {
     DCHECK(trans == entry->writer);
-
     // Assume there was a failure.
     bool success = false;
     if (cancel) {
@@ -855,35 +874,41 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
 
 void HttpCache::DoneWritingToEntry(ActiveEntry* entry, bool success) {
   DCHECK(entry->readers.empty());
+  DCHECK(!entry->shared_writers);
 
   entry->writer = NULL;
 
   if (success) {
     ProcessPendingQueue(entry);
   } else {
-    DCHECK(!entry->will_process_pending_queue);
+    DestroyEntryRestartPendingQueue(entry);
+  }
+}
 
-    // We failed to create this entry.
-    TransactionList pending_queue;
-    pending_queue.swap(entry->pending_queue);
+void HttpCache::DestroyEntryRestartPendingQueue(ActiveEntry* entry) {
+  DCHECK(!entry->will_process_pending_queue);
 
-    entry->disk_entry->Doom();
-    DestroyEntry(entry);
+  // We failed to create this entry.
+  TransactionList pending_queue;
+  pending_queue.swap(entry->pending_queue);
 
-    // We need to do something about these pending entries, which now need to
-    // be added to a new entry.
-    while (!pending_queue.empty()) {
-      // ERR_CACHE_RACE causes the transaction to restart the whole process.
-      pending_queue.front()->io_callback().Run(ERR_CACHE_RACE);
-      pending_queue.pop_front();
-    }
+  entry->disk_entry->Doom();
+
+  DestroyEntry(entry);
+
+  // We need to do something about these pending entries, which now need to
+  // be added to a new entry.
+  while (!pending_queue.empty()) {
+    // ERR_CACHE_RACE causes the transaction to restart the whole process.
+    pending_queue.front()->io_callback().Run(ERR_CACHE_RACE);
+    pending_queue.pop_front();
   }
 }
 
 void HttpCache::DoneReadingFromEntry(ActiveEntry* entry, Transaction* trans) {
-  DCHECK(!entry->writer);
+  DCHECK(!entry->writer && !entry->shared_writers);
 
-  auto it = std::find(entry->readers.begin(), entry->readers.end(), trans);
+  auto it = entry->readers.find(trans);
   DCHECK(it != entry->readers.end());
 
   entry->readers.erase(it);
@@ -899,7 +924,7 @@ void HttpCache::ConvertWriterToReader(ActiveEntry* entry) {
   Transaction* trans = entry->writer;
 
   entry->writer = NULL;
-  entry->readers.push_back(trans);
+  entry->readers.insert(trans);
 
   ProcessPendingQueue(entry);
 }
@@ -918,8 +943,9 @@ LoadState HttpCache::GetLoadStateForPendingTransaction(
 }
 
 void HttpCache::RemovePendingTransaction(Transaction* trans) {
-  auto i = active_entries_.find(trans->key());
   bool found = false;
+
+  auto i = active_entries_.find(trans->key());
   if (i != active_entries_.end())
     found = RemovePendingTransactionFromEntry(i->second.get(), trans);
 
@@ -952,12 +978,13 @@ void HttpCache::RemovePendingTransaction(Transaction* trans) {
 
 bool HttpCache::RemovePendingTransactionFromEntry(ActiveEntry* entry,
                                                   Transaction* trans) {
-  TransactionList& pending_queue = entry->pending_queue;
+  if (trans->shared())
+    return entry->shared_writers->RemoveWaitingTransaction(trans);
 
+  TransactionList& pending_queue = entry->pending_queue;
   auto j = find(pending_queue.begin(), pending_queue.end(), trans);
   if (j == pending_queue.end())
     return false;
-
   pending_queue.erase(j);
   return true;
 }
@@ -995,7 +1022,7 @@ void HttpCache::ProcessPendingQueue(ActiveEntry* entry) {
 
 void HttpCache::OnProcessPendingQueue(ActiveEntry* entry) {
   entry->will_process_pending_queue = false;
-  DCHECK(!entry->writer);
+  DCHECK(!entry->writer && !entry->shared_writers);
 
   // If no one is interested in this entry, then we can deactivate it.
   if (entry->pending_queue.empty()) {
@@ -1160,4 +1187,140 @@ void HttpCache::OnBackendCreated(int result, PendingOp* pending_op) {
     item->NotifyTransaction(result, NULL);
 }
 
+bool HttpCache::IsResponseCompleted(const ActiveEntry* entry,
+                                    const HttpResponseInfo* response_info) {
+  int current_size = entry->disk_entry->GetDataSize(kResponseContentIndex);
+  int64_t content_length = response_info->headers->GetContentLength();
+  if ((content_length >= 0 && content_length <= current_size) ||
+      content_length < 0)
+    return true;
+  return false;
+}
+
+int HttpCache::WriteResponseInfo(ActiveEntry* entry,
+                                 const HttpResponseInfo* response,
+                                 CompletionCallback& callback,
+                                 bool truncated,
+                                 int* io_buf_len) {
+  // When writing headers, we normally only write the non-transient headers.
+  bool skip_transient_headers = true;
+  scoped_refptr<PickledIOBuffer> data(new PickledIOBuffer());
+  response->Persist(data->pickle(), skip_transient_headers, truncated);
+  data->Done();
+
+  *io_buf_len = data->pickle()->size();
+  return entry->disk_entry->WriteData(kResponseInfoIndex, 0, data.get(),
+                                      *io_buf_len, callback, true);
+}
+
+// Histogram data from the end of 2010 show the following distribution of
+// response headers:
+//
+//   Content-Length............... 87%
+//   Date......................... 98%
+//   Last-Modified................ 49%
+//   Etag......................... 19%
+//   Accept-Ranges: bytes......... 25%
+//   Accept-Ranges: none.......... 0.4%
+//   Strong Validator............. 50%
+//   Strong Validator + ranges.... 24%
+//   Strong Validator + CL........ 49%
+//
+bool HttpCache::CanResumeEntry(bool has_data,
+                               const std::string& method,
+                               const HttpResponseInfo* response,
+                               ActiveEntry* entry) {
+  // Double check that there is something worth keeping.
+  if (has_data && !entry->disk_entry->GetDataSize(kResponseContentIndex))
+    return false;
+
+  if (method != "GET")
+    return false;
+
+  // Note that if this is a 206, content-length was already fixed after calling
+  // PartialData::ResponseHeadersOK().
+  if (response->headers->GetContentLength() <= 0 ||
+      response->headers->HasHeaderValue("Accept-Ranges", "none") ||
+      !response->headers->HasStrongValidators()) {
+    return false;
+  }
+
+  return true;
+}
+
+void HttpCache::CreateSharedWriters(
+    Transaction* cache_transaction,
+    std::unique_ptr<HttpTransaction> network_transaction,
+    RequestPriority priority) {
+  ActiveEntry* entry = cache_transaction->entry();
+  DCHECK(!entry->shared_writers);
+  DCHECK_EQ(entry->writer, cache_transaction);
+  DCHECK(entry->readers.empty());
+  entry->shared_writers.reset(
+      new HttpCache::SharedWriters(this, entry, cache_transaction, priority,
+                                   std::move(network_transaction)));
+
+  // entry->writer should no longer exist as it is moved to
+  // shared_writers.
+  entry->writer = nullptr;
+
+  // Add the eligible transactions to waiting_for_validation_ and process the
+  // first among them. After this, pending_queue will only contain transactions
+  // that are not eligible for shared writing.
+  entry->shared_writers->MoveFromPendingQueue();
+  entry->shared_writers->ProcessFirstWaitingValidation();
+}
+
+void HttpCache::ResponseDoneSharedWriters(ActiveEntry* entry,
+                                          bool success,
+                                          bool* destroyed) {
+  DCHECK(entry->shared_writers->empty());
+  entry->shared_writers.reset();
+  *destroyed = true;
+
+  if (success)
+    ProcessPendingQueue(entry);
+  else
+    DestroyEntryRestartPendingQueue(entry);
+}
+
+void HttpCache::ResetSharedWriters(ActiveEntry* entry) {
+  DCHECK(entry->shared_writers->empty());
+  entry->shared_writers.reset();
+}
+
+void HttpCache::DoomEntryRestartPendingQueue(const std::string& key,
+                                             ActiveEntry* entry) {
+  DoomActiveEntry(key);
+  // We need to do something about these pending entries, which now need to
+  // be added to a new entry.
+  while (!entry->pending_queue.empty()) {
+    // ERR_CACHE_RACE causes the transaction to restart the whole process.
+    entry->pending_queue.front()->io_callback().Run(ERR_CACHE_RACE);
+    entry->pending_queue.pop_front();
+  }
+}
+
+void HttpCache::ResetSharedWritersProcessPendingQueue(ActiveEntry* entry) {
+  if (entry->shared_writers->empty()) {
+    entry->shared_writers.reset();
+    DCHECK(!entry->writer);
+    ProcessPendingQueue(entry);
+  }
+}
+
+void HttpCache::RemovedSharedWriterTransaction(Transaction* transaction,
+                                               ActiveEntry* entry) {
+  if (entry->shared_writers->empty()) {
+    bool success = false;
+    success = transaction->AddTruncatedFlag();
+    if (success) {
+      ResetSharedWritersProcessPendingQueue(entry);
+    } else {
+      entry->shared_writers.reset();
+      DestroyEntryRestartPendingQueue(entry);
+    }
+  }
+}
+
 }  // namespace net