Improve locking surrounding dbghelp and lock in the dbghelp demangler

CMakeLists.txt

@@ -138,7 +138,7 @@ target_sources(
     src/unwind/unwind_with_winapi.cpp
     src/utils/microfmt.cpp
     src/utils/utils.cpp
-    src/platform/dbghelp_syminit_manager.cpp
+    src/platform/dbghelp_utils.cpp
 )
 
 target_include_directories(

src/demangle/demangle_with_winapi.cpp

@@ -1,6 +1,7 @@
 #ifdef CPPTRACE_DEMANGLE_WITH_WINAPI
 
 #include "demangle/demangle.hpp"
+#include "platform/dbghelp_utils.hpp"
 
 #include <string>
 
@@ -13,6 +14,8 @@
 namespace cpptrace {
 namespace detail {
     std::string demangle(const std::string& name, bool) {
+        // Dbghelp is is single-threaded, so acquire a lock.
+        auto lock = get_dbghelp_lock();
         char buffer[500];
         auto ret = UnDecorateSymbolName(name.c_str(), buffer, sizeof(buffer) - 1, 0);
         if(ret == 0) {

src/platform/dbghelp_syminit_manager.cpp → src/platform/dbghelp_utils.cpp

@@ -2,7 +2,11 @@
 
 #if IS_WINDOWS
 
-#include "platform/dbghelp_syminit_manager.hpp"
+#include "platform/dbghelp_utils.hpp"
+
+#if defined(CPPTRACE_UNWIND_WITH_DBGHELP) \
+    || defined(CPPTRACE_GET_SYMBOLS_WITH_DBGHELP) \
+    || defined(CPPTRACE_DEMANGLE_WITH_WINAPI)
 
 #include "utils/error.hpp"
 #include "utils/microfmt.hpp"
@@ -53,7 +57,15 @@ namespace detail {
         return syminit_manager;
     }
 
+    std::recursive_mutex dbghelp_lock;
+
+    std::unique_lock<std::recursive_mutex> get_dbghelp_lock() {
+        return std::unique_lock<std::recursive_mutex>{dbghelp_lock};
+    }
+
 }
 }
 
 #endif
+
+#endif

src/platform/dbghelp_syminit_manager.hpp → src/platform/dbghelp_utils.hpp

@@ -1,22 +1,30 @@
-#ifndef DBGHELP_SYMINIT_MANAGER_HPP
-#define DBGHELP_SYMINIT_MANAGER_HPP
+#ifndef DBGHELP_UTILS_HPP
+#define DBGHELP_UTILS_HPP
+
+#if defined(CPPTRACE_UNWIND_WITH_DBGHELP) \
+    || defined(CPPTRACE_GET_SYMBOLS_WITH_DBGHELP) \
+    || defined(CPPTRACE_DEMANGLE_WITH_WINAPI)
 
 #include <unordered_map>
+#include <mutex>
 
 namespace cpptrace {
 namespace detail {
     struct dbghelp_syminit_manager {
-        // The set below contains Windows `HANDLE` objects, `void*` is used to avoid 
+        // The set below contains Windows `HANDLE` objects, `void*` is used to avoid
         // including the (expensive) Windows header here
         std::unordered_map<void*, void*> cache;
 
         ~dbghelp_syminit_manager();
         void* init(void* proc);
     };
 
-    // Thread-safety: Must only be called from symbols_with_dbghelp while the dbghelp_lock lock is held
     dbghelp_syminit_manager& get_syminit_manager();
+
+    std::unique_lock<std::recursive_mutex> get_dbghelp_lock();
 }
 }
 
 #endif
+
+#endif

src/symbols/symbols_with_dbghelp.cpp

@@ -2,13 +2,12 @@
 
 #include <cpptrace/basic.hpp>
 #include "symbols/symbols.hpp"
-#include "platform/dbghelp_syminit_manager.hpp"
+#include "platform/dbghelp_utils.hpp"
 #include "binary/object.hpp"
 #include "utils/common.hpp"
 #include "utils/error.hpp"
 #include "options.hpp"
 
-#include <mutex>
 #include <regex>
 #include <system_error>
 #include <vector>
@@ -325,8 +324,6 @@ namespace dbghelp {
         return true;
     }
 
-    std::recursive_mutex dbghelp_lock;
-
     // TODO: Handle backtrace_pcinfo calling the callback multiple times on inlined functions
     stacktrace_frame resolve_frame(HANDLE proc, frame_ptr addr) {
         // The get_frame_object_info() ends up being inexpensive, at on my machine
@@ -336,7 +333,8 @@ namespace dbghelp {
         // get_frame_object_info()                 0.001-0.002 ms  0.0003-0.0006 ms
         // At some point it might make sense to make an option to control this.
         auto object_frame = get_frame_object_info(addr);
-        const std::lock_guard<std::recursive_mutex> lock(dbghelp_lock); // all dbghelp functions are not thread safe
+        // Dbghelp is is single-threaded, so acquire a lock.
+        auto lock = get_dbghelp_lock();
         alignas(SYMBOL_INFO) char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)];
         SYMBOL_INFO* symbol = (SYMBOL_INFO*)buffer;
         symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
@@ -421,7 +419,8 @@ namespace dbghelp {
     }
 
     std::vector<stacktrace_frame> resolve_frames(const std::vector<frame_ptr>& frames) {
-        const std::lock_guard<std::recursive_mutex> lock(dbghelp_lock); // all dbghelp functions are not thread safe
+        // Dbghelp is is single-threaded, so acquire a lock.
+        auto lock = get_dbghelp_lock();
         std::vector<stacktrace_frame> trace;
         trace.reserve(frames.size());
 

src/unwind/unwind_with_dbghelp.cpp

@@ -4,10 +4,9 @@
 #include "unwind/unwind.hpp"
 #include "utils/common.hpp"
 #include "utils/utils.hpp"
-#include "platform/dbghelp_syminit_manager.hpp"
+#include "platform/dbghelp_utils.hpp"
 
 #include <vector>
-#include <mutex>
 #include <cstddef>
 
 #include <windows.h>
@@ -96,8 +95,7 @@ namespace detail {
         std::vector<frame_ptr> trace;
 
         // Dbghelp is is single-threaded, so acquire a lock.
-        static std::mutex mutex;
-        std::lock_guard<std::mutex> lock(mutex);
+        auto lock = get_dbghelp_lock();
         // For some reason SymInitialize must be called before StackWalk64
         // Note that the code assumes that
         // SymInitialize( GetCurrentProcess(), NULL, TRUE ) has