Skip to content

Commit 855ebcb

Browse files
committed
Make FetcherConfig thread-safe by protecting it with a mutex.
Bug: b/461569672
1 parent 13fad52 commit 855ebcb

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include <cstddef>
2121
#include <fstream>
2222
#include <map>
23+
#include <memory>
24+
#include <mutex>
2325
#include <ostream>
2426
#include <string>
2527
#include <string_view>
@@ -68,7 +70,23 @@ std::ostream& operator<<(std::ostream& os, const CvdFile& cvd_file) {
6870
return os;
6971
}
7072

73+
FetcherConfig::FetcherConfig() : mutex_(std::make_unique<std::mutex>()) {}
74+
75+
FetcherConfig::FetcherConfig(FetcherConfig&& other) {
76+
mutex_ = std::move(other.mutex_);
77+
other.mutex_ = std::make_unique<std::mutex>();
78+
}
79+
80+
FetcherConfig& FetcherConfig::operator=(FetcherConfig&& other) {
81+
mutex_ = std::move(other.mutex_);
82+
other.mutex_ = std::make_unique<std::mutex>();
83+
84+
return *this;
85+
}
86+
7187
bool FetcherConfig::SaveToFile(const std::string& file) const {
88+
std::scoped_lock lock(*mutex_);
89+
7290
std::ofstream ofs(file);
7391
if (!ofs.is_open()) {
7492
LOG(ERROR) << "Unable to write to file " << file;
@@ -79,6 +97,8 @@ bool FetcherConfig::SaveToFile(const std::string& file) const {
7997
}
8098

8199
bool FetcherConfig::LoadFromFile(const std::string& file) {
100+
std::scoped_lock lock(*mutex_);
101+
82102
auto real_file_path = AbsolutePath(file);
83103
if (real_file_path.empty()) {
84104
LOG(ERROR) << "Could not get real path for file " << file;
@@ -135,6 +155,8 @@ Json::Value CvdFileToJson(const CvdFile& cvd_file) {
135155
} // namespace
136156

137157
bool FetcherConfig::add_cvd_file(const CvdFile& file, bool override_entry) {
158+
std::scoped_lock lock(*mutex_);
159+
138160
if (!dictionary_.isMember(kCvdFiles)) {
139161
Json::Value files_json(Json::objectValue);
140162
dictionary_[kCvdFiles] = files_json;
@@ -147,6 +169,8 @@ bool FetcherConfig::add_cvd_file(const CvdFile& file, bool override_entry) {
147169
}
148170

149171
std::map<std::string, CvdFile> FetcherConfig::get_cvd_files() const {
172+
std::scoped_lock lock(*mutex_);
173+
150174
if (!dictionary_.isMember(kCvdFiles)) {
151175
return {};
152176
}
@@ -160,6 +184,8 @@ std::map<std::string, CvdFile> FetcherConfig::get_cvd_files() const {
160184

161185
std::string FetcherConfig::FindCvdFileWithSuffix(
162186
const std::string& suffix) const {
187+
std::scoped_lock lock(*mutex_);
188+
163189
if (!dictionary_.isMember(kCvdFiles)) {
164190
return {};
165191
}
@@ -186,6 +212,9 @@ Result<void> FetcherConfig::AddFilesToConfig(
186212
FileSource purpose, const std::string& build_id,
187213
const std::string& build_target, const std::vector<std::string>& paths,
188214
const std::string& directory_prefix, bool override_entry) {
215+
// This neither acesses dictionary_ directly or locks *mutex, but it calls
216+
// `add_cvd_file` which does.
217+
189218
for (const std::string& path : paths) {
190219
std::string_view local_path(path);
191220
if (!android::base::ConsumePrefix(&local_path, directory_prefix)) {
@@ -208,6 +237,8 @@ Result<void> FetcherConfig::AddFilesToConfig(
208237
}
209238

210239
Result<void> FetcherConfig::RemoveFileFromConfig(const std::string& path) {
240+
std::scoped_lock lock(*mutex_);
241+
211242
if (!dictionary_.isMember(kCvdFiles)) {
212243
return {};
213244
}

base/cvd/cuttlefish/host/libs/config/fetcher_config.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#pragma once
1717

1818
#include <map>
19+
#include <memory>
20+
#include <mutex>
1921
#include <ostream>
2022
#include <string>
2123
#include <vector>
@@ -54,12 +56,16 @@ std::ostream& operator<<(std::ostream&, const CvdFile&);
5456
*
5557
* The output json also includes data relevant for human debugging, like which
5658
* flags fetch_cvd was invoked with.
59+
*
60+
* `FetcherConfig` is thread-safe for reads and writes, but the move constructor
61+
* and assignment operator are not thread-safe and cannot be called concurrently
62+
* with any other operations.
5763
*/
5864
class FetcherConfig {
5965
public:
60-
FetcherConfig() = default;
61-
FetcherConfig(FetcherConfig&&) = default;
62-
FetcherConfig& operator=(FetcherConfig&&) = default;
66+
FetcherConfig();
67+
FetcherConfig(FetcherConfig&&);
68+
FetcherConfig& operator=(FetcherConfig&&);
6369

6470
bool SaveToFile(const std::string& file) const;
6571
bool LoadFromFile(const std::string& file);
@@ -79,6 +85,7 @@ class FetcherConfig {
7985

8086
private:
8187
Json::Value dictionary_;
88+
std::unique_ptr<std::mutex> mutex_;
8289
};
8390

8491
class FetcherConfigs {

0 commit comments

Comments
 (0)