Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@
<PackageVersion Include="Microsoft.ApplicationInsights" Version="2.23.0" />
<PackageVersion Include="Microsoft.Data.OData" Version="5.8.4" />
<PackageVersion Include="Microsoft.Data.Services.Client" Version="$(MicrosoftDataServicesClientVersion)" />
<PackageVersion Include="Microsoft.Diagnostics.Runtime" Version="1.0.5" />
<PackageVersion Include="Microsoft.Diagnostics.NETCore.Client" Version="0.2.661903" />
<PackageVersion Include="Microsoft.Diagnostics.Runtime" Version="4.0.0-beta.26210.1" />
Comment thread
hoyosjs marked this conversation as resolved.
<PackageVersion Include="Microsoft.Identity.Client" Version="4.80.0" />
<PackageVersion Include="Microsoft.OpenApi" Version="1.3.2" />
<PackageVersion Include="Microsoft.OpenApi.Readers" Version="1.3.2" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Diagnostics.NETCore.Client" Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'" />
<PackageReference Include="Microsoft.Diagnostics.Runtime" />
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'" />
</ItemGroup>
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.DotNet.RemoteExecutor/src/MiniDump.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#if !NETCOREAPP
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.

nit: we usually exclude such files for given TFM in the project file


using System;
using System.ComponentModel;
using System.Diagnostics;
Expand Down Expand Up @@ -73,3 +75,5 @@ private enum MINIDUMP_TYPE : int
}
}
}

#endif
22 changes: 22 additions & 0 deletions src/Microsoft.DotNet.RemoteExecutor/src/RemoteExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,28 @@ private static RemoteInvokeHandle Invoke(MethodInfo method, string[] args,
psi.Environment.Remove("CoreClr_Enable_Profiling");
}

if (options.DisableCrashDumpCollection)
{
psi.Environment.Remove("DOTNET_DbgEnableMiniDump");
psi.Environment.Remove("DOTNET_DbgMiniDumpType");
psi.Environment.Remove("DOTNET_DbgMiniDumpName");
}
else if (options.CrashDumpCollectionType.HasValue)
{
psi.Environment["DOTNET_DbgEnableMiniDump"] = "1";
psi.Environment["DOTNET_DbgMiniDumpType"] = ((int)options.CrashDumpCollectionType.Value).ToString();
if (!string.IsNullOrWhiteSpace(options.CrashDumpPath))
{
psi.Environment["DOTNET_DbgMiniDumpName"] = options.CrashDumpPath;
}
else
{
string uploadPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT");
string dumpDir = !string.IsNullOrWhiteSpace(uploadPath) ? uploadPath : IOPath.GetTempPath();
psi.Environment["DOTNET_DbgMiniDumpName"] = IOPath.Combine(dumpDir, "%e.%p.%t.dmp");
}
}

// If we need the host (if it exists), use it, otherwise target the console app directly.
string metadataArgs = PasteArguments.Paste(new string[] { a.FullName, t.FullName, method.Name, options.ExceptionFile }, pasteFirstArgumentUsingArgV0Rules: false);
string passedArgs = pasteArguments ? PasteArguments.Paste(args, pasteFirstArgumentUsingArgV0Rules: false) : string.Join(" ", args);
Expand Down
82 changes: 41 additions & 41 deletions src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Diagnostics.Runtime;
using System;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -10,6 +9,10 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
#if NETCOREAPP
using Microsoft.Diagnostics.NETCore.Client;
#endif
using Microsoft.Diagnostics.Runtime;

