Skip to content

Commit 974ae7c

Browse files
Copilotbbockelm
andcommitted
Fix race condition in atomic operations for total_time_s
Co-authored-by: bbockelm <1093447+bbockelm@users.noreply.github.com>
1 parent ed86d9a commit 974ae7c

File tree

2 files changed

+7
-21
lines changed

2 files changed

+7
-21
lines changed

src/scitokens_internal.h

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,6 @@ class MonitoringStats {
133133
public:
134134
static MonitoringStats &instance();
135135

136-
// Get a reference to issuer stats for periodic updates
137-
IssuerStats* get_issuer_stats(const std::string &issuer);
138-
139136
void record_validation_success(const std::string &issuer,
140137
double duration_s);
141138
void record_validation_failure(const std::string &issuer,
@@ -486,7 +483,6 @@ class Validator {
486483
void verify(const SciToken &scitoken, time_t expiry_time) {
487484
std::string issuer = "";
488485
auto start_time = std::chrono::steady_clock::now();
489-
internal::IssuerStats* stats_ptr = nullptr;
490486

491487
try {
492488
auto result = verify_async(scitoken);
@@ -496,12 +492,11 @@ class Validator {
496492
scitoken.m_decoded.get();
497493
if (jwt_decoded && jwt_decoded->has_payload_claim("iss")) {
498494
issuer = jwt_decoded->get_issuer();
499-
stats_ptr = internal::MonitoringStats::instance().get_issuer_stats(issuer);
500495
}
501496

502497
while (!result->m_done) {
503498
auto timeout_val = result->get_timeout_val(expiry_time);
504-
// Limit select to 50ms for periodic updates
499+
// Limit select to 50ms for periodic checks
505500
if (timeout_val.tv_sec > 0 || timeout_val.tv_usec > 50000) {
506501
timeout_val.tv_sec = 0;
507502
timeout_val.tv_usec = 50000;
@@ -510,14 +505,6 @@ class Validator {
510505
select(result->get_max_fd() + 1, result->get_read_fd_set(),
511506
result->get_write_fd_set(), result->get_exc_fd_set(),
512507
&timeout_val);
513-
514-
// Update elapsed time periodically
515-
if (stats_ptr) {
516-
auto current_time = std::chrono::steady_clock::now();
517-
auto duration = std::chrono::duration_cast<std::chrono::duration<double>>(
518-
current_time - start_time);
519-
stats_ptr->total_time_s = duration.count();
520-
}
521508

522509
if (time(NULL) >= expiry_time) {
523510
throw CurlException("Timeout when loading the OIDC metadata.");

src/scitokens_monitoring.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,24 @@ MonitoringStats &MonitoringStats::instance() {
1616
return instance;
1717
}
1818

19-
IssuerStats* MonitoringStats::get_issuer_stats(const std::string &issuer) {
20-
std::lock_guard<std::mutex> lock(m_mutex);
21-
return &m_issuer_stats[issuer];
22-
}
23-
2419
void MonitoringStats::record_validation_success(const std::string &issuer,
2520
double duration_s) {
2621
std::lock_guard<std::mutex> lock(m_mutex);
2722
auto &stats = m_issuer_stats[issuer];
2823
stats.successful_validations++;
29-
stats.total_time_s = stats.total_time_s.load() + duration_s;
24+
// Add to the total time (accumulate across all validations)
25+
double current = stats.total_time_s.load();
26+
stats.total_time_s.store(current + duration_s);
3027
}
3128

3229
void MonitoringStats::record_validation_failure(const std::string &issuer,
3330
double duration_s) {
3431
std::lock_guard<std::mutex> lock(m_mutex);
3532
auto &stats = m_issuer_stats[issuer];
3633
stats.unsuccessful_validations++;
37-
stats.total_time_s = stats.total_time_s.load() + duration_s;
34+
// Add to the total time (accumulate across all validations)
35+
double current = stats.total_time_s.load();
36+
stats.total_time_s.store(current + duration_s);
3837
}
3938

4039
void MonitoringStats::record_expired_token(const std::string &issuer) {

0 commit comments

Comments
 (0)