0

Fix buggy use of memset in Rankings::Iterator

Using memset over this causes the raw_ptr's private ptr to be reset
incorrectly, without updating the refcount. It causes the quarantine size to increase infinitely.

This patch is likely to be merged into beta, so I made the minimal
change. I would like to refactor this more in a second step.

Plan:
1. Minimal fix (this patch)
2. Refactor: https://chromium-review.googlesource.com/c/chromium/src/+/4189439

Bug: 1409814
Change-Id: I0f7f057fe04ac5e90e9f720473c03521752e0db7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4188566
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096455}
This commit is contained in:
Arthur Sonzogni
2023-01-24 22:51:56 +00:00
committed by Chromium LUCI CQ
parent 2c83630249
commit 70e0aee252
2 changed files with 12 additions and 7 deletions
net/disk_cache/blockfile

@ -206,9 +206,7 @@ Rankings::ScopedRankingsBlock::ScopedRankingsBlock(Rankings* rankings,
CacheRankingsBlock* node)
: std::unique_ptr<CacheRankingsBlock>(node), rankings_(rankings) {}
Rankings::Iterator::Iterator() {
memset(this, 0, sizeof(Iterator));
}
Rankings::Iterator::Iterator() = default;
void Rankings::Iterator::Reset() {
if (my_rankings) {
@ -216,7 +214,9 @@ void Rankings::Iterator::Reset() {
ScopedRankingsBlock(my_rankings, node);
}
}
memset(this, 0, sizeof(Iterator));
my_rankings = nullptr;
nodes = {nullptr, nullptr, nullptr};
list = List::NO_USE;
}
Rankings::Rankings() = default;

@ -10,6 +10,7 @@
#include <list>
#include <memory>
#include <array>
#include "base/memory/raw_ptr.h"
#include "net/disk_cache/blockfile/addr.h"
#include "net/disk_cache/blockfile/mapped_file.h"
@ -96,13 +97,17 @@ class Rankings {
// If we have multiple lists, we have to iterate through all at the same time.
// This structure keeps track of where we are on the iteration.
// TODO(https://crbug.com/1409814) refactor this struct to make it clearer
// this owns the `nodes`.
struct Iterator {
Iterator();
void Reset();
List list; // Which entry was returned to the user.
CacheRankingsBlock* nodes[3]; // Nodes on the first three lists.
raw_ptr<Rankings> my_rankings;
// Which entry was returned to the user.
List list = List::NO_USE;
// Nodes on the first three lists.
std::array<CacheRankingsBlock*, 3> nodes = {nullptr, nullptr, nullptr};
raw_ptr<Rankings> my_rankings = nullptr;
};
Rankings();