[clr-interp] Skip RSLock leak assert for DbgTransportTarget on Unix

[kotlarmilos]

Description

The static g_DbgTransportTarget instance is destroyed after the host process's main() returns. On Unix (desktop interpreter) the dynamic loader does not invoke DbgDllMain(DLL_PROCESS_DETACH) for mscordbi.dylib, so DbgTransportTarget::Shutdown() may never run and the embedded RSLock m_sLock is left in its initialized state. In debug builds the RSLock destructor asserts that the lock was Destroy()'d before destruction, so the host process aborts.

Implementation

The shutdown path can't be made reliable on Unix without changes outside mscordbi, so the fix opts the lock out of the destructor assert instead of trying to release it:

• Add a new RSLock attribute cLockAllowLeak (src/coreclr/debug/di/rspriv.h) for static-lifetime locks whose owning shutdown path is not guaranteed to run.
• Update ~RSLock() (src/coreclr/debug/di/rspriv.inl) so the existing CONSISTENCY_CHECK_MSGF leak assert is bypassed when cLockAllowLeak is set; behavior for all other locks is unchanged.
• Initialize DbgTransportTarget::m_sLock with cLockFlat | cLockAllowLeak (src/coreclr/debug/di/dbgtransportmanager.cpp) so the static instance no longer trips the assert when destructed after main() returns.

Test impact

Locally on osx-arm64 Debugger.Tests --clrinterpreter the xunit fail count drops from 230 to 65:

• Inspection.BasicInspection, Inspection.EnumNames, Inspection.GenericInterfaces, Inspection.GenericSharedStatic, Inspection.GenericStatics, Inspection.NestedGenerics, Inspection.inspect_SetVars
• Stackwalking.ClrMethods
• Stepping.setIPTestwithUnvaildPos, Stepping.stepin, Stepping.stepover
• Async.AsyncStepInto

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR adds a defensive DbgTransportTarget destructor so the right-side debug transport can clean up its lock and tracked sessions during static destruction, covering Unix teardown paths where DbgDllMain(DLL_PROCESS_DETACH) may not run.

Changes:

• Declares an explicit destructor for DbgTransportTarget.
• Adds destructor logic that calls Shutdown() when the transport lock appears initialized.
• Keeps the fix localized to the debug transport manager used by the CoreCLR debugger transport layer.

Show a summary per file

File	Description
src/coreclr/debug/di/dbgtransportmanager.h	Declares the new DbgTransportTarget destructor.
src/coreclr/debug/di/dbgtransportmanager.cpp	Implements the destructor and conditionally invokes Shutdown() during static destruction.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[kotlarmilos]

As @janvorli noted, this is nice-to-have. The tests are intended to run primarily against release where the assert does not fire, so this only cleans up checked-build noise. The downside is that the order in which destructors of statics are executed is undefined, so one destructor can easily depend on something that was already destroyed by another.

[janvorli]

I would prefer fixing the issue by not using a static instance of the DbgTransportTarget instead. When the ~DbgTransportTarget is invoked, there is no guarantee that the Shutdown() call would succeed. The call can in the future rely on something that would be already torn down at the point when the C runtime calls the destructor.

However, I can see that @AaronRobinsonMSFT has introduced this static destructor recently and removed dynamic allocation of the transport, so I'd like to know his opinion first.

[AaronRobinsonMSFT]

However, I can see that @AaronRobinsonMSFT has introduced this static destructor recently and removed dynamic allocation of the transport, so I'd like to know his opinion first.

This was a simplification. I didn't add an explicit destructor for all the reasons we already know. We really should address the use of DbgDllMain in the Diagnostics PAL and avoid calling it entirely via explicit "Init" and "Shutdown" exports. I think this fix is fine for now. I do agree with you that relying on static constructors/destructors should generally be avoided.

[noahfalk]

We really should address the use of DbgDllMain

I agree that completely eliminating the DbgDllMain dependency would be a nicer resolution, but if that isn't straightforward I think we'd be fine allowing the leak. Typical debugger behavior loads DBI and never unloads it until process exit so any missed static cleanup from our code is likely backstopped by OS process cleanup.

[kotlarmilos]

Add a new RSLock attribute cLockAllowLeak update ~RSLock so the existing CONSISTENCY_CHECK_MSGF leak assert is bypassed when cLockAllowLeak is set.