Skip to content

Commit

Permalink
Ensure correct destruction order for FileProxy.
Browse files Browse the repository at this point in the history
This ensures that the base::FileProxy in FileFetcher is destroyed before
the TaskRunner that it's posting to from the destructor.

This may help with some of the ANRs.

b/304601532
  • Loading branch information
jellefoks committed Dec 4, 2024
1 parent e67b550 commit 318a771
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
16 changes: 10 additions & 6 deletions cobalt/loader/file_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ FileFetcher::FileFetcher(const base::FilePath& file_path, Handler* handler,
task_runner_(options.task_runner_proxy),
file_path_(file_path),
ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
file_proxy_(task_runner_.get()) {
file_proxy_(new base::FileProxy(task_runner_.get())) {
DCHECK_GT(buffer_size_, 0);

// Ensure the request does not attempt to navigate outside the whitelisted
Expand All @@ -107,6 +107,10 @@ FileFetcher::FileFetcher(const base::FilePath& file_path, Handler* handler,
FileFetcher::~FileFetcher() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);

// Ensure FileProxy runs its destructor before the TaskRunner that it posts
// to is destroyes.
file_proxy_.reset();

if (task_runner_ != base::SequencedTaskRunner::GetCurrentDefault()) {
// In case we are currently in the middle of a fetch (in which case it will
// be aborted), invalidate the weak pointers to this FileFetcher object to
Expand Down Expand Up @@ -145,18 +149,18 @@ void FileFetcher::TryFileOpen() {
base::FilePath actual_file_path;
actual_file_path = curr_search_path_iter_->Append(file_path_);

file_proxy_.CreateOrOpen(actual_file_path,
base::File::FLAG_OPEN | base::File::FLAG_READ,
base::Bind(&FileFetcher::DidCreateOrOpen,
weak_ptr_factory_.GetWeakPtr()));
file_proxy_->CreateOrOpen(actual_file_path,
base::File::FLAG_OPEN | base::File::FLAG_READ,
base::Bind(&FileFetcher::DidCreateOrOpen,
weak_ptr_factory_.GetWeakPtr()));
}

void FileFetcher::ReadNextChunk() {
int32 bytes_to_read = buffer_size_;
if (bytes_to_read > bytes_left_to_read_) {
bytes_to_read = static_cast<int32>(bytes_left_to_read_);
}
file_proxy_.Read(
file_proxy_->Read(
file_offset_, bytes_to_read,
base::Bind(&FileFetcher::DidRead, weak_ptr_factory_.GetWeakPtr()));
}
Expand Down
2 changes: 1 addition & 1 deletion cobalt/loader/file_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class FileFetcher : public Fetcher {
base::WeakPtrFactory<FileFetcher> weak_ptr_factory_;
// Current iterator into the search path vector.
std::vector<base::FilePath>::const_iterator curr_search_path_iter_;
base::FileProxy file_proxy_;
std::unique_ptr<base::FileProxy> file_proxy_;
};

} // namespace loader
Expand Down

0 comments on commit 318a771

Please sign in to comment.