Skip to content

Stop the DBI from unloading any DAC on non-Windows - #133211

Open
hoyosjs wants to merge 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/dbi-no-dac-unload-unix
Open

Stop the DBI from unloading any DAC on non-Windows#133211
hoyosjs wants to merge 1 commit into
dotnet:mainfrom
hoyosjs:juhoyosa/dbi-no-dac-unload-unix

Conversation

@hoyosjs

@hoyosjs hoyosjs commented Sep 3, 2026

Copy link
Copy Markdown
Member

CordbProcess::FreeDac calls FreeLibrary on the DAC module, which dlcloses it on Unix. That is not safe for either DAC flavor.

The DAC embeds the PAL. TLSInitialize registers a pthread key whose destructor (InternalEndCurrentThreadWrapper) lives in the DAC image, and nothing deletes that key before the unload:

  • the DAC's DLL_PROCESS_DETACH (daccess.cpp DllMain2) only destroys the DAC mutex,
  • TLSCleanup / pthread_key_delete is reachable only from the PAL_Initialize failure unwind or from PAL_TerminateEx, which ends in exit().

So glibc keeps a destructor pointer into memory that has just been unmapped, and any thread that entered the DAC's PAL faults in __nptl_deallocate_tsd when it exits. This is the same defect fixed host-side in microsoft/clrmd#1499.

It reproduces deterministically with the universal DBI: dbgshim hands libmscordaccore_universal.so to the DBI as its DAC module, so every debuggee teardown unloads the cDAC and the next launch remaps it at a new base. The NativeAOT runtime never calls pthread_key_delete at all (PalUnix.cpp creates the key with RuntimeThreadShutdown and there is no matching delete). The legacy DAC has the same hazard, just intermittently.

Fix

Detach the holder instead of freeing it on non-Windows, so the image stays mapped. Effectively leaks one mapping per DAC path. There is no reversible in-process PAL teardown to call instead.

Validation

Root-caused on Linux x64 with a dlclose breakpoint under gdb: the faulting dlclose of libmscordaccore_universal.so originates in CordbProcess::FreeDac inside libmscordbi_universal.so, reached from CordbProcess::Neuter on the RC event thread. Suppressing the unload clears a reproducible 4-test crash cluster in the diagnostics debugger tests

CordbProcess::FreeDac calls FreeLibrary on the DAC module, which dlcloses
it. The DAC embeds the PAL, whose TLS initialization registers a pthread
key with a destructor that lives in the DAC image, and nothing deletes
that key before the unload. Any thread that entered the DAC's PAL then
faults in __nptl_deallocate_tsd when it exits.

Detach the holder instead so the image stays mapped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8b4e8405-4057-4272-928b-25e9d44a3b01
Copilot AI lite review requested due to automatic review settings September 3, 2026 23:10
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

@steveisok
steveisok requested a review from a team September 3, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is minimal, uses existing HOST_UNIX patterns in the file, and safely avoids an unsafe unload path without introducing new API surface area.

Pull request overview

Prevents unloading the DAC module on Unix hosts during CordbProcess::FreeDac(), avoiding unsafe dlclose behavior where the DAC’s embedded PAL can leave process-global TLS destructor pointers referencing unmapped memory.

Changes:

  • On HOST_UNIX, replace m_hDacModule.Free() with m_hDacModule.Detach() so the DAC stays mapped for the lifetime of the debugging process.
  • Add an explanatory comment and a log message to clarify why the DAC is intentionally left loaded on Unix.
File summaries
File Description
src/coreclr/debug/di/process.cpp Avoids unloading the DAC on Unix by detaching the module holder instead of calling FreeLibrary/dlclose.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

@noahfalk noahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@hoyosjs
hoyosjs enabled auto-merge (squash) September 4, 2026 04:17
@hoyosjs
hoyosjs disabled auto-merge September 4, 2026 04:17
@hoyosjs hoyosjs changed the title Don't unload the DAC on non-Windows Stop the DBI from unloading any DAC on non-Windows Sep 4, 2026
@hoyosjs
hoyosjs enabled auto-merge (squash) September 4, 2026 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants