Skip to content

Commit ddcabfc

Browse files
RahulHereRahulHere
RahulHere
authored and
RahulHere
committed
Implement Thread-Safe Error Handling (#130)
- Enhanced thread-local error storage with isolation - Added thread-local error buffer for safe string returns - Made g_initialized an atomic flag for thread safety - Used memory ordering for atomic operations - Added thread-safe error buffer copying in get_last_error - Enhanced error context functions with thread safety - Documented that client error state is per-instance - Ensured error messages don't leak between threads - Protected initialization state with atomic operations - Made all error retrieval functions thread-safe
1 parent 78907c6 commit ddcabfc

File tree

1 file changed

+56
-30
lines changed

1 file changed

+56
-30
lines changed

‎src/c_api/mcp_c_auth_api.cc‎

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,16 @@
2828
#include <openssl/bn.h>
2929
#include <curl/curl.h>
3030

31-
// Thread-local error storage with context
31+
// Thread-local error storage with context (per-thread isolation)
3232
static thread_local std::string g_last_error;
3333
static thread_local std::string g_last_error_context; // Additional context information
3434
static thread_local mcp_auth_error_t g_last_error_code = MCP_AUTH_SUCCESS;
3535

36-
// Global initialization state
37-
static bool g_initialized = false;
36+
// Thread-local error buffer for safe string returns
37+
static thread_local char g_error_buffer[4096];
38+
39+
// Global initialization state with atomic flag
40+
static std::atomic<bool> g_initialized{false};
3841
static std::mutex g_init_mutex;
3942

4043
// Set error state
@@ -1031,13 +1034,16 @@ struct mcp_auth_validation_options {
10311034
}
10321035
};
10331036

1034-
// Store error in client structure with context (moved after struct definition)
1037+
// Store error in client structure with context (thread-safe)
10351038
static void set_client_error(mcp_auth_client_t client, mcp_auth_error_t code,
10361039
const std::string& message, const std::string& context = "") {
10371040
if (client) {
1041+
// Client error is not thread-local, so we just store it
1042+
// Each client instance maintains its own error state
10381043
client->last_error_code = code;
10391044
client->last_error_context = context.empty() ? message : message + " (" + context + ")";
10401045
}
1046+
// Also set thread-local error for immediate retrieval
10411047
set_error_with_context(code, message, context);
10421048
}
10431049

