summaryrefslogtreecommitdiff
path: root/net/disk_cache
diff options
context:
space:
mode:
Diffstat (limited to 'net/disk_cache')
-rw-r--r--net/disk_cache/DIR_METADATA15
-rw-r--r--net/disk_cache/backend_unittest.cc317
-rw-r--r--net/disk_cache/blockfile/bitmap.h6
-rw-r--r--net/disk_cache/blockfile/sparse_control.cc28
-rw-r--r--net/disk_cache/blockfile/stats.cc6
-rw-r--r--net/disk_cache/blockfile/storage_block-inl.h33
-rw-r--r--net/disk_cache/blockfile/storage_block.h1
-rw-r--r--net/disk_cache/disk_cache_fuzzer.cc2
-rw-r--r--net/disk_cache/disk_cache_test_util.cc8
-rw-r--r--net/disk_cache/disk_cache_test_util.h11
-rw-r--r--net/disk_cache/entry_unittest.cc5
-rw-r--r--net/disk_cache/simple/DIR_METADATA15
-rw-r--r--net/disk_cache/simple/simple_backend_impl.cc6
-rw-r--r--net/disk_cache/simple/simple_backend_impl.h1
-rw-r--r--net/disk_cache/simple/simple_entry_impl.cc56
-rw-r--r--net/disk_cache/simple/simple_entry_impl.h28
-rw-r--r--net/disk_cache/simple/simple_synchronous_entry.h6
-rw-r--r--net/disk_cache/simple/simple_util.cc14
18 files changed, 349 insertions, 209 deletions
diff --git a/net/disk_cache/DIR_METADATA b/net/disk_cache/DIR_METADATA
index 65a7d593c..ab24ecc66 100644
--- a/net/disk_cache/DIR_METADATA
+++ b/net/disk_cache/DIR_METADATA
@@ -1,11 +1,6 @@
-# Metadata information for this directory.
-#
-# For more information on DIR_METADATA files, see:
-# https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/tools/dirmd/README.md
-#
-# For the schema of this file, see Metadata message:
-# https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/tools/dirmd/proto/dir_metadata.proto
-
-monorail {
+monorail: {
component: "Internals>Network>Cache"
-} \ No newline at end of file
+}
+buganizer_public: {
+ component_id: 1456578
+}
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 5ff45407e..53b43ceb3 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -5518,105 +5518,120 @@ TEST_F(DiskCacheBackendTest, SimpleDoomIter) {
run_loop.Run();
}
-// See https://crbug.com/1486958
+// See https://crbug.com/1486958 for non-corrupting version,
+// https://crbug.com/1510452 for corrupting one.
TEST_F(DiskCacheBackendTest, SimpleOpenIter) {
constexpr int kEntries = 50;
SetSimpleCacheMode();
- // Note: this test relies on InitCache() making sure the index is ready.
- InitCache();
- // We create a whole bunch of entries so that deleting them will hopefully
- // finish after the iteration, in order to reproduce timing for the bug.
- for (int i = 0; i < kEntries; ++i) {
- disk_cache::Entry* entry = nullptr;
- ASSERT_THAT(CreateEntry(base::NumberToString(i), &entry), IsOk());
- entry->Close();
- }
- RunUntilIdle(); // Make sure close completes.
- EXPECT_EQ(kEntries, cache_->GetEntryCount());
+ for (bool do_corrupt : {false, true}) {
+ SCOPED_TRACE(do_corrupt);
- // Iterate once to get the order.
- base::queue<std::string> keys;
- auto iterator = cache_->CreateIterator();
- base::RunLoop run_loop;
- base::RepeatingCallback<void(EntryResult)> collect_entry_key =
- base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
- if (result.net_error() == net::ERR_FAILED) {
- run_loop.Quit();
- return; // iteration complete.
- }
- ASSERT_EQ(result.net_error(), net::OK);
- disk_cache::Entry* entry = result.ReleaseEntry();
- keys.push(entry->GetKey());
- entry->Close();
- result = iterator->OpenNextEntry(collect_entry_key);
- EXPECT_EQ(result.net_error(), net::ERR_IO_PENDING);
- });
-
- disk_cache::EntryResult result = iterator->OpenNextEntry(collect_entry_key);
- ASSERT_EQ(result.net_error(), net::ERR_IO_PENDING);
- run_loop.Run();
+ // Note: this test relies on InitCache() making sure the index is ready.
+ InitCache();
- // Open all entries with iterator...
- int opened = 0;
- int iter_opened = 0;
- bool iter_done = false;
- auto all_done = [&]() { return opened == kEntries && iter_done; };
+ // We create a whole bunch of entries so that deleting them will hopefully
+ // finish after the iteration, in order to reproduce timing for the bug.
+ for (int i = 0; i < kEntries; ++i) {
+ disk_cache::Entry* entry = nullptr;
+ ASSERT_THAT(CreateEntry(base::NumberToString(i), &entry), IsOk());
+ entry->Close();
+ }
+ RunUntilIdle(); // Make sure close completes.
+ EXPECT_EQ(kEntries, cache_->GetEntryCount());
+
+ // Iterate once to get the order.
+ std::list<std::string> keys;
+ auto iterator = cache_->CreateIterator();
+ base::RunLoop run_loop;
+ base::RepeatingCallback<void(EntryResult)> collect_entry_key =
+ base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
+ if (result.net_error() == net::ERR_FAILED) {
+ run_loop.Quit();
+ return; // iteration complete.
+ }
+ ASSERT_EQ(result.net_error(), net::OK);
+ disk_cache::Entry* entry = result.ReleaseEntry();
+ keys.push_back(entry->GetKey());
+ entry->Close();
+ result = iterator->OpenNextEntry(collect_entry_key);
+ EXPECT_EQ(result.net_error(), net::ERR_IO_PENDING);
+ });
+
+ disk_cache::EntryResult result = iterator->OpenNextEntry(collect_entry_key);
+ ASSERT_EQ(result.net_error(), net::ERR_IO_PENDING);
+ run_loop.Run();
+
+ // Corrupt all the files, if we're exercising that.
+ if (do_corrupt) {
+ for (const auto& key : keys) {
+ EXPECT_TRUE(disk_cache::simple_util::CreateCorruptFileForTests(
+ key, cache_path_));
+ }
+ }
- iterator = cache_->CreateIterator();
- base::RunLoop run_loop2;
- base::RepeatingCallback<void(EntryResult)> handle_entry =
- base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
- ++iter_opened;
- if (result.net_error() == net::ERR_FAILED) {
- EXPECT_EQ(iter_opened - 1, kEntries);
- iter_done = true;
+ // Open all entries with iterator...
+ int opened = 0;
+ int iter_opened = 0;
+ bool iter_done = false;
+ auto all_done = [&]() { return opened == kEntries && iter_done; };
+
+ iterator = cache_->CreateIterator();
+ base::RunLoop run_loop2;
+ base::RepeatingCallback<void(EntryResult)> handle_entry =
+ base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
+ ++iter_opened;
+ if (result.net_error() == net::ERR_FAILED) {
+ EXPECT_EQ(iter_opened - 1, do_corrupt ? 0 : kEntries);
+ iter_done = true;
+ if (all_done()) {
+ run_loop2.Quit();
+ }
+ return; // iteration complete.
+ }
+ EXPECT_EQ(result.net_error(), net::OK);
+ result = iterator->OpenNextEntry(handle_entry);
+ EXPECT_EQ(result.net_error(), net::ERR_IO_PENDING);
+ });
+
+ result = iterator->OpenNextEntry(handle_entry);
+ ASSERT_EQ(result.net_error(), net::ERR_IO_PENDING);
+
+ // ... while simultaneously opening them via name.
+ auto handle_open_result =
+ base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
+ int expected_status = do_corrupt ? net::ERR_FAILED : net::OK;
+ if (result.net_error() == expected_status) {
+ ++opened;
+ }
if (all_done()) {
run_loop2.Quit();
}
- return; // iteration complete.
- }
- EXPECT_EQ(result.net_error(), net::OK);
- result = iterator->OpenNextEntry(handle_entry);
- EXPECT_EQ(result.net_error(), net::ERR_IO_PENDING);
- });
-
- result = iterator->OpenNextEntry(handle_entry);
- ASSERT_EQ(result.net_error(), net::ERR_IO_PENDING);
-
- // ... while simultaneously opening them via name.
- auto handle_open_result =
- base::BindLambdaForTesting([&](disk_cache::EntryResult result) {
- if (result.net_error() == net::OK) {
- ++opened;
- }
- if (all_done()) {
- run_loop2.Quit();
- }
- });
-
- base::RepeatingClosure open_one_entry = base::BindLambdaForTesting([&]() {
- std::string key = keys.front();
- keys.pop();
- disk_cache::EntryResult result =
- cache_->OpenEntry(key, net::DEFAULT_PRIORITY, handle_open_result);
- if (result.net_error() != net::ERR_IO_PENDING) {
- handle_open_result.Run(std::move(result));
- }
-
- if (!keys.empty()) {
- base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
- open_one_entry);
- }
- });
- base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
- open_one_entry);
-
- run_loop2.Run();
-
- // Should not have eaten any entries.
- EXPECT_EQ(kEntries, cache_->GetEntryCount());
+ });
+
+ base::RepeatingClosure open_one_entry = base::BindLambdaForTesting([&]() {
+ std::string key = keys.front();
+ keys.pop_front();
+ disk_cache::EntryResult result =
+ cache_->OpenEntry(key, net::DEFAULT_PRIORITY, handle_open_result);
+ if (result.net_error() != net::ERR_IO_PENDING) {
+ handle_open_result.Run(std::move(result));
+ }
+
+ if (!keys.empty()) {
+ base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
+ FROM_HERE, open_one_entry);
+ }
+ });
+ base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
+ open_one_entry);
+
+ run_loop2.Run();
+
+ // Should not have eaten any entries, if not corrupting them.
+ EXPECT_EQ(do_corrupt ? 0 : kEntries, cache_->GetEntryCount());
+ }
}
// Make sure that if we close an entry in callback from open/create we do not
@@ -5636,3 +5651,129 @@ TEST_F(DiskCacheBackendTest, BlockFileImmediateCloseNoDangle) {
EXPECT_EQ(result.net_error(), net::ERR_IO_PENDING);
run_loop.Run();
}
+
+// Test that when a write causes a doom, it doesn't result in wrong delivery
+// order of callbacks due to re-entrant operation execution.
+TEST_F(DiskCacheBackendTest, SimpleWriteOrderEviction) {
+ SetSimpleCacheMode();
+ SetMaxSize(4096);
+ InitCache();
+
+ // Writes of [1, 2, ..., kMaxSize] are more than enough to trigger eviction,
+ // as (1 + 80)*80/2 * 2 = 6480 (last * 2 since two streams are written).
+ constexpr int kMaxSize = 80;
+
+ scoped_refptr<net::IOBufferWithSize> buffer =
+ CacheTestCreateAndFillBuffer(kMaxSize, /*no_nulls=*/false);
+
+ disk_cache::Entry* entry = nullptr;
+ ASSERT_THAT(CreateEntry("key", &entry), IsOk());
+ ASSERT_TRUE(entry);
+
+ bool expected_next_write_stream_1 = true;
+ int expected_next_write_size = 1;
+ int next_offset = 0;
+ base::RunLoop run_loop;
+ for (int size = 1; size <= kMaxSize; ++size) {
+ entry->WriteData(/*index=*/1, /*offset = */ next_offset, buffer.get(),
+ /*buf_len=*/size,
+ base::BindLambdaForTesting([&](int result) {
+ EXPECT_TRUE(expected_next_write_stream_1);
+ EXPECT_EQ(result, expected_next_write_size);
+ expected_next_write_stream_1 = false;
+ }),
+ /*truncate=*/true);
+ // Stream 0 writes are used here because unlike with stream 1 ones,
+ // WriteDataInternal can succeed and queue response callback immediately.
+ entry->WriteData(/*index=*/0, /*offset = */ next_offset, buffer.get(),
+ /*buf_len=*/size,
+ base::BindLambdaForTesting([&](int result) {
+ EXPECT_FALSE(expected_next_write_stream_1);
+ EXPECT_EQ(result, expected_next_write_size);
+ expected_next_write_stream_1 = true;
+ ++expected_next_write_size;
+ if (expected_next_write_size == (kMaxSize + 1)) {
+ run_loop.Quit();
+ }
+ }),
+ /*truncate=*/true);
+ next_offset += size;
+ }
+
+ entry->Close();
+ run_loop.Run();
+}
+
+// Test that when a write causes a doom, it doesn't result in wrong delivery
+// order of callbacks due to re-entrant operation execution. Variant that
+// uses stream 0 ops only.
+TEST_F(DiskCacheBackendTest, SimpleWriteOrderEvictionStream0) {
+ SetSimpleCacheMode();
+ SetMaxSize(4096);
+ InitCache();
+
+ // Writes of [1, 2, ..., kMaxSize] are more than enough to trigger eviction,
+ // as (1 + 120)*120/2 = 7260.
+ constexpr int kMaxSize = 120;
+
+ scoped_refptr<net::IOBufferWithSize> buffer =
+ CacheTestCreateAndFillBuffer(kMaxSize, /*no_nulls=*/false);
+
+ disk_cache::Entry* entry = nullptr;
+ ASSERT_THAT(CreateEntry("key", &entry), IsOk());
+ ASSERT_TRUE(entry);
+
+ int expected_next_write_size = 1;
+ int next_offset = 0;
+ base::RunLoop run_loop;
+ for (int size = 1; size <= kMaxSize; ++size) {
+ // Stream 0 writes are used here because unlike with stream 1 ones,
+ // WriteDataInternal can succeed and queue response callback immediately.
+ entry->WriteData(/*index=*/0, /*offset = */ next_offset, buffer.get(),
+ /*buf_len=*/size,
+ base::BindLambdaForTesting([&](int result) {
+ EXPECT_EQ(result, expected_next_write_size);
+ ++expected_next_write_size;
+ if (expected_next_write_size == (kMaxSize + 1)) {
+ run_loop.Quit();
+ }
+ }),
+ /*truncate=*/true);
+ next_offset += size;
+ }
+
+ entry->Close();
+ run_loop.Run();
+}
+
+// Test to make sure that if entry creation triggers eviction, a queued up
+// close (possible with optimistic ops) doesn't run from within creation
+// completion handler (which is indirectly detected as a dangling pointer).
+TEST_F(DiskCacheBackendTest, SimpleNoCloseFromWithinCreate) {
+ SetSimpleCacheMode();
+ SetMaxSize(4096);
+ InitCache();
+
+ // Make entries big enough to force their eviction.
+ constexpr int kDataSize = 4097;
+
+ auto buffer = base::MakeRefCounted<net::IOBufferWithSize>(kDataSize);
+ CacheTestFillBuffer(buffer->data(), kDataSize, false);
+
+ for (int i = 0; i < 100; ++i) {
+ std::string key = base::NumberToString(i);
+ EntryResult entry_result =
+ cache_->CreateEntry(key, net::HIGHEST, base::DoNothing());
+ ASSERT_EQ(entry_result.net_error(), net::OK);
+ disk_cache::Entry* entry = entry_result.ReleaseEntry();
+ // Doing stream 0 write to avoid need for thread round-trips for it to take
+ // effect if SimpleEntryImpl runs it.
+ entry->WriteData(/*index=*/0, /*offset = */ 0, buffer.get(),
+ /*buf_len=*/kDataSize,
+ base::BindLambdaForTesting(
+ [&](int result) { EXPECT_EQ(kDataSize, result); }),
+ /*truncate=*/true);
+ entry->Close();
+ }
+ RunUntilIdle();
+}
diff --git a/net/disk_cache/blockfile/bitmap.h b/net/disk_cache/blockfile/bitmap.h
index 14fc503e3..4638aa2a5 100644
--- a/net/disk_cache/blockfile/bitmap.h
+++ b/net/disk_cache/blockfile/bitmap.h
@@ -10,6 +10,7 @@
#include <memory>
+#include "base/containers/span.h"
#include "base/memory/raw_ptr.h"
#include "net/base/net_export.h"
@@ -75,6 +76,11 @@ class NET_EXPORT_PRIVATE Bitmap {
// Gets a pointer to the internal map.
const uint32_t* GetMap() const { return map_; }
+ // Gets a span describing the internal map.
+ base::span<const uint32_t> GetSpan() const {
+ return base::make_span(GetMap(), static_cast<size_t>(ArraySize()));
+ }
+
// Sets a range of bits to |value|.
void SetRange(int begin, int end, bool value);
diff --git a/net/disk_cache/blockfile/sparse_control.cc b/net/disk_cache/blockfile/sparse_control.cc
index 8341de259..877521f64 100644
--- a/net/disk_cache/blockfile/sparse_control.cc
+++ b/net/disk_cache/blockfile/sparse_control.cc
@@ -412,7 +412,7 @@ int SparseControl::CreateSparseEntry() {
// Save the header. The bitmap is saved in the destructor.
scoped_refptr<net::IOBuffer> buf = base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<char*>(&sparse_header_), sizeof(sparse_header_));
+ base::as_chars(base::make_span(&sparse_header_, 1u)));
int rv = entry_->WriteData(kSparseIndex, 0, buf.get(), sizeof(sparse_header_),
CompletionOnceCallback(), false);
@@ -442,7 +442,7 @@ int SparseControl::OpenSparseEntry(int data_len) {
return net::ERR_CACHE_OPERATION_NOT_SUPPORTED;
scoped_refptr<net::IOBuffer> buf = base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<char*>(&sparse_header_), sizeof(sparse_header_));
+ base::as_chars(base::span(&sparse_header_, 1u)));
// Read header.
int rv = entry_->ReadData(kSparseIndex, 0, buf.get(), sizeof(sparse_header_),
@@ -496,9 +496,8 @@ bool SparseControl::OpenChild() {
child_->GetDataSize(kSparseIndex) < static_cast<int>(sizeof(child_data_)))
return KillChildAndContinue(key, false);
- scoped_refptr<net::WrappedIOBuffer> buf =
- base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<char*>(&child_data_), sizeof(child_data_));
+ auto buf = base::MakeRefCounted<net::WrappedIOBuffer>(
+ base::as_chars(base::make_span(&child_data_, 1u)));
// Read signature.
int rv = child_->ReadData(kSparseIndex, 0, buf.get(), sizeof(child_data_),
@@ -521,9 +520,8 @@ bool SparseControl::OpenChild() {
}
void SparseControl::CloseChild() {
- scoped_refptr<net::WrappedIOBuffer> buf =
- base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<char*>(&child_data_), sizeof(child_data_));
+ auto buf = base::MakeRefCounted<net::WrappedIOBuffer>(
+ base::as_chars(base::make_span(&child_data_, 1u)));
// Save the allocation bitmap before closing the child entry.
int rv = child_->WriteData(kSparseIndex, 0, buf.get(), sizeof(child_data_),
@@ -590,13 +588,12 @@ void SparseControl::SetChildBit(bool value) {
}
void SparseControl::WriteSparseData() {
- int len = children_map_.ArraySize() * 4;
- scoped_refptr<net::IOBuffer> buf = base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<const char*>(children_map_.GetMap()), len);
+ auto buf = base::MakeRefCounted<net::WrappedIOBuffer>(
+ base::as_chars(children_map_.GetSpan()));
int rv = entry_->WriteData(kSparseIndex, sizeof(sparse_header_), buf.get(),
- len, CompletionOnceCallback(), false);
- if (rv != len) {
+ buf->size(), CompletionOnceCallback(), false);
+ if (rv != buf->size()) {
DLOG(ERROR) << "Unable to save sparse map";
}
}
@@ -688,9 +685,8 @@ void SparseControl::InitChildData() {
memset(&child_data_, 0, sizeof(child_data_));
child_data_.header = sparse_header_;
- scoped_refptr<net::WrappedIOBuffer> buf =
- base::MakeRefCounted<net::WrappedIOBuffer>(
- reinterpret_cast<char*>(&child_data_), sizeof(child_data_));
+ auto buf = base::MakeRefCounted<net::WrappedIOBuffer>(
+ base::as_chars(base::make_span(&child_data_, 1u)));
int rv = child_->WriteData(kSparseIndex, 0, buf.get(), sizeof(child_data_),
CompletionOnceCallback(), false);
diff --git a/net/disk_cache/blockfile/stats.cc b/net/disk_cache/blockfile/stats.cc
index 912d46090..faaab657b 100644
--- a/net/disk_cache/blockfile/stats.cc
+++ b/net/disk_cache/blockfile/stats.cc
@@ -4,7 +4,9 @@
#include "net/disk_cache/blockfile/stats.h"
-#include "base/bits.h"
+#include <bit>
+#include <cstdint>
+
#include "base/check.h"
#include "base/format_macros.h"
#include "base/strings/string_util.h"
@@ -261,7 +263,7 @@ int Stats::GetStatsBucket(int32_t size) {
return (size - 20 * 1024) / 4096 + 11;
// From this point on, use a logarithmic scale.
- int result = base::bits::Log2Floor(size) + 1;
+ int result = std::bit_width<uint32_t>(size);
static_assert(kDataSizesLength > 16, "update the scale");
if (result >= kDataSizesLength)
diff --git a/net/disk_cache/blockfile/storage_block-inl.h b/net/disk_cache/blockfile/storage_block-inl.h
index cbb810cd8..161b81fa7 100644
--- a/net/disk_cache/blockfile/storage_block-inl.h
+++ b/net/disk_cache/blockfile/storage_block-inl.h
@@ -10,6 +10,8 @@
#include <stddef.h>
#include <stdint.h>
+#include <type_traits>
+
#include "base/hash/hash.h"
#include "base/logging.h"
#include "base/notreached.h"
@@ -19,9 +21,9 @@ namespace disk_cache {
template <typename T>
StorageBlock<T>::StorageBlock(MappedFile* file, Addr address)
: file_(file), address_(address) {
- if (address.num_blocks() > 1)
- extended_ = true;
- DCHECK(!address.is_initialized() || sizeof(*data_) == address.BlockSize())
+ static_assert(std::is_trivial_v<T>); // T is loaded as bytes from a file.
+ DCHECK_NE(address.num_blocks(), 0);
+ DCHECK(!address.is_initialized() || sizeof(T) == address.BlockSize())
<< address.value();
}
@@ -37,7 +39,6 @@ void StorageBlock<T>::CopyFrom(StorageBlock<T>* other) {
DCHECK(!other->modified_);
Discard();
address_ = other->address_;
- extended_ = other->extended_;
file_ = other->file_;
*Data() = *other->Data();
}
@@ -47,9 +48,7 @@ template<typename T> void* StorageBlock<T>::buffer() const {
}
template<typename T> size_t StorageBlock<T>::size() const {
- if (!extended_)
- return sizeof(*data_);
- return address_.num_blocks() * sizeof(*data_);
+ return address_.num_blocks() * sizeof(T);
}
template<typename T> int StorageBlock<T>::offset() const {
@@ -64,10 +63,7 @@ template<typename T> bool StorageBlock<T>::LazyInit(MappedFile* file,
}
file_ = file;
address_.set_value(address.value());
- if (address.num_blocks() > 1)
- extended_ = true;
-
- DCHECK(sizeof(*data_) == address.BlockSize());
+ DCHECK(sizeof(T) == address.BlockSize());
return true;
}
@@ -87,7 +83,6 @@ template<typename T> void StorageBlock<T>::Discard() {
DeleteData();
data_ = nullptr;
modified_ = false;
- extended_ = false;
}
template<typename T> void StorageBlock<T>::StopSharingData() {
@@ -185,23 +180,13 @@ template<typename T> bool StorageBlock<T>::Store(FileIOCallback* callback,
template<typename T> void StorageBlock<T>::AllocateData() {
DCHECK(!data_);
- if (!extended_) {
- data_ = new T;
- } else {
- void* buffer = new char[address_.num_blocks() * sizeof(*data_)];
- data_ = new(buffer) T;
- }
+ data_ = new T[address_.num_blocks()];
own_data_ = true;
}
template<typename T> void StorageBlock<T>::DeleteData() {
if (own_data_) {
- if (!extended_) {
- data_.ClearAndDelete();
- } else {
- data_->~T();
- delete[] reinterpret_cast<char*>(data_.ExtractAsDangling().get());
- }
+ data_.ClearAndDeleteArray();
own_data_ = false;
}
}
diff --git a/net/disk_cache/blockfile/storage_block.h b/net/disk_cache/blockfile/storage_block.h
index e17b1733a..1ccb23a73 100644
--- a/net/disk_cache/blockfile/storage_block.h
+++ b/net/disk_cache/blockfile/storage_block.h
@@ -102,7 +102,6 @@ class StorageBlock : public FileBlock {
bool modified_ = false;
// Is data_ owned by this object or shared with someone else.
bool own_data_ = false;
- bool extended_ = false; // Used to store an entry of more than one block.
};
} // namespace disk_cache
diff --git a/net/disk_cache/disk_cache_fuzzer.cc b/net/disk_cache/disk_cache_fuzzer.cc
index a0dcbdf55..0f2155aec 100644
--- a/net/disk_cache/disk_cache_fuzzer.cc
+++ b/net/disk_cache/disk_cache_fuzzer.cc
@@ -105,7 +105,7 @@ struct InitGlobals {
// Disable noisy logging as per "libFuzzer in Chrome" documentation:
// testing/libfuzzer/getting_started.md#Disable-noisy-error-message-logging.
- logging::SetMinLogLevel(logging::LOG_FATAL);
+ logging::SetMinLogLevel(logging::LOGGING_FATAL);
// Re-using this buffer for write operations may technically be against
// IOBuffer rules but it shouldn't cause any actual problems.
diff --git a/net/disk_cache/disk_cache_test_util.cc b/net/disk_cache/disk_cache_test_util.cc
index c358160a0..7957874d8 100644
--- a/net/disk_cache/disk_cache_test_util.cc
+++ b/net/disk_cache/disk_cache_test_util.cc
@@ -41,6 +41,14 @@ void CacheTestFillBuffer(char* buffer, size_t len, bool no_nulls) {
buffer[0] = 'g';
}
+scoped_refptr<net::IOBufferWithSize> CacheTestCreateAndFillBuffer(
+ size_t len,
+ bool no_nulls) {
+ auto buffer = base::MakeRefCounted<net::IOBufferWithSize>(len);
+ CacheTestFillBuffer(buffer->data(), len, no_nulls);
+ return buffer;
+}
+
bool CreateCacheTestFile(const base::FilePath& name) {
int flags = base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ |
base::File::FLAG_WRITE;
diff --git a/net/disk_cache/disk_cache_test_util.h b/net/disk_cache/disk_cache_test_util.h
index e4f7db938..d4228cb43 100644
--- a/net/disk_cache/disk_cache_test_util.h
+++ b/net/disk_cache/disk_cache_test_util.h
@@ -12,12 +12,17 @@
#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
+#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
#include "base/timer/timer.h"
#include "build/build_config.h"
#include "net/base/test_completion_callback.h"
#include "net/disk_cache/disk_cache.h"
+namespace net {
+class IOBufferWithSize;
+} // namespace net
+
// Re-creates a given test file inside the cache test folder.
bool CreateCacheTestFile(const base::FilePath& name);
@@ -27,6 +32,12 @@ bool DeleteCache(const base::FilePath& path);
// Fills buffer with random values (may contain nulls unless no_nulls is true).
void CacheTestFillBuffer(char* buffer, size_t len, bool no_nulls);
+// Creates a buffer of size `len`, and fills in with random values, which
+// may contain 0 unless `no_nulls` is true.
+scoped_refptr<net::IOBufferWithSize> CacheTestCreateAndFillBuffer(
+ size_t len,
+ bool no_nulls);
+
// Generates a random key of up to 200 bytes.
std::string GenerateKey(bool same_length);
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 9a56851af..c8a4832e9 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -2042,9 +2042,8 @@ TEST_F(DiskCacheEntryTest, MemoryOnlyMisalignedSparseIO) {
// This loop writes back to back starting from offset 0 and 9000.
for (int i = 0; i < kSize; i += 1024) {
- scoped_refptr<net::WrappedIOBuffer> buf_3 =
- base::MakeRefCounted<net::WrappedIOBuffer>(buf_1->data() + i,
- kSize - i);
+ auto buf_3 =
+ base::MakeRefCounted<net::WrappedIOBuffer>(buf_1->span().subspan(i));
VerifySparseIO(entry, i, buf_3.get(), 1024, buf_2.get());
VerifySparseIO(entry, 9000 + i, buf_3.get(), 1024, buf_2.get());
}
diff --git a/net/disk_cache/simple/DIR_METADATA b/net/disk_cache/simple/DIR_METADATA
index 8004f8fa1..8722368bf 100644
--- a/net/disk_cache/simple/DIR_METADATA
+++ b/net/disk_cache/simple/DIR_METADATA
@@ -1,11 +1,6 @@
-# Metadata information for this directory.
-#
-# For more information on DIR_METADATA files, see:
-# https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/tools/dirmd/README.md
-#
-# For the schema of this file, see Metadata message:
-# https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/tools/dirmd/proto/dir_metadata.proto
-
-monorail {
+monorail: {
component: "Internals>Network>Cache>Simple"
-} \ No newline at end of file
+}
+buganizer_public: {
+ component_id: 1456938
+}
diff --git a/net/disk_cache/simple/simple_backend_impl.cc b/net/disk_cache/simple/simple_backend_impl.cc
index c31d50380..172694a8a 100644
--- a/net/disk_cache/simple/simple_backend_impl.cc
+++ b/net/disk_cache/simple/simple_backend_impl.cc
@@ -864,9 +864,8 @@ EntryResult SimpleBackendImpl::OpenEntryFromHash(uint64_t entry_hash,
simple_entry->SetActiveEntryProxy(
ActiveEntryProxy::Create(entry_hash, this));
post_open_by_hash_waiting_->OnOperationStart(entry_hash);
- callback =
- base::BindOnce(&SimpleBackendImpl::OnEntryOpenedFromHash, AsWeakPtr(),
- entry_hash, simple_entry, std::move(callback));
+ callback = base::BindOnce(&SimpleBackendImpl::OnEntryOpenedFromHash,
+ AsWeakPtr(), entry_hash, std::move(callback));
}
// Note: the !did_insert case includes when another OpenEntryFromHash is
@@ -904,7 +903,6 @@ net::Error SimpleBackendImpl::DoomEntryFromHash(
void SimpleBackendImpl::OnEntryOpenedFromHash(
uint64_t hash,
- const scoped_refptr<SimpleEntryImpl>& simple_entry,
EntryResultCallback callback,
EntryResult result) {
post_open_by_hash_waiting_->OnOperationComplete(hash);
diff --git a/net/disk_cache/simple/simple_backend_impl.h b/net/disk_cache/simple/simple_backend_impl.h
index 41e0a36a0..7032253ab 100644
--- a/net/disk_cache/simple/simple_backend_impl.h
+++ b/net/disk_cache/simple/simple_backend_impl.h
@@ -244,7 +244,6 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
// hash alone - this resumes operations that were waiting on the key
// information to have been loaded.
void OnEntryOpenedFromHash(uint64_t hash,
- const scoped_refptr<SimpleEntryImpl>& simple_entry,
EntryResultCallback callback,
EntryResult result);
diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc
index 13e312d27..41ce42f3f 100644
--- a/net/disk_cache/simple/simple_entry_impl.cc
+++ b/net/disk_cache/simple/simple_entry_impl.cc
@@ -455,8 +455,12 @@ int SimpleEntryImpl::WriteData(int stream_index,
// Stream 0 data is kept in memory, so can be written immediatly if there are
// no IO operations pending.
if (stream_index == 0 && state_ == STATE_READY &&
- pending_operations_.size() == 0)
- return SetStream0Data(buf, offset, buf_len, truncate);
+ pending_operations_.size() == 0) {
+ state_ = STATE_IO_PENDING;
+ SetStream0Data(buf, offset, buf_len, truncate);
+ state_ = STATE_READY;
+ return buf_len;
+ }
// We can only do optimistic Write if there is no pending operations, so
// that we are sure that the next call to RunNextOperationIfNeeded will
@@ -1012,16 +1016,20 @@ int SimpleEntryImpl::ReadDataInternal(bool sync_possible,
// Since stream 0 data is kept in memory, it is read immediately.
if (stream_index == 0) {
- int rv = ReadFromBuffer(stream_0_data_.get(), offset, buf_len, buf);
- return PostToCallbackIfNeeded(sync_possible, std::move(callback), rv);
+ state_ = STATE_IO_PENDING;
+ ReadFromBuffer(stream_0_data_.get(), offset, buf_len, buf);
+ state_ = STATE_READY;
+ return PostToCallbackIfNeeded(sync_possible, std::move(callback), buf_len);
}
// Sometimes we can read in-ram prefetched stream 1 data immediately, too.
if (stream_index == 1) {
if (stream_1_prefetch_data_) {
- int rv =
- ReadFromBuffer(stream_1_prefetch_data_.get(), offset, buf_len, buf);
- return PostToCallbackIfNeeded(sync_possible, std::move(callback), rv);
+ state_ = STATE_IO_PENDING;
+ ReadFromBuffer(stream_1_prefetch_data_.get(), offset, buf_len, buf);
+ state_ = STATE_READY;
+ return PostToCallbackIfNeeded(sync_possible, std::move(callback),
+ buf_len);
}
}
@@ -1090,10 +1098,12 @@ void SimpleEntryImpl::WriteDataInternal(int stream_index,
// Since stream 0 data is kept in memory, it will be written immediatly.
if (stream_index == 0) {
- int ret_value = SetStream0Data(buf, offset, buf_len, truncate);
+ state_ = STATE_IO_PENDING;
+ SetStream0Data(buf, offset, buf_len, truncate);
+ state_ = STATE_READY;
if (!callback.is_null()) {
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
- FROM_HERE, base::BindOnce(std::move(callback), ret_value));
+ FROM_HERE, base::BindOnce(std::move(callback), buf_len));
}
return;
}
@@ -1407,7 +1417,6 @@ void SimpleEntryImpl::CreationOperationComplete(
if (backend_ && doom_state_ == DOOM_NONE)
backend_->index()->Insert(entry_hash_);
- state_ = STATE_READY;
synchronous_entry_ = in_results->sync_entry;
// Copy over any pre-fetched data and its CRCs.
@@ -1459,6 +1468,8 @@ void SimpleEntryImpl::CreationOperationComplete(
// ultimately release `in_results->sync_entry`, and thus leading to having a
// dangling pointer here.
in_results = nullptr;
+
+ state_ = STATE_READY;
if (result_state == SimpleEntryOperation::ENTRY_NEEDS_CALLBACK) {
ReturnEntryToCallerAsync(!created, std::move(completion_callback));
}
@@ -1474,8 +1485,8 @@ void SimpleEntryImpl::UpdateStateAfterOperationComplete(
state_ = STATE_FAILURE;
MarkAsDoomed(DOOM_COMPLETED);
} else {
- state_ = STATE_READY;
UpdateDataFromEntryStat(entry_stat);
+ state_ = STATE_READY;
}
}
@@ -1637,7 +1648,10 @@ void SimpleEntryImpl::UpdateDataFromEntryStat(
const SimpleEntryStat& entry_stat) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(synchronous_entry_);
- DCHECK_EQ(STATE_READY, state_);
+ // We want to only be called in STATE_IO_PENDING so that if call to
+ // SimpleIndex::UpdateEntrySize() ends up triggering eviction and queuing
+ // Dooms it doesn't also run any queued operations.
+ CHECK_EQ(state_, STATE_IO_PENDING);
last_used_ = entry_stat.last_used();
last_modified_ = entry_stat.last_modified();
@@ -1662,23 +1676,22 @@ int64_t SimpleEntryImpl::GetDiskUsage() const {
return file_size;
}
-int SimpleEntryImpl::ReadFromBuffer(net::GrowableIOBuffer* in_buf,
- int offset,
- int buf_len,
- net::IOBuffer* out_buf) {
+void SimpleEntryImpl::ReadFromBuffer(net::GrowableIOBuffer* in_buf,
+ int offset,
+ int buf_len,
+ net::IOBuffer* out_buf) {
DCHECK_GE(buf_len, 0);
std::copy(in_buf->data() + offset, in_buf->data() + offset + buf_len,
out_buf->data());
UpdateDataFromEntryStat(SimpleEntryStat(base::Time::Now(), last_modified_,
data_size_, sparse_data_size_));
- return buf_len;
}
-int SimpleEntryImpl::SetStream0Data(net::IOBuffer* buf,
- int offset,
- int buf_len,
- bool truncate) {
+void SimpleEntryImpl::SetStream0Data(net::IOBuffer* buf,
+ int offset,
+ int buf_len,
+ bool truncate) {
// Currently, stream 0 is only used for HTTP headers, and always writes them
// with a single, truncating write. Detect these writes and record the size
// changes of the headers. Also, support writes to stream 0 that have
@@ -1717,7 +1730,6 @@ int SimpleEntryImpl::SetStream0Data(net::IOBuffer* buf,
UpdateDataFromEntryStat(
SimpleEntryStat(modification_time, modification_time, data_size_,
sparse_data_size_));
- return buf_len;
}
} // namespace disk_cache
diff --git a/net/disk_cache/simple/simple_entry_impl.h b/net/disk_cache/simple/simple_entry_impl.h
index 5703935ab..c7816fdff 100644
--- a/net/disk_cache/simple/simple_entry_impl.h
+++ b/net/disk_cache/simple/simple_entry_impl.h
@@ -343,20 +343,21 @@ class NET_EXPORT_PRIVATE SimpleEntryImpl : public Entry,
int64_t GetDiskUsage() const;
// Completes a read from the stream data kept in memory, logging metrics
- // and updating metadata. Returns the # of bytes read successfully.
- // This asumes the caller has already range-checked offset and buf_len
- // appropriately.
- int ReadFromBuffer(net::GrowableIOBuffer* in_buf,
- int offset,
- int buf_len,
- net::IOBuffer* out_buf);
+ // and updating metadata. This assumes the caller has already range-checked
+ // offset and buf_len appropriately, and therefore always reads `buf_len`
+ // bytes.
+ void ReadFromBuffer(net::GrowableIOBuffer* in_buf,
+ int offset,
+ int buf_len,
+ net::IOBuffer* out_buf);
// Copies data from |buf| to the internal in-memory buffer for stream 0. If
// |truncate| is set to true, the target buffer will be truncated at |offset|
// + |buf_len| before being written.
- int SetStream0Data(net::IOBuffer* buf,
- int offset, int buf_len,
- bool truncate);
+ void SetStream0Data(net::IOBuffer* buf,
+ int offset,
+ int buf_len,
+ bool truncate);
// We want all async I/O on entries to complete before recycling the dir.
scoped_refptr<BackendCleanupTracker> cleanup_tracker_;
@@ -421,12 +422,7 @@ class NET_EXPORT_PRIVATE SimpleEntryImpl : public Entry,
// an operation is being executed no one owns the synchronous entry. Therefore
// SimpleEntryImpl should not be deleted while an operation is running as that
// would leak the SimpleSynchronousEntry.
- // This dangling raw_ptr occurred in:
- // content_unittests:
- // GeneratedCodeCacheTest/GeneratedCodeCacheTest.StressVeryLargeEntries/1
- // https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1425125/test-results?q=ExactID%3Aninja%3A%2F%2Fcontent%2Ftest%3Acontent_unittests%2FGeneratedCodeCacheTest.StressVeryLargeEntries%2FGeneratedCodeCacheTest.1+VHash%3Ab3ba0803668e9981&sortby=&groupby=
- raw_ptr<SimpleSynchronousEntry, FlakyDanglingUntriaged> synchronous_entry_ =
- nullptr;
+ raw_ptr<SimpleSynchronousEntry> synchronous_entry_ = nullptr;
scoped_refptr<net::PrioritizedTaskRunner> prioritized_task_runner_;
diff --git a/net/disk_cache/simple/simple_synchronous_entry.h b/net/disk_cache/simple/simple_synchronous_entry.h
index 112b5f8dd..0d24aa038 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.h
+++ b/net/disk_cache/simple/simple_synchronous_entry.h
@@ -105,11 +105,7 @@ struct SimpleEntryCreationResults {
explicit SimpleEntryCreationResults(SimpleEntryStat entry_stat);
~SimpleEntryCreationResults();
- // This dangling raw_ptr occurred in:
- // content_unittests:
- // GeneratedCodeCacheTest/GeneratedCodeCacheTest.StressVeryLargeEntries/1
- // https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1425125/test-results?q=ExactID%3Aninja%3A%2F%2Fcontent%2Ftest%3Acontent_unittests%2FGeneratedCodeCacheTest.StressVeryLargeEntries%2FGeneratedCodeCacheTest.1+VHash%3Ab3ba0803668e9981&sortby=&groupby=
- raw_ptr<SimpleSynchronousEntry, FlakyDanglingUntriaged> sync_entry;
+ raw_ptr<SimpleSynchronousEntry> sync_entry;
// This is set when `sync_entry` is null.
std::unique_ptr<UnboundBackendFileOperations> unbound_file_operations;
diff --git a/net/disk_cache/simple/simple_util.cc b/net/disk_cache/simple/simple_util.cc
index d823335b5..369412a2e 100644
--- a/net/disk_cache/simple/simple_util.cc
+++ b/net/disk_cache/simple/simple_util.cc
@@ -4,6 +4,8 @@
#include "net/disk_cache/simple/simple_util.h"
+#include <string.h>
+
#include <limits>
#include "base/check_op.h"
@@ -49,13 +51,13 @@ bool GetEntryHashKeyFromHexString(base::StringPiece hash_key,
}
uint64_t GetEntryHashKey(const std::string& key) {
- union {
- unsigned char sha_hash[base::kSHA1Length];
- uint64_t key_hash;
- } u;
+ unsigned char sha_hash[base::kSHA1Length];
+
base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(key.data()),
- key.size(), u.sha_hash);
- return u.key_hash;
+ key.size(), sha_hash);
+ uint64_t as_uint64;
+ memcpy(&as_uint64, sha_hash, sizeof(as_uint64));
+ return as_uint64;
}
std::string GetFilenameFromEntryFileKeyAndFileIndex(
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-07 11:30:41 +0000