Skip to content

[WIN] Enhance threadsafety of DbgHelp - #754

Merged
tqchen merged 1 commit into
apache:mainfrom
tqchen:fix-win-dbghelp-session
Sep 6, 2026
Merged

[WIN] Enhance threadsafety of DbgHelp#754
tqchen merged 1 commit into
apache:mainfrom
tqchen:fix-win-dbghelp-session

Conversation

@tqchen

@tqchen tqchen commented Sep 6, 2026

Copy link
Copy Markdown
Member

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.

@tqchen

tqchen commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

fixes #727 , thanks to @pisarev

DbgHelp is single-threaded and the caller must serialize it, but TVMFFIBacktrace
called it unguarded. An exception raised while another thread was formatting a
backtrace crashed the process inside the reporter, so the original error was
never printed.

Guard every DbgHelp call with one mutex. Key the session on a duplicated process
handle rather than GetCurrentProcess(), whose value is shared by every component,
so another DbgHelp user cannot close our symbols. Initialize once and keep the
session instead of doing it per call, which also removes module enumeration from
the locked region.

A failed initialization now yields a backtrace carrying only the caller's own
frame instead of walking against a session that was never established.
@tqchen tqchen changed the title [BUGFIX] Serialize DbgHelp and give tvm_ffi a private symbol session [WIN] Enhance threadsafety of DbgHelp Sep 6, 2026
@tqchen
tqchen force-pushed the fix-win-dbghelp-session branch from b3930f1 to 9e25fce Compare September 6, 2026 14:28
@tqchen
tqchen merged commit 9d784c4 into apache:main Sep 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants