[MrBot] ARM64: fix intermittent process-exit hang in VLD teardown leak report [Ready]

[aiagentpool-mrbot]

Summary

On ARM64, a process that links VLD can hang forever after main returns —
inside VLD's leak report at DLL_PROCESS_DETACH, under the loader lock. This
surfaced in Azure-MessagingStore's gated ARM64 unit-test leg: under
concurrent ctest -j load, *_ut_exe_ebs.exe processes intermittently wedged
at process exit and stalled the whole leg until the job timeout. It is not
specific to any one test — three different test exes wedged simultaneously in
one prior repro.

Addresses ADO Task 38853991
(https://msazure.visualstudio.com/One/_workitems/edit/38853991).

Root cause (confirmed from crash dumps + source)

Full-memory dumps of the wedged processes (single-threaded post-main) show
thread 0 stuck in:

ntdll!NtQueryVirtualMemory  <-  KERNELBASE!QueryVirtualMemoryInformation
  vld!GetCallingModule                        (utility.cpp)
  ... VLD leak-report internals ...
  vld!VisualLeakDetector::~VisualLeakDetector  (leak report, under the loader lock)

~VisualLeakDetector runs the leak report under the loader lock. GetCallingModule()
resolves the owning module of an address using QueryVirtualMemoryInformation
(and VirtualQuery as a fallback) — both of which reach NtQueryVirtualMemory.
On ARM64, under the loader lock at process exit, that per-address memory-region
query intermittently livelocks (documented dbghelp/ARM64 interaction,
aka.ms/AA10dvw4); on x64 it does not.

During teardown GetCallingModule() is reached from two kinds of caller, so
guarding only one is not enough:

• (a) VLD's own allocation hooks → CaptureContext::~CaptureContext →
  IsExcludedModule(), for allocations the report itself makes (dbghelp's
  internal allocations during symbol resolution; VLD's report bookkeeping).
• (b) CallStack::resolve() / resolveFunction() → GetCallingModule()
  per frame, driven by the ARM64 pre-warm GetLeaksCount() that resolves
  call stacks to decide leak suppression.

The pre-existing Arm64TeardownReportScope only suppressed dbghelp
symbolization; it did nothing about either GetCallingModule path.

The fix and regression coverage

1. GetCallingModule() uses a loader-based lookup on the ARM64 teardown
   thread. The teardown thread ID is published before the pre-warm (so it
   covers caller (b), unlike Arm64TeardownReportScope which must stay off during
   the pre-warm so suppression names can be resolved). GetCallingModule() uses
   the fallback only when the current thread matches that ID, so concurrently
   active threads retain the normal non-loader lookup. On the teardown thread it
   resolves the module with
   GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | …_UNCHANGED_REFCOUNT)
   — a loader-list walk that cannot reach NtQueryVirtualMemory and is safe
   under the already-held loader lock. For a code address (every call-stack frame)
   it returns the same module base; for a non-module address it returns NULL,
   which every caller already treats as “not a tracked module.” This is the
   change that removes the hang.
2. ~CaptureContext(): when leak detection is disabled on this thread or the
   thread already holds the DbgHelp lock, skip IsExcludedModule() — the inner
   heap hooks already recorded no block, so nothing is mapped either way. Mirrors
   the existing _HeapAlloc guard; removes redundant GetCallingModule calls
   (caller (a)).
3. ~VisualLeakDetector(): disable leak detection on the teardown thread
   across the pre-warm + ReportLeaks(), restoring it before thread-local storage
   is torn down later in the same destructor, so the report’s own transient
   allocations are not tracked.
4. Deterministic ARM64 integration test: patch VLD's imported
   QueryVirtualMemoryInformation entry with a blocking function immediately
   before process exit. The unfixed teardown path deterministically times out;
   the fixed loader-based path never reaches the hook and exits normally. CMake
   registers the test only for ARM64 with a 15-second timeout, so the existing
   ARM64 ctest steps run it in both Debug and RelWithDebInfo builds.

Non-weakening / same reported leaks: the loader lookup returns the same module
base for code addresses; the guarded allocations in (2)/(3) were never going to be
mapped. No assertion or leak check is disabled or relaxed. x64/x86 behavior is
untouched (every change is under #if defined(_M_ARM64)).

Verification — native ARM64 hardware (Azure Cobalt Standard_D16plds_v6)

Using the gate's own published ARM64 unit-test binaries (build 172760233,
273 test exes) run with the gate command ctest -C Debug -j 16 (a per-test
--timeout reaps a stuck-at-exit process so a wedge shows up as a Timeout).
Baseline and fix were both built from source with the identical
RelWithDebInfo config (same dll size; only this change differs), so the fix is
the only variable:

• Baseline — unfixed master @ a5e71c7: wedged on pass 1 (a test
  process failed to exit and was reaped by --timeout). Matches the captured
  dumps.
• With this fix: 20 / 20 consecutive full-suite ctest -j16
  passes were green — 100% of the 273 tests passed on every pass, ~161 s per
  pass, zero exit-time wedges. (Run as a 3-pass warm-up plus 17 more,
  back-to-back with the same fixed dll.)
• Regression harness: the new test cross-compiles for ARM64 with strict
  warnings enabled. Its import hook also deterministically blocks the equivalent
  unfixed lookup path in a local x64 harness, while the PR's ARM64 pipeline runs
  the fixed-path assertion.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Matt Durak (mattdurak)]

@microsoft-github-policy-service rerun

[Matt Durak (mattdurak)]

@microsoft-github-policy-service rerun

[Matt Durak (mattdurak)]

@microsoft-github-policy-service rerun

[Matt Durak (mattdurak)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Matt Durak (mattdurak)]

• High — process-global teardown flag can deadlock other threads.  g_arm64InTeardown  ( src/vld.cpp:243-252 ) affects every thread. While the teardown thread holds the loader lock, other threads calling  GetCallingModule()  ( src/utility.cpp:1516-1533 ) switch to  GetModuleHandleExW  and block on that lock.  waitForAllVLDThreads()  can then time out and free VLD state before those threads resume, causing deadlock or use-after-free.

Scope the fallback to the teardown thread, e.g. store its thread ID and require  GetCurrentThreadId()  to match. No other actionable findings.

[aiagentpool-mrbot]

🤖 MrBot: Addressed in e0aeff7 by replacing the process-wide teardown flag with the teardown thread ID. GetCallingModule() now selects the loader-based fallback only when the current thread ID matches, so other active threads continue using the normal lookup and cannot block on the teardown thread's loader lock. The x64 Debug build and all 17 tests pass, and the changed teardown logic cross-compiles for ARM64. I also requested the Azure VLD gate, but Azure Pipelines rejected the bot account because it lacks rerun privileges.

[aiagentpool-mrbot]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 65 in repo Azure/vld

[Matt Durak (mattdurak)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Matt Durak (mattdurak)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[anporumb]

we should have an int test that

1. would hang on "previous" SW version. Would be awesome to have the test deterministic.
2. would not hang with the proposed.
   Probably the bot can write it, there are such bugs happening which were the trigger for this fix.

[Matt Durak (mattdurak)]

Mr. Bot, as Andrei suggests, create a minimal repro. Locally prove that it hangs deterministically with ARM64 without the fix and then ensure it passes with this fixed version. The repro int test must be run as part of the build yml on ARM64.

[Matt Durak (mattdurak)]

.

[aiagentpool-mrbot]

🤖 MrBot: Addressed in 3aba07c. I added an ARM64-only CTest integration repro that patches VLD's QueryVirtualMemoryInformation import with a blocking function immediately before ExitProcess. The unfixed teardown deterministically reaches that hook and times out; the fixed teardown uses the loader-based lookup and exits without calling it. The existing ARM64 Debug and RelWithDebInfo jobs already run the full CTest suite, so this test is now part of both ARM64 build legs.

The test source cross-compiles for ARM64 with /W4 /WX, the full x64 Debug suite remains green, and the hook deterministically blocks the equivalent unfixed lookup path in a local x64 harness. This agent host is x64, so the native execution of the fixed ARM64 test will occur in the PR's ARM64 pipeline.

This was just while we fix the "require two human reviewers" bug

[Matt Durak (mattdurak)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Matt Durak (mattdurak)]

sign off as driver, waiting on second review

[anporumb]

Looks good.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.