namespace Microsoft.DotNet.RemoteExecutor
{
Expand Down Expand Up @@ -146,81 +149,78 @@ private void Dispose(bool disposing)
{
description.AppendLine($"Timed out at {DateTime.Now} after {Options.TimeOut}ms waiting for remote process.");

// Create a dump if possible
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
if (Options.EnableTimeoutDumpCollection)
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.

perhaps it's a good opportunity for moving this logic to a dedicated method? The level of nesting was very high even before this PR

{
string uploadPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT");
if (!string.IsNullOrWhiteSpace(uploadPath))
{
try
{
string miniDmpPath = Path.Combine(uploadPath, $"{Process.Id}.{Path.GetRandomFileName()}.dmp");
MiniDump.Create(Process, miniDmpPath);
description.AppendLine($"Wrote mini dump to: {miniDmpPath}");
string dumpPath = Path.Combine(uploadPath, $"{Process.Id}.{Path.GetRandomFileName()}.dmp");
#if NETCOREAPP
// These define guards assume that harness running on .NET Framework implies test process runs on .NET Framework.
var client = new DiagnosticsClient(Process.Id);
client.WriteDump(DumpType.Full, dumpPath, logDumpGeneration: false);
#else
MiniDump.Create(Process, dumpPath);
#endif
description.AppendLine($"Wrote dump to: {dumpPath}");
}
catch (Exception exc)
{
description.AppendLine($"Failed to create mini dump: {exc.Message}");
description.AppendLine($"Failed to create dump: {exc.Message}");
}
}
}

// Gather additional details about the process if possible
try
{
description.AppendLine($"\tProcess ID: {Process.Id}");
description.AppendLine($"\tHandle: {Process.Handle}");
description.AppendLine($"\tName: {Process.ProcessName}");
description.AppendLine($"\tMainModule: {Process.MainModule?.FileName}");
description.AppendLine($"\tStartTime: {Process.StartTime}");
description.AppendLine($"\tTotalProcessorTime: {Process.TotalProcessorTime}");

// Attach ClrMD to gather some additional details.
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && // As of Microsoft.Diagnostics.Runtime v1.0.5, process attach only works on Windows.
Interlocked.CompareExchange(ref s_clrMdLock, 1, 0) == 0) // Make sure we only attach to one process at a time.
// Gather additional details about the process if possible
try
{
try
description.AppendLine($"\tProcess ID: {Process.Id}");
description.AppendLine($"\tHandle: {Process.Handle}");
description.AppendLine($"\tName: {Process.ProcessName}");
description.AppendLine($"\tMainModule: {Process.MainModule?.FileName}");
description.AppendLine($"\tStartTime: {Process.StartTime}");
description.AppendLine($"\tTotalProcessorTime: {Process.TotalProcessorTime}");

// Attach ClrMD to gather some additional details.
if (Interlocked.CompareExchange(ref s_clrMdLock, 1, 0) == 0) // Make sure we only attach to one process at a time.
{
using (DataTarget dt = DataTarget.AttachToProcess(Process.Id, msecTimeout: 20_000)) // arbitrary timeout
try
{
using DataTarget dt = DataTarget.CreateSnapshotAndAttach(Process.Id);
ClrRuntime runtime = dt.ClrVersions.FirstOrDefault()?.CreateRuntime();
if (runtime != null)
if (runtime is not null)
{
// Dump the threads in the remote process.
description.AppendLine("\tThreads:");
foreach (ClrThread thread in runtime.Threads.Where(t => t.IsAlive))
{
string threadKind =
thread.IsThreadpoolCompletionPort ? "[Thread pool completion port]" :
thread.IsThreadpoolGate ? "[Thread pool gate]" :
thread.IsThreadpoolTimer ? "[Thread pool timer]" :
thread.IsThreadpoolWait ? "[Thread pool wait]" :
thread.IsThreadpoolWorker ? "[Thread pool worker]" :
string threadKind =
thread.IsGc ? "[Thread that started suspension]" :
thread.IsFinalizer ? "[Finalizer]" :
thread.IsGC ? "[GC]" :
"";
"Unknown";

string isBackground = thread.IsBackground ? "[Background]" : "";
string apartmentModel = thread.IsMTA ? "[MTA]" :
thread.IsSTA ? "[STA]" :
string isBackground = thread.State.HasFlag(ClrThreadState.TS_Background) ? "[Background]" : "";
string apartmentModel = thread.State.HasFlag(ClrThreadState.TS_InMTA) ? "[MTA]" :
thread.State.HasFlag(ClrThreadState.TS_InSTA) ? "[STA]" :
"";

description.AppendLine($"\t\tThread #{thread.ManagedThreadId} (OS 0x{thread.OSThreadId:X}) {threadKind} {isBackground} {apartmentModel}");
foreach (ClrStackFrame frame in thread.StackTrace)
foreach (ClrStackFrame frame in thread.EnumerateStackTrace())
{
description.AppendLine($"\t\t\t{frame}");
}
}
}
}
}
finally
{
Interlocked.Exchange(ref s_clrMdLock, 0);
finally
{
Interlocked.Exchange(ref s_clrMdLock, 0);
}
}
}
catch { }
}
catch { }

throw new RemoteExecutionException(description.ToString());
}
Expand Down
41 changes: 41 additions & 0 deletions src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@

