Skip to content

Anporumb/fixcopy - #67

Draft
anporumb wants to merge 2 commits into
masterfrom
anporumb/fixcopy
Draft

anporumb wants to merge 2 commits into
masterfrom
anporumb/fixcopy

Conversation

@anporumb

Copy link
Copy Markdown

No description provided.

Copilot AI added 2 commits August 5, 2026 11:58
Under concurrent ctest -j load on ARM64, a unit-test process can wedge forever
after main returns. Full-memory dumps of the wedged *_ut_exe_ebs.exe show
thread 0 (single-threaded post-main) stuck in NtQueryVirtualMemory <-
KERNELBASE!QueryVirtualMemoryInformation, reached from VLD's leak report that
runs under the loader lock at DLL_PROCESS_DETACH:

  ~VisualLeakDetector -> leak report (pre-warm GetLeaksCount + ReportLeaks)
    -> GetCallingModule() -> QueryVirtualMemoryInformation -> NtQueryVirtualMemory

On ARM64 that per-address memory-region query intermittently livelocks under
the loader lock at process exit (aka.ms/AA10dvw4); on x64 it does not. It can
strike ANY VLD-linked exe under concurrent -j load, not a specific test. The
existing Arm64TeardownReportScope only suppressed dbghelp symbolization, not the
GetCallingModule -> QueryVirtualMemoryInformation path that actually wedges.

GetCallingModule() is reached during teardown from two kinds of caller:
  (a) VLD's allocation hooks -> CaptureContext::~CaptureContext ->
      IsExcludedModule() (for the report's own transient allocations), and
  (b) CallStack::resolve()/resolveFunction() -> GetCallingModule() per frame,
      driven by the pre-warm's call-stack resolution.

Fix (ARM64-only, non-weakening), all in src/vld.cpp + src/utility.cpp:

1. GetCallingModule(): during the ARM64 teardown report, look the module up via
   the loader (GetModuleHandleExW FROM_ADDRESS) instead of
   QueryVirtualMemoryInformation/VirtualQuery. The loader walk cannot reach
   NtQueryVirtualMemory and is safe under the (already-held) loader lock. For a
   code address this returns the same module base; for a non-module address it
   returns NULL, which every caller already treats as 'not a tracked module'.
   Gated by a new broad g_arm64InTeardown flag set BEFORE the pre-warm (separate
   from g_arm64InTeardownReport, which must stay off during the pre-warm so
   suppression names can still be resolved). This covers caller (b) and (a).
2. ~CaptureContext(): when leak detection is disabled on this thread or the
   thread 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 TLS is torn down, so the
   report's own transient allocations are not tracked.

The reported leak set is unchanged (guarded allocations were never going to be
mapped; the loader lookup returns the same module base for code addresses).
x64/x86 behavior is untouched. Verified on ARM64 (Cobalt) hardware by looping
the gate's full unit-test ctest -j16 suite: unfixed wedges on pass 1, fixed
stays green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

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

Comment thread src/utility.cpp
extern VisualLeakDetector g_vld;
extern ImageDirectoryEntries g_Ide;
#if defined(_M_ARM64)
extern "C" bool VldArm64InTeardown(void);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VldArm64InTeardown

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.

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