This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 9d784c4d [WIN] Enhance threadsafety of DbgHelp (#754)
9d784c4d is described below
commit 9d784c4da74ff7360d76c79a89fe60a63516f880
Author: Tianqi Chen <[email protected]>
AuthorDate: Sun Sep 6 10:45:09 2026 -0400
[WIN] Enhance threadsafety of DbgHelp (#754)
`src/ffi/backtrace_win.cc` called DbgHelp unguarded, though DbgHelp is
single-threaded and requires the caller to serialize it. The process
died while
formatting a backtrace for another thread's exception, so the original
error was
never printed.
- One mutex around every DbgHelp call made by tvm_ffi.
- A duplicated process handle as the session key. `GetCurrentProcess()`
has
the same value in every component, so another DbgHelp user in the
process could
close our symbols. A fresh key also makes `SymInitialize`'s failure
unambiguous, so its result can be checked.
- Initialize once and keep the session, dropping the per-call
`SymInitialize`/`SymCleanup` and with it module enumeration inside the
lock.
There is deliberately no destructor: `SymCleanup` at static-destruction
time
would reintroduce teardown at an unpredictable point relative to DLL
unload.
A failed initialization now returns a backtrace with only the caller's
own frame.
`StackWalk64` resolves through the session, so without one there is no
unwinding
at all.
---
src/ffi/backtrace_win.cc | 56 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 52 insertions(+), 4 deletions(-)
diff --git a/src/ffi/backtrace_win.cc b/src/ffi/backtrace_win.cc
index ca5cf06d..6f782b90 100644
--- a/src/ffi/backtrace_win.cc
+++ b/src/ffi/backtrace_win.cc
@@ -32,10 +32,51 @@
#include <tvm/ffi/error.h>
#include <iostream>
+#include <mutex>
#include <vector>
#include "./backtrace_utils.h"
+namespace {
+
+/*! \brief Global singleton holding tvm_ffi's DbgHelp symbol session. */
+struct DbgHelpSession {
+ /*! \brief Serializes every DbgHelp call made by tvm_ffi. */
+ std::mutex mutex;
+ /*! \brief Session handle, or nullptr when no session could be established.
*/
+ HANDLE handle = nullptr;
+
+ static DbgHelpSession* Global() {
+ static DbgHelpSession inst;
+ return &inst;
+ }
+
+ DbgHelpSession() {
+ HANDLE current_process_handle = GetCurrentProcess();
+ // Duplicate rather than key the session on GetCurrentProcess() directly:
that
+ // value is the same in every component, so another DbgHelp user in this
process
+ // could close our symbols. A duplicated handle is a key only we hold.
+ if (!DuplicateHandle(current_process_handle, current_process_handle,
current_process_handle,
+ &handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ handle = nullptr;
+ return;
+ }
+ // LOAD_LINES keeps file and line information; UNDNAME demangles C++ names.
+ SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME);
+ // TRUE enumerates loaded modules now, so no module is discovered later
while
+ // the mutex is held. Unlike the previous code the result is checked: a
session
+ // that failed to initialize must not be cached as usable.
+ if (!SymInitialize(handle, NULL, TRUE)) {
+ // Nothing was filed under this key, so there is no session to
SymCleanup;
+ // release the handle and leave it null to report the failure.
+ CloseHandle(handle);
+ handle = nullptr;
+ }
+ }
+};
+
+} // namespace
+
const TVMFFIByteArray* TVMFFIBacktrace(const char* filename, int lineno, const
char* func,
int cross_ffi_boundary) {
static thread_local std::string backtrace_str;
@@ -51,11 +92,19 @@ const TVMFFIByteArray* TVMFFIBacktrace(const char*
filename, int lineno, const c
backtrace.Append(filename, func, lineno);
}
- HANDLE process = GetCurrentProcess();
HANDLE thread = GetCurrentThread();
- SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME);
- SymInitialize(process, NULL, TRUE);
+ DbgHelpSession* session = DbgHelpSession::Global();
+ std::lock_guard<std::mutex> lock(session->mutex);
+ HANDLE process = session->handle;
+ if (process == nullptr) {
+ // StackWalk64 resolves through the session, so without one there is
nothing
+ // to walk. Report the caller's own frame instead of using a dead session.
+ backtrace_str = backtrace.GetBacktrace();
+ backtrace_array.data = backtrace_str.data();
+ backtrace_array.size = backtrace_str.size();
+ return &backtrace_array;
+ }
CONTEXT context = {};
RtlCaptureContext(&context);
@@ -138,7 +187,6 @@ const TVMFFIByteArray* TVMFFIBacktrace(const char*
filename, int lineno, const c
}
backtrace.Append(filename, symbol, lineno);
}
- SymCleanup(process);
backtrace_str = backtrace.GetBacktrace();
backtrace_array.data = backtrace_str.data();
backtrace_array.size = backtrace_str.size();