namespace Microsoft.DotNet.RemoteExecutor
{
/// <summary>
/// The type of crash dump to collect. Maps to DOTNET_DbgMiniDumpType values
/// as documented in https://learn.microsoft.com/en-us/dotnet/core/diagnostics/collect-dumps-crash#types-of-mini-dumps. Only applies to .NET Core subprocesses.
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.

Suggested change
/// as documented in https://learn.microsoft.com/en-us/dotnet/core/diagnostics/collect-dumps-crash#types-of-mini-dumps. Only applies to .NET Core subprocesses.
/// as documented in <see href="https://learn.microsoft.com/dotnet/core/diagnostics/collect-dumps-crash#types-of-mini-dumps">the docs</see>. Only applies to .NET Core subprocesses.

/// </summary>
public enum CrashDumpCollectionType
{
Mini = 1,
Heap = 2,
Triage = 3,
Full = 4
}

/// <summary>
/// Options used with RemoteInvoke.
/// </summary>
Expand All @@ -22,6 +34,8 @@ public sealed class RemoteInvokeOptions

public bool EnableProfiling { get; set; } = true;

public bool EnableTimeoutDumpCollection { get; set; } = true;
Comment thread
hoyosjs marked this conversation as resolved.

public bool CheckExitCode { get; set; } = true;

/// <summary>
Expand Down Expand Up @@ -62,5 +76,32 @@ public bool RunAsSudo
/// Specifies the roll-forward policy for dotnet cli to use. Only applies when running .NET Core
/// </summary>
public string RollForward { get; set; }

/// <summary>
/// Gets or sets whether to configure crash dump collection on the subprocess via
/// DOTNET_DbgEnableMiniDump / DOTNET_DbgMiniDumpType / DOTNET_DbgMiniDumpName.
/// When set to a <see cref="CrashDumpCollectionType"/> value, crash dump collection is enabled
/// with that dump type. When set to null (default), the environment variables are left as-is.
/// To explicitly disable crash dumps (removing any inherited env vars), set <see cref="DisableCrashDumpCollection"/> to true.
Comment on lines +83 to +85
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.

Have you considered having a single property to control CrashDumpCollectionType and DisableCrashDumpCollection? Something like CrashDumpCollectionType having a default non-null value and then setting it to null would disable the colleciton?

/// </summary>
/// <remarks>
/// Only applies to .NET Core subprocesses.
/// </remarks>
public CrashDumpCollectionType? CrashDumpCollectionType { get; set; }

/// <summary>
/// Gets or sets the path template for crash dump files. When <see cref="CrashDumpCollectionType"/> is set,
/// this value is used for DOTNET_DbgMiniDumpName. Supports the same placeholders as createdump:
/// %p (PID), %e (process name), %t (timestamp), etc.
/// When null (default), defaults to HELIX_WORKITEM_UPLOAD_ROOT/%e.%p.%t.dmp if running in Helix,
/// or the system temp directory otherwise.
/// </summary>
public string CrashDumpPath { get; set; }

/// <summary>
/// When true, explicitly removes the DOTNET_DbgEnableMiniDump, DOTNET_DbgMiniDumpType, and
/// DOTNET_DbgMiniDumpName environment variables from the subprocess, disabling any inherited crash dump configuration.
/// </summary>
public bool DisableCrashDumpCollection { get; set; }
}
}
87 changes: 87 additions & 0 deletions src/Microsoft.DotNet.RemoteExecutor/tests/RemoteExecutorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.IO;
using System.Threading.Tasks;
using Xunit;
using Xunit.Sdk;
Expand Down Expand Up @@ -158,5 +159,91 @@ public static void IgnoreExitCode()
Assert.Equal(exitCode, h.ExitCode);
}
}

