Thread safety

src/demangle/demangle_with_cxxabi.cpp

@@ -11,6 +11,7 @@ namespace cpptrace {
     namespace detail {
         std::string demangle(const std::string& name) {
             int status;
+            // presumably thread-safe
             char* demangled = abi::__cxa_demangle(name.c_str(), nullptr, nullptr, &status);
             if(demangled) {
                 std::string str = demangled;

src/full/full_trace_with_libbacktrace.cpp

@@ -7,6 +7,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <cstdio>
+#include <mutex>
 #include <vector>
 
 #ifdef CPPTRACE_BACKTRACE_PATH
@@ -52,7 +53,10 @@ namespace cpptrace {
             fprintf(stderr, "Libbacktrace error: %s, code %d\n", msg, errnum);
         }
 
+        std::mutex state_mutex;
+
         backtrace_state* get_backtrace_state() {
+            const std::lock_guard<std::mutex> lock(state_mutex);
             // backtrace_create_state must be called only one time per program
             // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
             static backtrace_state* state = nullptr;

src/platform/program_name.hpp

@@ -1,22 +1,38 @@
 #ifndef PROGRAM_NAME_HPP
 #define PROGRAM_NAME_HPP
 
+#include <mutex>
 #include <string>
 
+namespace cpptrace {
+    namespace detail {
+        inline std::mutex& get_program_name_mutex() { // stupid workaround for an inline variable
+            static std::mutex mutex;
+            return mutex;
+        }
+    }
+}
+
 #if defined(_WIN32)
 #include <windows.h>
 
 namespace cpptrace {
     namespace detail {
         inline std::string program_name() {
-            // TODO: Cache this better
+            const std::lock_guard<std::mutex> lock(get_program_name_mutex());
-            char buffer[MAX_PATH + 1];
+            static std::string name;
-            int res = GetModuleFileNameA(nullptr, buffer, MAX_PATH);
+            static bool did_init = false;
-            if(res) {
+            static bool valid = false;
-                return buffer;
+            if(!did_init) {
-            } else {
+                did_init = true;
-                return "";
+                char buffer[MAX_PATH + 1];
+                int res = GetModuleFileNameA(nullptr, buffer, MAX_PATH);
+                if(res) {
+                    name = buffer;
+                    valid = true;
+                }
             }
+            return valid && !name.empty() ? name.c_str() : nullptr;
         }
     }
 }
@@ -30,16 +46,20 @@ namespace cpptrace {
 namespace cpptrace {
     namespace detail {
         inline const char* program_name() {
-            // TODO: Cache this better
+            const std::lock_guard<std::mutex> lock(get_program_name_mutex());
             static std::string name;
-            if (!name.empty()) {
+            static bool did_init = false;
+            static bool valid = false;
+            if(!did_init) {
+                did_init = true;
                 std::uint32_t bufferSize = PATH_MAX + 1;
                 char buffer[bufferSize];
-                if (_NSGetExecutablePath(buffer, &bufferSize) != 0)
+                if(_NSGetExecutablePath(buffer, &bufferSize) == 0) {
-                    return nullptr;
+                    name.assign(buffer, bufferSize);
-                name.assign(buffer, bufferSize);
+                    valid = true;
+                }
             }
-            return name.c_str();
+            return valid && !name.empty() ? name.c_str() : nullptr;
         }
     }
 }
@@ -53,6 +73,7 @@ namespace cpptrace {
 namespace cpptrace {
     namespace detail {
         inline const char* program_name() {
+            const std::lock_guard<std::mutex> lock(get_program_name_mutex());
             static std::string name;
             static bool did_init = false;
             static bool valid = false;
@@ -67,7 +88,7 @@ namespace cpptrace {
                 name = buffer;
                 valid = true;
             }
-            return valid ? name.c_str() : nullptr;
+            return valid && !name.empty() ? name.c_str() : nullptr;
         }
     }
 }

src/symbols/symbols_with_addr2line.cpp

@@ -44,7 +44,7 @@ namespace cpptrace {
                 Dl_info info;
                 dlframe frame;
                 frame.raw_address = reinterpret_cast<uintptr_t>(addr);
-                if(dladdr(addr, &info)) {
+                if(dladdr(addr, &info)) { // thread safe
                     // dli_sname and dli_saddr are only present with -rdynamic, sname will be included
                     // but we don't really need dli_saddr
                     frame.obj_path = info.dli_fname;
@@ -153,13 +153,15 @@ namespace cpptrace {
                 dlframe frame;
                 frame.raw_address = reinterpret_cast<uintptr_t>(addr);
                 HMODULE handle;
+                // Multithread safe as long as another thread doesn't come along and free the module
                 if(GetModuleHandleExA(
                     GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT | GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
                     static_cast<const char*>(addr),
                     &handle
                 )) {
-                    fflush(stderr);
+                    fflush(stderr); // TODO: remove
                     char path[MAX_PATH];
+                    // TODO: Memoize
                     if(GetModuleFileNameA(handle, path, sizeof(path))) {
                         ///fprintf(stderr, "path: %s base: %p\n", path, handle);
                         frame.obj_path = path;
@@ -177,6 +179,7 @@ namespace cpptrace {
         }
 
         bool has_addr2line() {
+            // TODO: Memoize
             // TODO: Popen is a hack. Implement properly with CreateProcess and pipes later.
             FILE* p = popen("addr2line --version", "r");
             return pclose(p) == 0;

src/symbols/symbols_with_dbghelp.cpp

@@ -299,6 +299,8 @@ namespace cpptrace {
             return true;
         }
 
+        std::mutex dbghelp_lock;
+
         // TODO: Handle backtrace_pcinfo calling the callback multiple times on inlined functions
         struct symbolizer::impl {
             bool good = true;
@@ -320,6 +322,7 @@ namespace cpptrace {
             }
 
             stacktrace_frame resolve_frame(void* addr) {
+                const std::lock_guard<std::mutex> lock(dbghelp_lock); // all dbghelp functions are not thread safe
                 alignas(SYMBOL_INFO) char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)];
                 SYMBOL_INFO* symbol = (SYMBOL_INFO*)buffer;
                 symbol->SizeOfStruct = sizeof(SYMBOL_INFO);

src/symbols/symbols_with_dl.cpp

@@ -15,7 +15,7 @@ namespace cpptrace {
             // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
             stacktrace_frame resolve_frame(const void* addr) {
                 Dl_info info;
-                if(dladdr(addr, &info)) {
+                if(dladdr(addr, &info)) { // thread-safe
                     return {
                         reinterpret_cast<uintptr_t>(addr),
                         0,

src/symbols/symbols_with_libbacktrace.cpp

@@ -7,6 +7,7 @@
 #include <cstdint>
 #include <cstdio>
 #include <memory>
+#include <mutex>
 #include <vector>
 
 #ifdef CPPTRACE_BACKTRACE_PATH
@@ -41,7 +42,10 @@ namespace cpptrace {
             fprintf(stderr, "Libbacktrace error: %s, code %d\n", msg, errnum);
         }
 
+        std::mutex state_mutex;
+
         backtrace_state* get_backtrace_state() {
+            const std::lock_guard<std::mutex> lock(state_mutex);
             // backtrace_create_state must be called only one time per program
             // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
             static backtrace_state* state = nullptr;

src/unwind/unwind_with_execinfo.cpp

@@ -14,7 +14,7 @@ namespace cpptrace {
         CPPTRACE_FORCE_NO_INLINE
         std::vector<void*> capture_frames(size_t skip) {
             std::vector<void*> frames(hard_max_frames + skip, nullptr);
-            const int n_frames = backtrace(frames.data(), int(hard_max_frames + skip));
+            const int n_frames = backtrace(frames.data(), int(hard_max_frames + skip)); // thread safe
             frames.resize(n_frames);
             frames.erase(frames.begin(), frames.begin() + ptrdiff_t(std::min(skip + 1, frames.size())));
             frames.shrink_to_fit();

src/unwind/unwind_with_unwind.cpp

@@ -38,6 +38,7 @@ namespace cpptrace {
             if (ip == uintptr_t(0) || state.count == state.vec.size()) {
                 return _URC_END_OF_STACK;
             } else {
+                // TODO: push_back?...
                 state.vec[state.count++] = (void*)ip;
                 return _URC_NO_REASON;
             }
@@ -47,7 +48,7 @@ namespace cpptrace {
         std::vector<void*> capture_frames(size_t skip) {
             std::vector<void*> frames(hard_max_frames, nullptr);
             unwind_state state{skip + 1, 0, frames};
-            _Unwind_Backtrace(unwind_callback, &state);
+            _Unwind_Backtrace(unwind_callback, &state); // presumably thread-safe
             frames.resize(state.count);
             frames.shrink_to_fit();
             return frames;