This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 316b325d refactor: use lock_guard instead of unique_lock when
available (#398)
316b325d is described below
commit 316b325d7bea68865569ccdafdab1c22e1d52310
Author: Zehua Zou <[email protected]>
AuthorDate: Sun Dec 7 14:10:36 2025 +0800
refactor: use lock_guard instead of unique_lock when available (#398)
Follow the answer in stackoverflow
https://stackoverflow.com/a/60172828/11568166
1. `lock_guard` if you need to lock exactly 1 mutex for an entire scope.
2. `scoped_lock` if you need to lock a number of mutexes that is not
exactly 1.
3. `unique_lock` if you need to unlock within the scope of the block
(which includes use with a condition_variable).
So let clang-tidy only report `modernize-use-scoped-lock` warning when
there are multiple `lock_guard`.
---
.clang-tidy | 2 ++
src/iceberg/catalog/memory/in_memory_catalog.cc | 26 ++++++++++++-------------
src/iceberg/catalog/rest/http_client.cc | 10 +++++-----
src/iceberg/table_metadata.h | 2 +-
4 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
index c6fcc6c5..8977deae 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -38,5 +38,7 @@ CheckOptions:
value: '_'
- key: readability-identifier-naming.ProtectedMemberSuffix
value: '_'
+ - key: modernize-use-scoped-lock.WarnOnSingleLocks
+ value: 'false'
HeaderFilterRegex: 'src/iceberg|example'
diff --git a/src/iceberg/catalog/memory/in_memory_catalog.cc
b/src/iceberg/catalog/memory/in_memory_catalog.cc
index ebc49010..d644c44f 100644
--- a/src/iceberg/catalog/memory/in_memory_catalog.cc
+++ b/src/iceberg/catalog/memory/in_memory_catalog.cc
@@ -20,9 +20,9 @@
#include "iceberg/catalog/memory/in_memory_catalog.h"
#include <algorithm>
-#include <iterator> // IWYU pragma: keep
+#include <iterator>
+#include <mutex>
-#include "iceberg/exception.h"
#include "iceberg/table.h"
#include "iceberg/table_metadata.h"
#include "iceberg/util/macros.h"
@@ -337,42 +337,42 @@ std::string_view InMemoryCatalog::name() const { return
catalog_name_; }
Status InMemoryCatalog::CreateNamespace(
const Namespace& ns, const std::unordered_map<std::string, std::string>&
properties) {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->CreateNamespace(ns, properties);
}
Result<std::unordered_map<std::string, std::string>>
InMemoryCatalog::GetNamespaceProperties(const Namespace& ns) const {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->GetProperties(ns);
}
Result<std::vector<Namespace>> InMemoryCatalog::ListNamespaces(
const Namespace& ns) const {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->ListNamespaces(ns);
}
Status InMemoryCatalog::DropNamespace(const Namespace& ns) {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->DropNamespace(ns);
}
Result<bool> InMemoryCatalog::NamespaceExists(const Namespace& ns) const {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->NamespaceExists(ns);
}
Status InMemoryCatalog::UpdateNamespaceProperties(
const Namespace& ns, const std::unordered_map<std::string, std::string>&
updates,
const std::unordered_set<std::string>& removals) {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->UpdateNamespaceProperties(ns, updates, removals);
}
Result<std::vector<TableIdentifier>> InMemoryCatalog::ListTables(
const Namespace& ns) const {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
const auto& table_names = root_namespace_->ListTables(ns);
ICEBERG_RETURN_UNEXPECTED(table_names);
std::vector<TableIdentifier> table_idents;
@@ -405,12 +405,12 @@ Result<std::shared_ptr<Transaction>>
InMemoryCatalog::StageCreateTable(
}
Result<bool> InMemoryCatalog::TableExists(const TableIdentifier& identifier)
const {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
return root_namespace_->TableExists(identifier);
}
Status InMemoryCatalog::DropTable(const TableIdentifier& identifier, bool
purge) {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
// TODO(Guotao): Delete all metadata files if purge is true.
return root_namespace_->UnregisterTable(identifier);
}
@@ -428,7 +428,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(
Result<std::string> metadata_location;
{
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
ICEBERG_ASSIGN_OR_RAISE(metadata_location,
root_namespace_->GetTableMetadataLocation(identifier));
}
@@ -443,7 +443,7 @@ Result<std::unique_ptr<Table>> InMemoryCatalog::LoadTable(
Result<std::shared_ptr<Table>> InMemoryCatalog::RegisterTable(
const TableIdentifier& identifier, const std::string&
metadata_file_location) {
- std::unique_lock lock(mutex_);
+ std::lock_guard guard(mutex_);
if (!root_namespace_->NamespaceExists(identifier.ns)) {
return NoSuchNamespace("table namespace does not exist.");
}
diff --git a/src/iceberg/catalog/rest/http_client.cc
b/src/iceberg/catalog/rest/http_client.cc
index 1e4fc1ee..3e70b9d9 100644
--- a/src/iceberg/catalog/rest/http_client.cc
+++ b/src/iceberg/catalog/rest/http_client.cc
@@ -139,7 +139,7 @@ Result<HttpResponse> HttpClient::Get(
const ErrorHandler& error_handler) {
cpr::Response response;
{
- std::scoped_lock<std::mutex> lock(session_mutex_);
+ std::lock_guard guard(session_mutex_);
PrepareSession(path, headers, params);
response = session_->Get();
}
@@ -156,7 +156,7 @@ Result<HttpResponse> HttpClient::Post(
const ErrorHandler& error_handler) {
cpr::Response response;
{
- std::scoped_lock<std::mutex> lock(session_mutex_);
+ std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
session_->SetBody(cpr::Body{body});
response = session_->Post();
@@ -176,7 +176,7 @@ Result<HttpResponse> HttpClient::PostForm(
cpr::Response response;
{
- std::scoped_lock<std::mutex> lock(session_mutex_);
+ std::lock_guard guard(session_mutex_);
// Override default Content-Type (application/json) with form-urlencoded
auto form_headers = headers;
@@ -204,7 +204,7 @@ Result<HttpResponse> HttpClient::Head(
const ErrorHandler& error_handler) {
cpr::Response response;
{
- std::scoped_lock<std::mutex> lock(session_mutex_);
+ std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
response = session_->Head();
}
@@ -220,7 +220,7 @@ Result<HttpResponse> HttpClient::Delete(
const ErrorHandler& error_handler) {
cpr::Response response;
{
- std::scoped_lock<std::mutex> lock(session_mutex_);
+ std::lock_guard guard(session_mutex_);
PrepareSession(path, headers);
response = session_->Delete();
}
diff --git a/src/iceberg/table_metadata.h b/src/iceberg/table_metadata.h
index 2d53fcb0..c00bd5e8 100644
--- a/src/iceberg/table_metadata.h
+++ b/src/iceberg/table_metadata.h
@@ -419,7 +419,7 @@ class ICEBERG_EXPORT TableMetadataBuilder : public
ErrorCollector {
Result<std::unique_ptr<TableMetadata>> Build();
/// \brief Destructor
- ~TableMetadataBuilder();
+ ~TableMetadataBuilder() override;
// Delete copy operations (use BuildFrom to create a new builder)
TableMetadataBuilder(const TableMetadataBuilder&) = delete;