[Theory]
[InlineData(CrashDumpCollectionType.Mini, "1")]
[InlineData(CrashDumpCollectionType.Heap, "2")]
[InlineData(CrashDumpCollectionType.Triage, "3")]
[InlineData(CrashDumpCollectionType.Full, "4")]
public void CrashDumpCollection_SetsEnvVars(CrashDumpCollectionType dumpType, string expectedTypeValue)
{
using RemoteInvokeHandle h = RemoteExecutor.Invoke(expectedType =>
{
Assert.Equal("1", Environment.GetEnvironmentVariable("DOTNET_DbgEnableMiniDump"));
Assert.Equal(expectedType, Environment.GetEnvironmentVariable("DOTNET_DbgMiniDumpType"));
return RemoteExecutor.SuccessExitCode;
}, expectedTypeValue, new RemoteInvokeOptions
{
RollForward = "Major",
CrashDumpCollectionType = dumpType
});
}

[Fact]
public void DisableCrashDumpCollection_RemovesEnvVars()
{
// Pre-set the env vars on the StartInfo to simulate inherited values
var options = new RemoteInvokeOptions
{
RollForward = "Major",
DisableCrashDumpCollection = true
};
options.StartInfo.Environment["DOTNET_DbgEnableMiniDump"] = "1";
options.StartInfo.Environment["DOTNET_DbgMiniDumpType"] = "4";
options.StartInfo.Environment["DOTNET_DbgMiniDumpName"] = "/tmp/test.dmp";

using RemoteInvokeHandle h = RemoteExecutor.Invoke(() =>
{
Assert.Null(Environment.GetEnvironmentVariable("DOTNET_DbgEnableMiniDump"));
Assert.Null(Environment.GetEnvironmentVariable("DOTNET_DbgMiniDumpType"));
Assert.Null(Environment.GetEnvironmentVariable("DOTNET_DbgMiniDumpName"));
}, options);
}

[Fact]
public void CrashDumpCollection_DefaultLeavesEnvVarsUntouched()
{
// When neither option is set, env vars should pass through from the parent unchanged
using RemoteInvokeHandle h = RemoteExecutor.Invoke(() =>
{
// Without explicit config, the child inherits whatever the parent has.
// The parent test process shouldn't have DOTNET_DbgEnableMiniDump set,
// so the child shouldn't either.
string parentValue = Environment.GetEnvironmentVariable("DOTNET_DbgEnableMiniDump");
Assert.Null(parentValue);
}, new RemoteInvokeOptions { RollForward = "Major" });
}

[Fact]
public static unsafe void CrashDumpCollection_CreatesDumpOnCrash()
{
string dumpDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
Directory.CreateDirectory(dumpDir);
try
{
var options = new RemoteInvokeOptions
{
RollForward = "Major",
CrashDumpCollectionType = CrashDumpCollectionType.Mini,
CheckExitCode = false,
// Point the dump path to our temp directory so we can verify the file is created.
// Use %p so the filename includes the PID and is unique.
CrashDumpPath = Path.Combine(dumpDir, "crashdump.%p.dmp")
};

RemoteExecutor.Invoke(() =>
{
// Trigger an access violation to crash the process
*(int*)0x10000 = 0;
}, options).Dispose();

string[] dumpFiles = Directory.GetFiles(dumpDir, "*.dmp");
Assert.NotEmpty(dumpFiles);
}
finally
{
Directory.Delete(dumpDir, recursive: true);
}
}
}
}
Loading