Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Jan 26, 2025
28 changes: 19 additions & 9 deletions src/platform/dbghelp_syminit_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "utils/error.hpp"
#include "utils/microfmt.hpp"

#include <unordered_set>
#include <map>

#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
Expand All @@ -19,20 +19,30 @@ 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());
}
}
}

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;

}
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved
HANDLE duplicatedHandle = nullptr;
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved
if (!DuplicateHandle(proc, proc, proc, &duplicatedHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved
throw internal_error("DuplicateHandle failed");
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved
}

if(!SymInitialize(duplicatedHandle, NULL, TRUE)) {
throw internal_error("SymInitialize failed {}", GetLastError());
}
cache[proc] = duplicatedHandle;
return duplicatedHandle;
}

// Thread-safety: Must only be called from symbols_with_dbghelp while the dbghelp_lock lock is held
Expand Down
6 changes: 3 additions & 3 deletions src/platform/dbghelp_syminit_manager.hpp
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
#ifndef DBGHELP_SYMINIT_MANAGER_HPP
#define DBGHELP_SYMINIT_MANAGER_HPP

#include <unordered_set>
#include <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::map<void*, void*> cache;
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved

~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
Expand Down
12 changes: 8 additions & 4 deletions src/unwind/unwind_with_dbghelp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,24 @@ namespace detail {
// SymInitialize( GetCurrentProcess(), NULL, TRUE ) has
// already been called.
//
HANDLE duplicatedHandle = nullptr;
HANDLE proc = GetCurrentProcess();
HANDLE thread = GetCurrentThread();
if(get_cache_mode() == cache_mode::prioritize_speed) {
get_syminit_manager().init(proc);
duplicatedHandle = get_syminit_manager().init(proc);
jeremy-rifkin marked this conversation as resolved.
Show resolved Hide resolved
} else {
if(!SymInitialize(proc, NULL, TRUE)) {
if (!DuplicateHandle(proc, proc, proc, &duplicatedHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic appears in a couple places and having grown I'm thinking it would be good to refactor out. That's something I can do after merging if you'd like to not delay this pr.

throw internal_error("DuplicateHandle failed");
}
if(!SymInitialize(duplicatedHandle, NULL, TRUE)) {
throw internal_error("SymInitialize failed");
}
}
while(trace.size() < max_depth) {
if(
!StackWalk64(
machine_type,
proc,
duplicatedHandle,
thread,
&frame,
machine_type == IMAGE_FILE_MACHINE_I386 ? NULL : &context,
Expand Down Expand Up @@ -145,7 +149,7 @@ namespace detail {
}
}
if(get_cache_mode() != cache_mode::prioritize_speed) {
if(!SymCleanup(proc)) {
if(!SymCleanup(duplicatedHandle)) {
throw internal_error("SymCleanup failed");
}
}
Expand Down
Loading