Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(647)

Unified Diff: net/http/http_cache.cc

Issue 2519473002: Fixes the cache lock issue. (Closed)
Patch Set: Feedback addressed Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/http/http_cache.h ('k') | net/http/http_cache_shared_writers.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/http/http_cache.cc
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index b02ddf02adf0a12669426d13131ce0e33839fe58..13da198d6f1e9bfbe6e16f0914a6b4fe5c755b9e 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();
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,29 @@ 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 {
+ return entry->shared_writers->AddTransaction(trans);
+ }
+ } else if (trans->mode() & Transaction::WRITE) { // No SharedWriters.
// transaction needs exclusive access to the entry
if (entry->readers.empty()) {
entry->writer = trans;
@@ -814,7 +825,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 +839,8 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
bool cancel) {
+ 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 +848,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 +867,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 +917,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 +936,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 +971,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->RemoveWaitingForValidationTransaction(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 +1015,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 +1180,117 @@ 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::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
« no previous file with comments | « net/http/http_cache.h ('k') | net/http/http_cache_shared_writers.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698