Incompatible with JNI on Windows: use unique handle for SymInitialize #…

…204 (#206) WinDbg: Duplicate the process handle before using it in SymInitialize, because other libraries may have already called SymInitialize for the process (https://learn.microsoft.com/en-us/windows/win32/debug/initializing-the-symbol-handler) Fixes #204 --------- Co-authored-by: Jeremy <[email protected]>
jeremy-rifkin · Jan 26, 2025 · 1bcfcff · 1bcfcff
1 parent 485d9a6
commit 1bcfcff
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 22 deletions.
diff --git a/src/platform/dbghelp_syminit_manager.cpp b/src/platform/dbghelp_syminit_manager.cpp
@@ -7,7 +7,7 @@
 #include "utils/error.hpp"
 #include "utils/microfmt.hpp"
 
-#include <unordered_set>
+#include <unordered_map>
 
 #ifndef WIN32_LEAN_AND_MEAN
  #define WIN32_LEAN_AND_MEAN
@@ -19,20 +19,32 @@ namespace cpptrace {
 namespace detail {
 
     dbghelp_syminit_manager::~dbghelp_syminit_manager() {
-        for(auto handle : set) {
-            if(!SymCleanup(handle)) {
+        for(auto kvp : cache) {
+            if(!SymCleanup(kvp.second)) {
                 ASSERT(false, microfmt::format("Cpptrace SymCleanup failed with code {}\n", GetLastError()).c_str());
             }
+            if(!CloseHandle(kvp.second)) {
+                ASSERT(false, microfmt::format("Cpptrace CloseHandle failed with code {}\n", GetLastError()).c_str());
+            }
         }
     }
 
-    void dbghelp_syminit_manager::init(HANDLE proc) {
-        if(set.count(proc) == 0) {
-            if(!SymInitialize(proc, NULL, TRUE)) {
-                throw internal_error("SymInitialize failed {}", GetLastError());
-            }
-            set.insert(proc);
+    HANDLE dbghelp_syminit_manager::init(HANDLE proc) {
+        auto itr = cache.find(proc);
+
+        if(itr != cache.end()) {
+            return itr->second;
+        }
+        HANDLE duplicated_handle = nullptr;
+        if(!DuplicateHandle(proc, proc, proc, &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+            throw internal_error("DuplicateHandle failed {}", GetLastError());
+        }
+
+        if(!SymInitialize(duplicated_handle, NULL, TRUE)) {
+            throw internal_error("SymInitialize failed {}", GetLastError());
         }
+        cache[proc] = duplicated_handle;
+        return duplicated_handle;
     }
 
     // Thread-safety: Must only be called from symbols_with_dbghelp while the dbghelp_lock lock is held

diff --git a/src/platform/dbghelp_syminit_manager.hpp b/src/platform/dbghelp_syminit_manager.hpp
@@ -1,17 +1,17 @@
 #ifndef DBGHELP_SYMINIT_MANAGER_HPP
 #define DBGHELP_SYMINIT_MANAGER_HPP
 
-#include <unordered_set>
+#include <unordered_map>
 
 namespace cpptrace {
 namespace detail {
     struct dbghelp_syminit_manager {
         // The set below contains Windows `HANDLE` objects, `void*` is used to avoid 
         // including the (expensive) Windows header here
-        std::unordered_set<void*> set;
+        std::unordered_map<void*, void*> cache;
 
         ~dbghelp_syminit_manager();
-        void init(void* proc);
+        void* init(void* proc);
     };
 
     // Thread-safety: Must only be called from symbols_with_dbghelp while the dbghelp_lock lock is held

diff --git a/src/symbols/symbols_with_dbghelp.cpp b/src/symbols/symbols_with_dbghelp.cpp
@@ -426,17 +426,22 @@ namespace dbghelp {
 
         // TODO: When does this need to be called? Can it be moved to the symbolizer?
         SymSetOptions(SYMOPT_ALLOW_ABSOLUTE_SYMBOLS);
+
+        HANDLE duplicated_handle  = nullptr;
         HANDLE proc = GetCurrentProcess();
         if(get_cache_mode() == cache_mode::prioritize_speed) {
-            get_syminit_manager().init(proc);
+            duplicated_handle = get_syminit_manager().init(proc);
         } else {
-            if(!SymInitialize(proc, NULL, TRUE)) {
-                throw internal_error("SymInitialize failed");
+            if(!DuplicateHandle(proc, proc, proc, &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+                throw internal_error("DuplicateHandle failed {}", GetLastError());
+            }
+            if(!SymInitialize(duplicated_handle, NULL, TRUE)) {
+                throw internal_error("SymInitialize failed {}", GetLastError());
             }
         }
         for(const auto frame : frames) {
             try {
-                trace.push_back(resolve_frame(proc, frame));
+                trace.push_back(resolve_frame(duplicated_handle , frame));
             } catch(...) { // NOSONAR
                 if(!detail::should_absorb_trace_exceptions()) {
                     throw;
@@ -447,9 +452,12 @@ namespace dbghelp {
             }
         }
         if(get_cache_mode() != cache_mode::prioritize_speed) {
-            if(!SymCleanup(proc)) {
+            if(!SymCleanup(duplicated_handle)) {
                 throw internal_error("SymCleanup failed");
             }
+            if(!CloseHandle(duplicated_handle)) {
+                throw internal_error("CloseHandle failed");
+            }
         }
         return trace;
     }

diff --git a/src/unwind/unwind_with_dbghelp.cpp b/src/unwind/unwind_with_dbghelp.cpp
@@ -103,20 +103,24 @@ namespace detail {
         // SymInitialize( GetCurrentProcess(), NULL, TRUE ) has
         // already been called.
         //
+        HANDLE duplicated_handle = nullptr;
         HANDLE proc = GetCurrentProcess();
         HANDLE thread = GetCurrentThread();
         if(get_cache_mode() == cache_mode::prioritize_speed) {
-            get_syminit_manager().init(proc);
+            duplicated_handle = get_syminit_manager().init(proc);
         } else {
-            if(!SymInitialize(proc, NULL, TRUE)) {
-                throw internal_error("SymInitialize failed");
+            if(!DuplicateHandle(proc, proc, proc, &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+                throw internal_error("DuplicateHandle failed{}", GetLastError());
+            }
+            if(!SymInitialize(duplicated_handle, NULL, TRUE)) {
+                throw internal_error("SymInitialize failed{}", GetLastError());
             }
         }
         while(trace.size() < max_depth) {
             if(
                 !StackWalk64(
                     machine_type,
-                    proc,
+                    duplicated_handle,
                     thread,
                     &frame,
                     machine_type == IMAGE_FILE_MACHINE_I386 ? NULL : &context,
@@ -145,9 +149,12 @@ namespace detail {
             }
         }
         if(get_cache_mode() != cache_mode::prioritize_speed) {
-            if(!SymCleanup(proc)) {
+            if(!SymCleanup(duplicated_handle)) {
                 throw internal_error("SymCleanup failed");
             }
+            if(!CloseHandle(duplicated_handle)) {
+                throw internal_error("CloseHandle failed");
+            }
         }
         return trace;
     }