@@ -1359,11 +1365,13 @@ extern "C" {
13591365

13601366
mcp_auth_error_t mcp_auth_init(void) {
13611367
std::lock_guard<std::mutex> lock(g_init_mutex);
1362-
if (g_initialized) {
1368+
1369+
// Check atomic flag with memory ordering
1370+
if (g_initialized.load(std::memory_order_acquire)) {
13631371
return MCP_AUTH_SUCCESS;
13641372
}
13651373

1366-
// Initialize libcurl globally
1374+
// Initialize libcurl globally (thread-safe initialization)
13671375
CURLcode res = curl_global_init(CURL_GLOBAL_ALL);
13681376
if (res != CURLE_OK) {
13691377
set_error(MCP_AUTH_ERROR_INTERNAL_ERROR,
@@ -1372,13 +1380,15 @@ mcp_auth_error_t mcp_auth_init(void) {
13721380
}
13731381

13741382
clear_error();
1375-
g_initialized = true;
1383+
1384+
// Set atomic flag with memory ordering
1385+
g_initialized.store(true, std::memory_order_release);
13761386
return MCP_AUTH_SUCCESS;
13771387
}
13781388

13791389
mcp_auth_error_t mcp_auth_shutdown(void) {
13801390
std::lock_guard<std::mutex> lock(g_init_mutex);
1381-
if (!g_initialized) {
1391+
if (!g_initialized.load()) {
13821392
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
13831393
return MCP_AUTH_ERROR_NOT_INITIALIZED;
13841394
}
@@ -1389,7 +1399,8 @@ mcp_auth_error_t mcp_auth_shutdown(void) {
13891399
// Clear any cached errors
13901400
clear_error();
13911401

1392-
g_initialized = false;
1402+
// Clear atomic flag with memory ordering
1403+
g_initialized.store(false, std::memory_order_release);
13931404
return MCP_AUTH_SUCCESS;
13941405
}
13951406

@@ -1406,7 +1417,7 @@ mcp_auth_error_t mcp_auth_client_create(
14061417
const char* jwks_uri,
14071418
const char* issuer) {
14081419

1409-
if (!g_initialized) {
1420+
if (!g_initialized.load()) {
14101421
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
14111422
return MCP_AUTH_ERROR_NOT_INITIALIZED;
14121423
}
@@ -1449,7 +1460,7 @@ mcp_auth_error_t mcp_auth_client_create(
14491460
}
14501461

14511462
mcp_auth_error_t mcp_auth_client_destroy(mcp_auth_client_t client) {
1452-
if (!g_initialized) {
1463+
if (!g_initialized.load()) {
14531464
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
14541465
return MCP_AUTH_ERROR_NOT_INITIALIZED;
14551466
}
@@ -1501,7 +1512,7 @@ mcp_auth_error_t mcp_auth_client_set_option(
15011512
const char* option,
15021513
const char* value) {
15031514

1504-
if (!g_initialized) {
1515+
if (!g_initialized.load()) {
15051516
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
15061517
return MCP_AUTH_ERROR_NOT_INITIALIZED;
15071518
}
@@ -1565,7 +1576,7 @@ mcp_auth_error_t mcp_auth_client_set_option(
15651576
mcp_auth_error_t mcp_auth_validation_options_create(
15661577
mcp_auth_validation_options_t* options) {
15671578

1568-
if (!g_initialized) {
1579+
if (!g_initialized.load()) {
15691580
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
15701581
return MCP_AUTH_ERROR_NOT_INITIALIZED;
15711582
}
@@ -1593,7 +1604,7 @@ mcp_auth_error_t mcp_auth_validation_options_create(
15931604
mcp_auth_error_t mcp_auth_validation_options_destroy(
15941605
mcp_auth_validation_options_t options) {
15951606

1596-
if (!g_initialized) {
1607+
if (!g_initialized.load()) {
15971608
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
15981609
return MCP_AUTH_ERROR_NOT_INITIALIZED;
15991610
}
@@ -1621,7 +1632,7 @@ mcp_auth_error_t mcp_auth_validation_options_set_scopes(
16211632
mcp_auth_validation_options_t options,
16221633
const char* scopes) {
16231634

1624-
if (!g_initialized) {
1635+
if (!g_initialized.load()) {
16251636
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
16261637
return MCP_AUTH_ERROR_NOT_INITIALIZED;
16271638
}
@@ -1655,7 +1666,7 @@ mcp_auth_error_t mcp_auth_validation_options_set_audience(
16551666
mcp_auth_validation_options_t options,
16561667
const char* audience) {
16571668

1658-
if (!g_initialized) {
1669+
if (!g_initialized.load()) {
16591670
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
16601671
return MCP_AUTH_ERROR_NOT_INITIALIZED;
16611672
}
@@ -1692,7 +1703,7 @@ mcp_auth_error_t mcp_auth_validation_options_set_clock_skew(
16921703
mcp_auth_validation_options_t options,
16931704
int64_t seconds) {
16941705

1695-
if (!g_initialized) {
1706+
if (!g_initialized.load()) {
16961707
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
16971708
return MCP_AUTH_ERROR_NOT_INITIALIZED;
16981709
}
@@ -1731,7 +1742,7 @@ mcp_auth_error_t mcp_auth_validate_token(
17311742
mcp_auth_validation_options_t options,
17321743
mcp_auth_validation_result_t* result) {
17331744

1734-
if (!g_initialized) {
1745+
if (!g_initialized.load()) {
17351746
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
17361747
return MCP_AUTH_ERROR_NOT_INITIALIZED;
17371748
}
@@ -1943,7 +1954,7 @@ mcp_auth_error_t mcp_auth_extract_payload(
19431954
const char* token,
19441955
mcp_auth_token_payload_t* payload) {
19451956

1946-
if (!g_initialized) {
1957+
if (!g_initialized.load()) {
19471958
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
19481959
return MCP_AUTH_ERROR_NOT_INITIALIZED;
19491960
}
@@ -1978,7 +1989,7 @@ mcp_auth_error_t mcp_auth_payload_get_subject(
19781989
mcp_auth_token_payload_t payload,
19791990
char** value) {
19801991

1981-
if (!g_initialized) {
1992+
if (!g_initialized.load()) {
19821993
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
19831994
return MCP_AUTH_ERROR_NOT_INITIALIZED;
19841995
}
@@ -1997,7 +2008,7 @@ mcp_auth_error_t mcp_auth_payload_get_issuer(
19972008
mcp_auth_token_payload_t payload,
19982009
char** value) {
19992010

2000-
if (!g_initialized) {
2011+
if (!g_initialized.load()) {
20012012
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
20022013
return MCP_AUTH_ERROR_NOT_INITIALIZED;
20032014
}
@@ -2016,7 +2027,7 @@ mcp_auth_error_t mcp_auth_payload_get_audience(
20162027
mcp_auth_token_payload_t payload,
20172028
char** value) {
20182029

2019-
if (!g_initialized) {
2030+
if (!g_initialized.load()) {
20202031
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
20212032
return MCP_AUTH_ERROR_NOT_INITIALIZED;
20222033
}
@@ -2035,7 +2046,7 @@ mcp_auth_error_t mcp_auth_payload_get_scopes(
20352046
mcp_auth_token_payload_t payload,
20362047
char** value) {
20372048

2038-
if (!g_initialized) {
2049+
if (!g_initialized.load()) {
20392050
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
20402051
return MCP_AUTH_ERROR_NOT_INITIALIZED;
20412052
}
@@ -2054,7 +2065,7 @@ mcp_auth_error_t mcp_auth_payload_get_expiration(
20542065
mcp_auth_token_payload_t payload,
20552066
int64_t* value) {
20562067

2057-
if (!g_initialized) {
2068+
if (!g_initialized.load()) {
20582069
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
20592070
return MCP_AUTH_ERROR_NOT_INITIALIZED;
20602071
}
@@ -2074,7 +2085,7 @@ mcp_auth_error_t mcp_auth_payload_get_claim(
20742085
const char* claim_name,
20752086
char** value) {
20762087

2077-
if (!g_initialized) {
2088+
if (!g_initialized.load()) {
20782089
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
20792090
return MCP_AUTH_ERROR_NOT_INITIALIZED;
20802091
}
@@ -2097,7 +2108,7 @@ mcp_auth_error_t mcp_auth_payload_get_claim(
20972108
}
20982109

20992110
mcp_auth_error_t mcp_auth_payload_destroy(mcp_auth_token_payload_t payload) {
2100-
if (!g_initialized) {
2111+
if (!g_initialized.load()) {
21012112
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
21022113
return MCP_AUTH_ERROR_NOT_INITIALIZED;
21032114
}
@@ -2138,7 +2149,7 @@ mcp_auth_error_t mcp_auth_generate_www_authenticate(
21382149
const char* error_description,
21392150
char** header) {
21402151

2141-
if (!g_initialized) {
2152+
if (!g_initialized.load()) {
21422153
set_error(MCP_AUTH_ERROR_NOT_INITIALIZED, "Library not initialized");
21432154
return MCP_AUTH_ERROR_NOT_INITIALIZED;
21442155
}
@@ -2188,25 +2199,40 @@ void mcp_auth_free_string(char* str) {
21882199
}
21892200

21902201
const char* mcp_auth_get_last_error(void) {
2191-
// Return empty string instead of nullptr for safety
2192-
return g_last_error.empty() ? "" : g_last_error.c_str();
2202+
// Thread-safe: Each thread has its own error state
2203+
// Copy to thread-local buffer for safe return
2204+
if (g_last_error.empty()) {
2205+
return "";
2206+
}
2207+
2208+
// Copy error to thread-local buffer to ensure string lifetime
2209+
size_t len = g_last_error.length();
2210+
if (len >= sizeof(g_error_buffer)) {
2211+
len = sizeof(g_error_buffer) - 1;
2212+
}
2213+
memcpy(g_error_buffer, g_last_error.c_str(), len);
2214+
g_error_buffer[len] = '\0';
2215+
2216+
return g_error_buffer;
21932217
}
21942218

21952219
mcp_auth_error_t mcp_auth_get_last_error_code(void) {
21962220
return g_last_error_code;
21972221
}
21982222

2199-
// Get last error with full context information
2223+
// Get last error with full context information (thread-safe)
22002224
mcp_auth_error_t mcp_auth_get_last_error_full(char** error_message) {
22012225
if (!error_message) {
22022226
return MCP_AUTH_ERROR_INVALID_PARAMETER;
22032227
}
22042228

2229+
// Thread-safe: Build message from thread-local storage
22052230
std::string full_message = g_last_error;
22062231
if (!g_last_error_context.empty()) {
22072232
full_message += " [Context: " + g_last_error_context + "]";
22082233
}
22092234

2235+
// Allocate new string for caller
22102236
*error_message = safe_strdup(full_message);
22112237
return g_last_error_code;
22122238
}

0 commit comments

Comments
 (0)