Skip to content

Commit

Permalink
code style: scoped_lock (#2407)
Browse files Browse the repository at this point in the history
  • Loading branch information
battlmonstr authored Oct 8, 2024
1 parent 8c6d8db commit 6783426
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 24 deletions.
3 changes: 2 additions & 1 deletion docs/code_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ Bad:

### P4 RAII lock type usage

Use `std::scoped_lock` for a common case where a single mutex needs to be locked immediately and unlocked at the end of the scope. Use other lock types sparingly if other features are needed (e.g. multiple mutexes, deferred locking etc).
Use `std::scoped_lock` for a common case where a single mutex needs to be locked immediately and unlocked at the end of the scope. Do not use `std::lock_guard`.
Use `std::unique_lock` where a manual `unlock()` is required, for working with `std::condition_variable` or if other unique_lock features are needed (e.g. deferred locking, adoption).

### P5 SILKWORM_ASSERT

Expand Down
4 changes: 2 additions & 2 deletions silkworm/db/etl/collector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class Collector {

//! \brief Returns the hex representation of current load key (for progress tracking)
[[nodiscard]] std::string get_load_key() const {
std::unique_lock lock{mutex_};
std::scoped_lock lock{mutex_};
return loading_key_;
}

Expand All @@ -100,7 +100,7 @@ class Collector {
void flush_buffer(); // Write buffer to file

void set_loading_key(ByteView key) {
std::unique_lock lock{mutex_};
std::scoped_lock lock{mutex_};
loading_key_ = to_hex(key, true);
}

Expand Down
4 changes: 2 additions & 2 deletions silkworm/db/etl/in_memory_collector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class InMemoryCollector {

//! \brief Returns the hex representation of current load key (for progress tracking)
[[nodiscard]] std::string get_load_key() const {
std::unique_lock l{mutex_};
std::scoped_lock l{mutex_};
return loading_key_;
}

Expand All @@ -148,7 +148,7 @@ class InMemoryCollector {

// for progress tracking only
void set_loading_key(ByteView key) {
std::unique_lock l{mutex_};
std::scoped_lock l{mutex_};
loading_key_ = to_hex(key, true);
}
mutable std::mutex mutex_{}; // To sync loading_key_
Expand Down
8 changes: 4 additions & 4 deletions silkworm/db/kv/api/state_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ CoherentStateCache::CoherentStateCache(CoherentCacheConfig config) : config_(con

std::unique_ptr<StateView> CoherentStateCache::get_view(Transaction& tx) {
const auto view_id = tx.view_id();
std::unique_lock write_lock{rw_mutex_};
std::scoped_lock write_lock{rw_mutex_};
CoherentStateRoot* root = get_root(view_id);
return root->ready ? std::make_unique<CoherentStateView>(tx, this) : nullptr;
}
Expand Down Expand Up @@ -76,7 +76,7 @@ void CoherentStateCache::on_new_block(const api::StateChangeSet& state_changes_s
return;
}

std::unique_lock write_lock{rw_mutex_};
std::scoped_lock write_lock{rw_mutex_};

const auto view_id = state_changes_set.state_version_id;
CoherentStateRoot* root = advance_root(view_id);
Expand Down Expand Up @@ -248,7 +248,7 @@ Task<std::optional<Bytes>> CoherentStateCache::get(ByteView key, Transaction& tx
}

read_lock.unlock();
std::unique_lock write_lock{rw_mutex_};
std::scoped_lock write_lock{rw_mutex_};

kv.value = value;
add(std::move(kv), root_it->second.get(), view_id);
Expand Down Expand Up @@ -290,7 +290,7 @@ Task<std::optional<Bytes>> CoherentStateCache::get_code(ByteView key, Transactio
}

read_lock.unlock();
std::unique_lock write_lock{rw_mutex_};
std::scoped_lock write_lock{rw_mutex_};

kv.value = value;
add_code(std::move(kv), root_it->second.get(), view_id);
Expand Down
8 changes: 4 additions & 4 deletions silkworm/db/kv/grpc/server/state_change_collection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ namespace silkworm {

std::optional<StateChangeToken> StateChangeCollection::subscribe(StateChangeConsumer consumer,
StateChangeFilter /*filter*/) {
std::unique_lock consumers_lock{consumers_mutex_};
std::scoped_lock consumers_lock{consumers_mutex_};
StateChangeToken token = ++next_token_;
const auto [_, inserted] = consumers_.insert({token, consumer});
return inserted ? std::make_optional(token) : std::nullopt;
}

bool StateChangeCollection::unsubscribe(StateChangeToken token) {
std::unique_lock consumers_lock{consumers_mutex_};
std::scoped_lock consumers_lock{consumers_mutex_};
const auto consumer_it = consumers_.erase(token);
return consumer_it != 0;
}
Expand Down Expand Up @@ -204,7 +204,7 @@ void StateChangeCollection::notify_batch(uint64_t pending_base_fee, uint64_t gas
state_changes_.set_block_gas_limit(gas_limit);
state_changes_.set_state_version_id(tx_id_);

std::unique_lock consumers_lock{consumers_mutex_};
std::scoped_lock consumers_lock{consumers_mutex_};
for (const auto& [_, batch_callback] : consumers_) {
// Make a copy of state change batch for every consumer because lifecycle is independent
const std::optional<remote::StateChangeBatch> frozen_batch = state_changes_;
Expand All @@ -218,7 +218,7 @@ void StateChangeCollection::notify_batch(uint64_t pending_base_fee, uint64_t gas
}

void StateChangeCollection::close() {
std::unique_lock consumers_lock{consumers_mutex_};
std::scoped_lock consumers_lock{consumers_mutex_};
for (const auto& [_, batch_callback] : consumers_) {
SILK_DEBUG << "Notify close to callback=" << &batch_callback;
batch_callback(std::nullopt);
Expand Down
8 changes: 4 additions & 4 deletions silkworm/db/mdbx/bitmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ void IndexLoader::unwind_bitmaps_impl(RWTxn& txn, BlockNum to, const std::map<By
throw std::runtime_error("Operation cancelled");
}

std::unique_lock log_lck(log_mtx_);
std::scoped_lock log_lck{log_mtx_};
current_key_ = abridge(to_hex(key, true), kAddressLength);
log_time = now + 5s;
}
Expand Down Expand Up @@ -192,7 +192,7 @@ void IndexLoader::unwind_bitmaps_impl(RWTxn& txn, BlockNum to, const std::map<By
}
}

std::unique_lock log_lck(log_mtx_);
std::scoped_lock log_lck{log_mtx_};
current_key_.clear();
}

Expand Down Expand Up @@ -226,7 +226,7 @@ void IndexLoader::prune_bitmaps_impl(RWTxn& txn, BlockNum threshold) {
if (SignalHandler::signalled()) {
throw std::runtime_error("Operation cancelled");
}
std::unique_lock log_lck(log_mtx_);
std::scoped_lock log_lck{log_mtx_};
current_key_ = abridge(to_hex(data_key_view, true), kAddressLength);
log_time = now + 5s;
}
Expand Down Expand Up @@ -259,7 +259,7 @@ void IndexLoader::prune_bitmaps_impl(RWTxn& txn, BlockNum threshold) {
target_data = target.to_next(/*throw_notfound=*/false);
}

std::unique_lock log_lck(log_mtx_);
std::scoped_lock log_lck{log_mtx_};
current_key_.clear();
}

Expand Down
2 changes: 1 addition & 1 deletion silkworm/db/mdbx/bitmap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class IndexLoader {

//! \brief Returns the hex representation of currently processed key
[[nodiscard]] std::string get_current_key() const {
std::unique_lock lock{log_mtx_};
std::scoped_lock lock{log_mtx_};
return current_key_;
}

Expand Down
2 changes: 1 addition & 1 deletion silkworm/infra/common/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ void BufferBase::flush() {
line = std::regex_replace(line, kColorPattern, "");
colorized = false;
}
std::unique_lock out_lck{out_mtx};
std::scoped_lock out_lck{out_mtx};
auto& out = settings_.log_std_out ? std::cout : std::cerr;
out << line << '\n';
if (file_ && file_->is_open()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ evmc::bytes32 TrieLoader::calculate_root() {

if (const auto now{std::chrono::steady_clock::now()}; log_time <= now) {
SignalHandler::throw_if_signalled();
std::unique_lock log_lck(log_mtx_);
std::scoped_lock log_lck(log_mtx_);
log_key_ = to_hex(hashed_account_data_key_view, true);
log_time = now + 2s;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class TrieLoader {

//! \brief Returns the hex representation of current load key (for progress tracking)
[[nodiscard]] std::string get_log_key() const {
std::unique_lock lock{log_mtx_};
std::scoped_lock lock{log_mtx_};
return log_key_;
}

Expand Down
6 changes: 3 additions & 3 deletions silkworm/rpc/core/filter_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ FilterStorage::FilterStorage(size_t max_size, double max_filter_age) : generator
FilterStorage::FilterStorage(Generator& generator, size_t max_size, double max_filter_age) : generator_{generator}, max_size_{max_size}, max_filter_age_{max_filter_age} {}

std::optional<std::string> FilterStorage::add_filter(const StoredFilter& filter) {
std::lock_guard<std::mutex> lock(mutex_);
std::scoped_lock lock{mutex_};

if (storage_.size() >= max_size_) {
clean_up();
Expand Down Expand Up @@ -62,7 +62,7 @@ std::optional<std::string> FilterStorage::add_filter(const StoredFilter& filter)
}

bool FilterStorage::remove_filter(const std::string& filter_id) {
std::lock_guard<std::mutex> lock(mutex_);
std::scoped_lock lock{mutex_};

const auto itr = storage_.find(filter_id);
if (itr == storage_.end()) {
Expand All @@ -74,7 +74,7 @@ bool FilterStorage::remove_filter(const std::string& filter_id) {
}

std::optional<std::reference_wrapper<StoredFilter>> FilterStorage::get_filter(const std::string& filter_id) {
std::lock_guard<std::mutex> lock(mutex_);
std::scoped_lock lock{mutex_};

clean_up();

Expand Down

0 comments on commit 6783426

Please sign in to comment.