[fix] Fix missing dumps for .NET Framework child processes in NetClientHangDumper

[Jakub Jareš (nohwnd)]

Summary

🤖 This is an automated fix created by the Issue Repro Triage & Auto-Fix agent.

Fixes #15580

Root Cause

NetClientHangDumper is used on Windows for .NETCoreApp targets and on all platforms for Linux/macOS. It uses DiagnosticsClient.WriteDump() (from Microsoft.Diagnostics.NETCore.Client) to collect hang dumps from the process tree.

However, DiagnosticsClient can only connect to managed .NET Core/5+ processes via the diagnostics IPC socket. When a .NET Core test host has child processes that target .NET Framework (net462, net48, etc.) or native processes, there is no diagnostics socket — WriteDump() throws after a 30-second timeout. The exception was silently swallowed with only an error log, resulting in missing dump files for those child processes.

Fix

On Windows, when DiagnosticsClient.WriteDump() fails, fall back to WindowsHangDumper.CollectDump() which uses MiniDumpWriteDump via P/Invoke (dbghelp.dll). This API works for any Windows process, regardless of runtime (.NET Framework, native, etc.).

Changes:

• Move outputFile path computation before the try block so it's accessible in the catch
• Add Windows fallback to WindowsHangDumper.CollectDump() in the catch block
• Both the primary failure and any secondary fallback failure are logged clearly

Testing

Existing unit tests pass. The existing HangDumpChildProcesses acceptance test in BlameDataCollectorTests.cs covers the child-process dump collection path (for .NET Core child processes). A new test specifically for .NET Framework child processes would require a Windows-only test asset and is not added here — the fallback path is exercised whenever DiagnosticsClient fails for any reason on Windows.

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

[Copilot]

Pull request overview

This PR improves hang-dump collection reliability in the Blame Data Collector when dumping process trees on Windows: if Microsoft.Diagnostics.NETCore.Client.DiagnosticsClient.WriteDump() fails (notably for .NET Framework or native child processes that lack a diagnostics IPC endpoint), the code now falls back to the Windows minidump-based dumper.

Changes:

• Compute the dump output path before entering the try so it can be reused on failures.
• On Windows, add a fallback path that calls WindowsHangDumper.CollectDump() when DiagnosticsClient.WriteDump() throws.
• Improve error logging to distinguish the primary (DiagnosticsClient) failure from the fallback failure.

[Jakub Jareš (nohwnd)]

Code Review — [Crash & Hang Dump Reliability]

Activated dimensions (from routing): Crash & Hang Dump Reliability · Error Reporting & Diagnostic Clarity · Environment Variable & Feature Flag Contracts

Summary

The fix is sound. Moving outputFile before the try block and adding the WindowsHangDumper.CollectDump fallback in the catch correctly addresses the scenario where a .NET Framework child process has no diagnostics socket and DiagnosticsClient.WriteDump times out after 30 s.

What was checked:

• Fallback safety: MiniDumpWriteDump.CollectDumpUsingMiniDumpWriteDump opens the file with FileMode.Create, so any partial output left by the primary attempt is cleanly overwritten. Since the try block contains no code after WriteDump(), a successful primary dump is never at risk of being overwritten.
• Architecture mismatches: WindowsHangDumper.CollectDump correctly handles x86/x64/ARM64 mismatches via DumpMinitool — this is important for 32-bit .NET Framework child processes running under a 64-bit host.
• Process suspension: The fallback calls CollectDump (the static helper) rather than Dump, so processes are not suspended before dumping. This is consistent with NetClientHangDumper's explicit no-suspension design, and an unsuspended MiniDump is far better than no dump for a hang scenario.
• Platform guard: The RuntimeInformation.IsOSPlatform(OSPlatform.Windows) check is correct — .NET Framework processes can only exist on Windows, and dbghelp.dll is Windows-only.
• Timeout interaction: The CancellationToken is checked before the task starts, not mid-execution, so the fallback may run slightly past the 5-minute deadline. This is benign: CollectDump checks process.HasExited, and the kill loop runs after Task.WhenAll throws, so if the timeout fires and processes are killed before the fallback runs, it exits gracefully.
• PR description alignment: Matches the diff accurately.

One comment posted inline about comment scope vs. actual catch breadth — no blocking issues found.

---

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Jakub Jareš (nohwnd)]

[Crash & Hang Dump Reliability] The inline comment on lines 82–84 frames the fallback as specific to the "no diagnostics socket" case (.NET Framework / native processes), but the catch block is catch (Exception ex) — it activates for any DiagnosticsClient.WriteDump failure on Windows.

That broader scope is actually correct and beneficial: MiniDumpWriteDump.CollectDumpUsingMiniDumpWriteDump uses FileMode.Create, so it safely overwrites any partial/corrupt output the primary attempt may have left behind. And since the try block ends immediately after WriteDump() returns, a successful primary dump can never be accidentally clobbered by the fallback.

The risk is that a future reader may try to narrow this to ServerNotAvailableException based on the comment, inadvertently regressing the broader-failure handling. Consider updating the comment to make the intentional scope explicit, e.g.:

// DiagnosticsClient can only connect to .NET Core/5+ processes. On Windows, fall back
// to MiniDumpWriteDump for any DiagnosticsClient failure — this primarily covers .NET Framework
// and native child processes with no diagnostics socket, but also acts as a last resort for
// other transient failures. FileMode.Create in CollectDump ensures any partial output is replaced.

[Jakub Jareš (nohwnd)]

Done — updated the comment to explicitly call out that the catch (Exception ex) intentionally covers all DiagnosticsClient failures on Windows (not just missing-socket), and noted that FileMode.Create in CollectDump safely replaces any partial primary output.

🔧 Iterated by PR Iteration Agent 🔧

[Jakub Jareš (nohwnd)]

Commit pushed: a77e3cf

🔧 Iterated by PR Iteration Agent 🔧

[Jakub Jareš (nohwnd)]

Code Review — [Crash & Hang Dump Reliability] · Follow-up

Activated dimensions: Crash & Hang Dump Reliability · Error Reporting & Diagnostic Clarity · Resource & IDisposable Management · Algorithmic Correctness

Summary

This follow-up review confirms the previous finding has been fully addressed by the PR Iteration Agent (run 27049397912). No new issues found.

What was re-checked

Finding from prior review — resolved ✅
The comment in the catch block (lines 82–85) now explicitly documents that catch (Exception ex) intentionally covers all DiagnosticsClient.WriteDump failures on Windows — not just the missing-socket case. The FileMode.Create safety note is also present. The comment in the current head matches the suggestion exactly.

Additional checks completed:

• outputFile moved before try: Safe in practice — p.ProcessName was already accessed and implicitly cached during the processTree LINQ filter (line 28, .ToList()). The theoretical exception path from Path.Combine bypassing the inner catch is benign; no real scenario where this would regress.
• new ProcessHelper(): ProcessHelper does not implement IDisposable — no resource leak from the inline allocation.
• Fallback isolation: The fallback try/catch (Exception fallbackEx) correctly prevents a failing CollectDump from masking the primary error already logged.
• process.HasExited guard: WindowsHangDumper.CollectDump checks HasExited before dumping — graceful behavior if the process exits before the fallback executes.
• Thread safety: ProcessHelper.GetCurrentProcessArchitecture() and GetProcessArchitecture() are read-only operations; concurrent Task.Run invocations are safe.
• PR description alignment: Description accurately reflects all code changes.

---

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