[release/10.0] Enhance crash dump collection options in RemoteExecutor - #17060
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR ports the “enhance crash dump collection options in RemoteExecutor” feature to release/10.0, adding configurable crash-dump behavior for RemoteExecutor subprocesses and modernizing timeout diagnostics (including updated diagnostic dependencies required on this branch).
Changes:
- Adds
CrashDumpCollectionType,CrashDumpPath, andEnableTimeoutDumpCollectionoptions to control crash-dump environment variables and timeout dump behavior. - Refactors timeout diagnostics to use
DiagnosticsClient.WriteDumpon .NET (Core) and CLRMD v4 thread enumeration, while keeping the legacyMiniDumppath for .NET Framework. - Updates package versions (CLRMD upgrade + new NETCore.Client) and adds tests validating crash-dump env-var behavior and dump creation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Packages.props | Updates central package versions; adds Microsoft.Diagnostics.NETCore.Client and upgrades CLRMD to v4. |
| eng/Versions.props | Bumps SystemMemoryVersion to satisfy upgraded dependency requirements. |
| src/Microsoft.DotNet.RemoteExecutor/src/RemoteExecutor.cs | Sets/removes DOTNET_Dbg* env vars based on new RemoteInvokeOptions crash-dump settings. |
| src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs | Adds EnableTimeoutDumpCollection gate and refactors timeout dump + thread diagnostics using DiagnosticsClient/CLRMD v4. |
| src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeOptions.cs | Introduces crash-dump configuration API surface (CrashDumpCollectionType, CrashDumpPath, EnableTimeoutDumpCollection). |
| src/Microsoft.DotNet.RemoteExecutor/src/Microsoft.DotNet.RemoteExecutor.csproj | Adds NETCore.Client dependency (netcore only), removes MiniDump.cs from netcore compilation, and applies scoped VersionOverrides. |
| src/Microsoft.DotNet.RemoteExecutor/tests/RemoteExecutorTests.cs | Adds tests for crash-dump env-var behavior and dump creation on crash. |
| src/Microsoft.DotNet.RemoteExecutor/tests/Microsoft.DotNet.RemoteExecutor.Tests.csproj | Mirrors scoped VersionOverrides to keep test restore graph consistent with src project. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use Invoke(...).Dispose() instead of unused 'using' locals in three crash-dump env-var tests for consistency with the rest of the file - Inline dumpFiles into SpinWait.SpinUntil in CrashDumpCollection_CreatesDumpOnCrash - Reword inaccurate #if NETCOREAPP comment in RemoteInvokeHandle.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@mmitche - mind taking a look at this? Had to pin some versions as backporting the full thing means things start to change a lot of things. |
mmitche
left a comment
There was a problem hiding this comment.
.NET package pinning looks incorrect. Otherwise fine.
…vert global bumps, refine crash-dump test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Microsoft.Diagnostics.Runtime 4.0.732101 hard-required Azure.Identity 1.21.0 -> Azure.Core 1.53.0, which dragged in the full .NET 10 (10.0.x) runtime cascade and forced nine local VersionOverrides plus Azure package pins in RemoteExecutor. Lowering to the stable CLRMD 4.0.725402 (and matching NETCore.Client 0.2.652701) is satisfied by the repo's existing Azure.Core 1.47.0 / Azure.Identity 1.14.2, so no Azure or 10.0.x overrides are needed. The only remaining conflict is a small set of stable 9.0.x transitive packages (System.Collections.Immutable 9.0.10, Microsoft.Bcl.AsyncInterfaces 9.0.8, Microsoft.Extensions.DependencyInjection.Abstractions 9.0.8) that exceed release/10.0's flowed 9.0.0-rc.2 central versions; those three are bumped up via project-local VersionOverride instead of touching the central (Maestro-flowed) versions repo-wide. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ride Per review feedback, move the three stable 9.0.x runtime-package bumps required by Microsoft.Diagnostics.Runtime 4.0.725402 / NETCore.Client 0.2.652701 from project-local VersionOverrides into the central version definitions: - Microsoft.Bcl.AsyncInterfaces 9.0.0-rc.2 -> 9.0.8 - Microsoft.Extensions.DependencyInjection.Abstractions 9.0.0-rc.2 -> 9.0.8 - System.Collections.Immutable 9.0.0-rc.2 -> 9.0.10 Values are updated in eng/Version.Details.props (the PackageVersion properties) with the matching Dependency versions in eng/Version.Details.xml kept in sync so darc stays consistent. The VersionOverride ItemGroups and NU1510 suppressions are removed from both RemoteExecutor csproj files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…a VersionOverride Microsoft.Diagnostics.Runtime 4.0.732101 + Microsoft.Diagnostics.NETCore.Client 0.2.721401 pull in newer Azure.Identity/Azure.Core and 10.0.x runtime packages than this servicing branch's central versions. Applying those bumps centrally would break the repo's net472 MSBuild build tasks, which are capped at 8.0.x by the toolset MSBuild.exe binding redirects (eng/BuildTask.Packages.props). Scope the required bumps to the RemoteExecutor src + tests projects via VersionOverride instead, leaving central versions at baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ion allocation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| <PackageVersion Include="Microsoft.Diagnostics.NETCore.Client" Version="0.2.721401" /> | ||
| <PackageVersion Include="Microsoft.Diagnostics.Runtime" Version="4.0.732101" /> |
| <PropertyGroup> | ||
| <NoWarn>$(NoWarn);NU1510</NoWarn> | ||
| </PropertyGroup> |
| catch { } | ||
| } |
| <PackageReference Include="Azure.Core" VersionOverride="1.53.0" /> | ||
| <PackageReference Include="Azure.Identity" VersionOverride="1.21.0" /> | ||
| <PackageReference Include="Microsoft.Identity.Client" VersionOverride="4.83.1" /> |
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="Microsoft.Extensions.FileProviders.Abstractions" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="System.Collections.Immutable" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="System.Text.Encodings.Web" VersionOverride="10.0.7" /> |
| <PackageReference Include="Azure.Core" VersionOverride="1.53.0" /> | ||
| <PackageReference Include="Azure.Identity" VersionOverride="1.21.0" /> | ||
| <PackageReference Include="Microsoft.Identity.Client" VersionOverride="4.83.1" /> |
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="Microsoft.Extensions.FileProviders.Abstractions" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="System.Collections.Immutable" VersionOverride="10.0.7" /> | ||
| <PackageReference Include="System.Text.Encodings.Web" VersionOverride="10.0.7" /> |
| /// <summary> | ||
| /// Collects diagnostic information (dump + thread stacks) from the timed-out remote process. | ||
| /// </summary> |
|
This broke building with VS msbuild repos that use 10.0 Arcade: #17103. The unconditional SystemMemoryVersion bump, more precisely. |
Summary
This is the
release/10.0servicing port of #16716 ("Enhance crash dump collection options in RemoteExecutor"). It brings the crash-dump-collection feature toMicrosoft.DotNet.RemoteExecutor:CrashDumpCollectionTypeenum andEnableTimeoutDumpCollection/CrashDumpCollectionType/CrashDumpPathoptions onRemoteInvokeOptions.CollectTimeoutDiagnosticsthat usesMicrosoft.Diagnostics.NETCore.ClientDiagnosticsClient.WriteDumpon .NET Core plus CLRMD v4 thread enumeration; the .NET Framework#elsepath still usesMiniDump.cs.RemoteExecutorTests.cs.The feature source/test files (
RemoteExecutor.cs,RemoteInvokeOptions.cs,RemoteInvokeHandle.cs,RemoteExecutorTests.cs) are taken from #16716.Dependency handling (the release/10.0 adaptation)
mainalready had CLRMD (Microsoft.Diagnostics.Runtime) on v4 with high enough Azure/runtime central versions.release/10.0is on older dependencies, so the feature's required upgrades needed adapting:Directory.Packages.props):Microsoft.Diagnostics.Runtime1.0.5→4.1.735601and addMicrosoft.Diagnostics.NETCore.Client0.2.721401.SystemMemoryVersion4.5.5→4.6.3(required by CLRMD's net462 leg; low-risk facade bump, whole-solution restore is clean).VersionOverrideon the RemoteExecutorsrc+testsprojects only for the transitive Azure/runtime packages CLRMD pulls in:Azure.Core 1.53.0,Azure.Identity 1.21.0,Microsoft.Identity.Client 4.83.1,System.Collections.Immutable 10.0.7,Microsoft.Bcl.AsyncInterfaces 10.0.7,Microsoft.Extensions.DependencyInjection.Abstractions 10.0.7,Microsoft.Extensions.FileProviders.Abstractions 10.0.7,System.Text.Encodings.Web 10.0.7,System.Text.Json 10.0.7.Why VersionOverride instead of a central bump: applying those Azure/runtime bumps centrally would break the repo's net472 MSBuild build-task projects, which are capped at 8.0.x by the toolset
MSBuild.exebinding redirects (seeeng/BuildTask.Packages.props). Scoping the bumps to RemoteExecutor keeps the build tasks unaffected while still satisfying CLRMD.NU1510is suppressed on those two projects because some overridden packages are provided by the shared framework on .NETCoreApp targets. Central runtime versions inVersion.Details.props/.xmlare left at baseline.Intentionally excluded
The Managed Identity /
DefaultIdentityTokenCredentialchange from #16716 is not ported — it requires Azure.Identity 1.21 repo-wide and is unrelated to crash dumps.Port of #16716.