Skip to content

Don't attempt to unload cdac - #132028

Merged
rcj1 merged 3 commits into
dotnet:mainfrom
rcj1:dont-unload-cdac
Aug 8, 2026
Merged

Don't attempt to unload cdac#132028
rcj1 merged 3 commits into
dotnet:mainfrom
rcj1:dont-unload-cdac

Conversation

@rcj1

@rcj1 rcj1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

As cDAC is a NativeAOT library, it cannot be safely unloaded. #131960 should prevent an actual unmapping from happening, but this makes the intent clear.

@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.

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.

Pull request overview

This PR clarifies that the cDAC (a NativeAOT library) should not be unloaded on Unix by guarding FreeLibrary calls with #ifndef HOST_UNIX, aligning intent with the prior change to prevent actual unmapping.

Changes:

  • Skip unloading the cDAC library on Unix when initialization fails.
  • Skip unloading the cDAC library on Unix during CDAC destruction.
Show a summary per file
File Description
src/coreclr/debug/daccess/cdac.cpp Adds HOST_UNIX guards around FreeLibrary calls to avoid attempting to unload cDAC on Unix.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/coreclr/debug/daccess/cdac.cpp Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 23:16

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.

Review details

Suppressed comments (2)

src/coreclr/debug/daccess/cdac.cpp:172

  • On Unix the module is now intentionally kept loaded by skipping FreeLibrary. Without an explanatory comment, this looks like a resource leak and makes the lifetime rule easy to misunderstand. Consider adding a short #else comment clarifying why unloading is avoided here.
#ifndef HOST_UNIX
        ::FreeLibrary(m_module);
#endif // HOST_UNIX

src/coreclr/debug/daccess/cdac.cpp:141

  • On Unix this now intentionally skips freeing the cDAC module, which can look like an accidental leak/refcount imbalance to future maintainers. Add an explicit comment in the #else branch explaining that this is deliberate (NativeAOT library unload is unsafe) so the behavior isn’t “fixed” later.

This issue also appears on line 170 of the same file.

#ifndef HOST_UNIX
        ::FreeLibrary(cdacLib);
#endif // HOST_UNIX
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment thread src/coreclr/debug/daccess/cdac.cpp Outdated
Copilot AI review requested due to automatic review settings August 8, 2026 00:30
@rcj1 rcj1 changed the title Don't attempt to unload cdac on Unix Don't attempt to unload cdac Aug 8, 2026

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.

Review details

Suppressed comments (1)

src/coreclr/debug/daccess/cdac.cpp:140

  • Since FreeLibrary was removed, a failed cdac_reader_init (or any subsequent CDAC::Create call) leaves the newly loaded module referenced forever. If DAC instances are created repeatedly in a long-lived process, this can accumulate module references unnecessarily. Consider caching the loaded HMODULE and reusing it for subsequent Create calls so the library is loaded once per process while still never being unloaded.
    intptr_t handle;
    if (init(descriptorAddr, &ReadFromTargetCallback, &WriteToTargetCallback, &ReadThreadContext, &WriteThreadContext, allocCallback, target, &handle) != 0)
    {
        return {};
    }
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@rcj1
rcj1 merged commit a94502c into dotnet:main Aug 8, 2026
100 of 103 checks passed
@rcj1
rcj1 deleted the dont-unload-cdac branch August 8, 2026 04:13
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.

4 participants