From 116a868e41046707133bc6384a88e7fc1f3094f4 Mon Sep 17 00:00:00 2001 From: dsciebu Date: Mon, 13 Jan 2025 12:16:50 +0000 Subject: [PATCH] core/logging: Fix data race on log_prefix In ofi_get_core_info, which is supposed to be thread safe ("Multiple threads may call fi_getinfo simultaneously, without any requirement for serialization."), a global variable 'log_prefix' is modified, which may lead to a data race. Changing the variable to a thread local one, fixes that problem. Signed-off-by: Dariusz Sciebura --- include/ofi.h | 2 +- include/unix/osd.h | 16 ++++++++++++++++ include/windows/osd.h | 17 +++++++++++++++++ src/common.c | 2 +- 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/include/ofi.h b/include/ofi.h index 9661a7553d9..f5ca0bd6042 100644 --- a/include/ofi.h +++ b/include/ofi.h @@ -320,7 +320,7 @@ struct ofi_filter { extern struct ofi_filter prov_log_filter; extern struct fi_provider core_prov; -extern const char *log_prefix; +extern OFI_THREAD_LOCAL const char *log_prefix; void ofi_create_filter(struct ofi_filter *filter, const char *env_name); void ofi_free_filter(struct ofi_filter *filter); diff --git a/include/unix/osd.h b/include/unix/osd.h index 1b59254ee61..6ed3c7bf876 100644 --- a/include/unix/osd.h +++ b/include/unix/osd.h @@ -83,6 +83,22 @@ #define OFI_MAX_SOCKET_BUF_SIZE SIZE_MAX +#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L) + // C23 and above: use thread_local directly + #define OFI_THREAD_LOCAL thread_local +#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201102L) && defined(_Thread_local) + // C11: use _Thread_local + #define OFI_THREAD_LOCAL _Thread_local +#elif defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__SUNPRO_CC) || defined(__IBMCPP__) || defined(__clang__) + // GCC/Clang/Intel/SunPro/IBM compilers + #define OFI_THREAD_LOCAL __thread +#else + // Unsupported compiler + #warning "Thread-local storage is not supported on this platform" + #define OFI_THREAD_LOCAL +#endif + + struct util_shm { int shared_fd; diff --git a/include/windows/osd.h b/include/windows/osd.h index efd3bf0d125..1f185a5c5e3 100644 --- a/include/windows/osd.h +++ b/include/windows/osd.h @@ -1141,6 +1141,23 @@ ofi_cpuid(unsigned func, unsigned subfunc, unsigned cpuinfo[4]) #endif /* defined(_M_X64) || defined(_M_AMD64) */ +#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L) + // C23 and above: use thread_local directly + #define OFI_THREAD_LOCAL thread_local +#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201102L) && defined(_Thread_local) + // C11: use _Thread_local + #define OFI_THREAD_LOCAL _Thread_local +#elif defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__SUNPRO_CC) || defined(__IBMCPP__) || defined(__clang__) + // GCC/Clang/Intel/SunPro/IBM compilers + #define OFI_THREAD_LOCAL __thread +#elif defined(_MSC_VER) + // Microsoft Visual C++ compiler + #define OFI_THREAD_LOCAL __declspec(thread) +#else + // Unsupported compiler + #warning "Thread-local storage is not supported on this platform" + #define OFI_THREAD_LOCAL +#endif #ifdef __cplusplus } diff --git a/src/common.c b/src/common.c index 0d641ac74a1..2f619b191a6 100644 --- a/src/common.c +++ b/src/common.c @@ -2429,4 +2429,4 @@ size_t ofi_vrb_speed(uint8_t speed, uint8_t width) } /* log_prefix is used by fi_log and by prov/util */ -const char *log_prefix = ""; +OFI_THREAD_LOCAL const char *log_prefix = "";