Skip to content

Commit 87b727b

Browse files
Copilotbbockelm
andcommitted
Replace shared_ptr with mutex-protected strings for thread safety
- Replace std::shared_ptr<std::string> with mutex-protected std::string - Add atomic flags for fast-path checking to avoid lock contention - Use relaxed memory ordering for atomic flags as suggested - Ensures thread-safe access to cache_home and tls_ca_file configuration - All formatting done with clang-format per project standards Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
1 parent dcdb1be commit 87b727b

File tree

2 files changed

+48
-11
lines changed

2 files changed

+48
-11
lines changed

src/scitokens_internal.cpp

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,7 +1131,9 @@ configurer::Configuration::set_cache_home(const std::string dir_path) {
11311131
// If setting to "", then we should treat as though it is unsetting the
11321132
// config
11331133
if (dir_path.length() == 0) { // User is configuring to empty string
1134-
get_cache_home_ref() = std::make_shared<std::string>(dir_path);
1134+
std::lock_guard<std::mutex> lock(get_cache_home_mutex());
1135+
get_cache_home_string() = dir_path;
1136+
get_cache_home_set().store(false, std::memory_order_relaxed);
11351137
return std::make_pair(true, "");
11361138
}
11371139

@@ -1154,20 +1156,38 @@ configurer::Configuration::set_cache_home(const std::string dir_path) {
11541156

11551157
// Now it exists and we can write to it, set the value and let
11561158
// scitokens_cache handle the rest
1157-
get_cache_home_ref() = std::make_shared<std::string>(cleaned_dir_path);
1159+
{
1160+
std::lock_guard<std::mutex> lock(get_cache_home_mutex());
1161+
get_cache_home_string() = cleaned_dir_path;
1162+
get_cache_home_set().store(true, std::memory_order_relaxed);
1163+
}
11581164
return std::make_pair(true, "");
11591165
}
11601166

11611167
void configurer::Configuration::set_tls_ca_file(const std::string ca_file) {
1162-
get_tls_ca_file_ref() = std::make_shared<std::string>(ca_file);
1168+
std::lock_guard<std::mutex> lock(get_tls_ca_file_mutex());
1169+
get_tls_ca_file_string() = ca_file;
1170+
get_tls_ca_file_set().store(!ca_file.empty(), std::memory_order_relaxed);
11631171
}
11641172

11651173
std::string configurer::Configuration::get_cache_home() {
1166-
return *get_cache_home_ref();
1174+
// Fast path: check if the value has been set
1175+
if (!get_cache_home_set().load(std::memory_order_relaxed)) {
1176+
return "";
1177+
}
1178+
// Slow path: acquire lock and read the value
1179+
std::lock_guard<std::mutex> lock(get_cache_home_mutex());
1180+
return get_cache_home_string();
11671181
}
11681182

11691183
std::string configurer::Configuration::get_tls_ca_file() {
1170-
return *get_tls_ca_file_ref();
1184+
// Fast path: check if the value has been set
1185+
if (!get_tls_ca_file_set().load(std::memory_order_relaxed)) {
1186+
return "";
1187+
}
1188+
// Slow path: acquire lock and read the value
1189+
std::lock_guard<std::mutex> lock(get_tls_ca_file_mutex());
1190+
return get_tls_ca_file_string();
11711191
}
11721192

11731193
// bool configurer::Configuration::check_dir(const std::string dir_path) {

src/scitokens_internal.h

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,31 @@ class Configuration {
5959
static std::atomic_int instance{4 * 24 * 3600};
6060
return instance;
6161
}
62-
static std::shared_ptr<std::string> &get_cache_home_ref() {
63-
static std::shared_ptr<std::string> instance =
64-
std::make_shared<std::string>("");
62+
63+
// Thread-safe accessors for string configurations
64+
static std::mutex &get_cache_home_mutex() {
65+
static std::mutex instance;
66+
return instance;
67+
}
68+
static std::string &get_cache_home_string() {
69+
static std::string instance;
70+
return instance;
71+
}
72+
static std::atomic<bool> &get_cache_home_set() {
73+
static std::atomic<bool> instance{false};
74+
return instance;
75+
}
76+
77+
static std::mutex &get_tls_ca_file_mutex() {
78+
static std::mutex instance;
79+
return instance;
80+
}
81+
static std::string &get_tls_ca_file_string() {
82+
static std::string instance;
6583
return instance;
6684
}
67-
static std::shared_ptr<std::string> &get_tls_ca_file_ref() {
68-
static std::shared_ptr<std::string> instance =
69-
std::make_shared<std::string>("");
85+
static std::atomic<bool> &get_tls_ca_file_set() {
86+
static std::atomic<bool> instance{false};
7087
return instance;
7188
}
7289

0 commit comments

Comments
 (0)