Dumping child processes (#2518)

* Add env variables in the incorrect place

* Collect all dumps from temp dir

* Try to build against net5.0

* Remove internals visibleto

* Collect child crash dumps

* Use procdump even on newer fmw to dump testhost

* Fix collecting dumps

* Get dumps only from guid dir

* Hangdumps of children on Windows

* Make it work on Linux

* Children and suspend on mac

* Fix

* enabled oses

* Fix

* Fix unit tests and review feedback

* Build is flaky

* Fix tests for dotnet5 update

azure-pipelines.yml

@@ -64,12 +64,12 @@ jobs:
       testResultsFiles: '**\*.trx'
     condition: succeededOrFailed()
 
-- job: Linux
-  pool:
-    vmImage: 'ubuntu-18.04'
-  variables:
-      buildConfiguration: 'Release'
-  steps:
-  - script: ./build.sh -c $(buildConfiguration)
-    displayName: './build.sh -c $(buildConfiguration)'
+# - job: Linux
+#   pool:
+#     vmImage: 'ubuntu-18.04'
+#   variables:
+#       buildConfiguration: 'Release'
+#   steps:
+#   - script: ./build.sh -c $(buildConfiguration)
+#     displayName: './build.sh -c $(buildConfiguration)'
 

global.json

@@ -1,12 +1,12 @@
 {
   "sdk": {
-    "version": "3.1.101",
+    "version": "5.0.100-rc.1.20380.12",
     "rollForward": "minor",
     "allowPrerelease": false,
     "architecture": "x64"
   },
   "tools": {
-    "dotnet": "3.1.101"
+    "dotnet": "5.0.100-rc.1.20380.12"
   },
   "msbuild-sdks": {
     "Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.20411.9",

scripts/build.ps1

@@ -82,7 +82,7 @@ $env:DOTNET_SKIP_FIRST_TIME_EXPERIENCE = 1
 # Dotnet build doesn't support --packages yet. See https://github.com/dotnet/cli/issues/2712
 $env:NUGET_PACKAGES = $env:TP_PACKAGES_DIR
 $env:NUGET_EXE_Version = "3.4.3"
-$env:DOTNET_CLI_VERSION = "3.1.101"
+$env:DOTNET_CLI_VERSION = "5.0.100-rc.1.20380.12"
 # $env:DOTNET_RUNTIME_VERSION = "LATEST"
 $env:VSWHERE_VERSION = "2.0.2"
 $env:MSBUILD_VERSION = "15.0"

scripts/build.sh

@@ -111,7 +111,7 @@ VERSION=$(test -z $VERSION && grep TPVersionPrefix $TP_ROOT_DIR/scripts/build/Te
 export DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
 # Dotnet build doesnt support --packages yet. See https://github.com/dotnet/cli/issues/2712
 export NUGET_PACKAGES=$TP_PACKAGES_DIR
-DOTNET_CLI_VERSION="3.1.101"
+DOTNET_CLI_VERSION="5.0.100-rc.1.20380.12"
 #DOTNET_RUNTIME_VERSION="LATEST"
 
 #

src/Microsoft.TestPlatform.Common/Microsoft.TestPlatform.Common.csproj

@@ -46,8 +46,5 @@
     <RootNamespace>Microsoft.VisualStudio.TestPlatform.Common</RootNamespace>
   </PropertyGroup>
 
-  <ItemGroup>
-    <InternalsVisibleTo Include="Microsoft.TestPlatform.Common.UnitTests" />
-  </ItemGroup>
   <Import Project="$(TestPlatformRoot)scripts\build\TestPlatform.targets" />
 </Project>

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs

@@ -48,6 +48,7 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier
         private int testHostProcessId;
         private bool dumpWasCollectedByHangDumper;
         private string targetFramework;
+        private List<KeyValuePair<string, string>> environmentVariables = new List<KeyValuePair<string, string>>();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="BlameCollector"/> class.
@@ -91,7 +92,7 @@ internal BlameCollector(
         /// <returns>Environment variables that should be set in the test execution environment</returns>
         public IEnumerable<KeyValuePair<string, string>> GetTestExecutionEnvironmentVariables()
         {
-            return Enumerable.Empty<KeyValuePair<string, string>>();
+            return this.environmentVariables;
         }
 
         /// <summary>
@@ -133,12 +134,26 @@ public override void Initialize(
                 if (this.collectProcessDumpOnTrigger)
                 {
                     this.ValidateAndAddTriggerBasedProcessDumpParameters(collectDumpNode);
+
+                    // enabling dumps on MacOS needs to be done explicitly https://github.com/dotnet/runtime/pull/40105
+                    this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgEnableElfDumpOnMacOS", "1"));
+                    this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgEnableMiniDump", "1"));
+
+                    var guid = Guid.NewGuid().ToString();
+
+                    var dumpDirectory = Path.Combine(Path.GetTempPath(), guid);
+                    Directory.CreateDirectory(dumpDirectory);
+                    var dumpPath = Path.Combine(dumpDirectory, $"dotnet_%d_crashdump.dmp");
+                    this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgMiniDumpName", dumpPath));
                 }
 
                 var collectHangBasedDumpNode = this.configurationElement[Constants.CollectDumpOnTestSessionHang];
                 this.collectProcessDumpOnTestHostHang = collectHangBasedDumpNode != null;
                 if (this.collectProcessDumpOnTestHostHang)
                 {
+                    // enabling dumps on MacOS needs to be done explicitly https://github.com/dotnet/runtime/pull/40105
+                    this.environmentVariables.Add(new KeyValuePair<string, string>("COMPlus_DbgEnableElfDumpOnMacOS", "1"));
+
                     this.ValidateAndAddHangBasedProcessDumpParameters(collectHangBasedDumpNode);
                 }
 
@@ -183,7 +198,7 @@ private void CollectDumpAndAbortTesthost()
 
             try
             {
-                this.processDumpUtility.StartHangBasedProcessDump(this.testHostProcessId, this.attachmentGuid, this.GetTempDirectory(), this.processFullDumpEnabled, this.targetFramework);
+                this.processDumpUtility.StartHangBasedProcessDump(this.testHostProcessId, this.GetTempDirectory(), this.processFullDumpEnabled, this.targetFramework);
             }
             catch (Exception ex)
             {
@@ -199,22 +214,33 @@ private void CollectDumpAndAbortTesthost()
 
             try
             {
-                var dumpFile = this.processDumpUtility.GetDumpFile();
-                if (!string.IsNullOrEmpty(dumpFile))
+                var dumpFiles = this.processDumpUtility.GetDumpFiles();
+                foreach (var dumpFile in dumpFiles)
                 {
-                    this.dumpWasCollectedByHangDumper = true;
-                    var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper);
-                    this.dataCollectionSink.SendFileAsync(fileTransferInformation);
-                }
-                else
-                {
-                    EqtTrace.Error("BlameCollector.CollectDumpAndAbortTesthost: blame:CollectDumpOnHang was enabled but dump file was not generated.");
+                    try
+                    {
+                        if (!string.IsNullOrEmpty(dumpFile))
+                        {
+                            this.dumpWasCollectedByHangDumper = true;
+                            var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper);
+                            this.dataCollectionSink.SendFileAsync(fileTransferInformation);
+                        }
+                    }
+                    catch (Exception ex)
+                    {
+                        // Eat up any exception here and log it but proceed with killing the test host process.
+                        EqtTrace.Error(ex);
+                    }
+
+                    if (!dumpFiles.Any())
+                    {
+                        EqtTrace.Error("BlameCollector.CollectDumpAndAbortTesthost: blame:CollectDumpOnHang was enabled but dump file was not generated.");
+                    }
                 }
             }
             catch (Exception ex)
             {
-                // Eat up any exception here and log it but proceed with killing the test host process.
-                EqtTrace.Error(ex);
+                ConsoleOutput.Instance.Error(true, $"Blame: Collecting hang dump failed with error {ex}.");
             }
 
             try
@@ -404,16 +430,22 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
                     {
                         try
                         {
-                            var dumpFile = this.processDumpUtility.GetDumpFile();
-                            if (!string.IsNullOrEmpty(dumpFile))
+                            var dumpFiles = this.processDumpUtility.GetDumpFiles();
+                            foreach (var dumpFile in dumpFiles)
                             {
-                                var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
-                                this.dataCollectionSink.SendFileAsync(fileTranferInformation);
-                            }
-                            else
-                            {
-                                EqtTrace.Warning("BlameCollector.SessionEndedHandler: blame:CollectDump was enabled but dump file was not generated.");
-                                this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
+                                if (!string.IsNullOrEmpty(dumpFile))
+                                {
+                                    try
+                                    {
+                                        var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
+                                        this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                                    }
+                                    catch (FileNotFoundException ex)
+                                    {
+                                        EqtTrace.Warning(ex.ToString());
+                                        this.logger.LogWarning(args.Context, ex.ToString());
+                                    }
+                                }
                             }
                         }
                         catch (FileNotFoundException ex)
@@ -453,7 +485,7 @@ private void TestHostLaunchedHandler(object sender, TestHostLaunchedEventArgs ar
 
             try
             {
-                this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, this.attachmentGuid, this.GetTempDirectory(), this.processFullDumpEnabled, ".NETFramework,Version=v4.0");
+                this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, this.GetTempDirectory(), this.processFullDumpEnabled, this.targetFramework);
             }
             catch (TestPlatformException e)
             {
@@ -508,12 +540,25 @@ private void DeregisterEvents()
 
         private string GetTempDirectory()
         {
-            var tmp = Path.GetTempPath();
-            if (!Directory.Exists(tmp))
+            string tempPath = null;
+            var netDumperPath = this.environmentVariables.SingleOrDefault(p => p.Key == "COMPlus_DbgMiniDumpName").Value;
+
+            try
             {
-                Directory.CreateDirectory(tmp);
+                if (!string.IsNullOrWhiteSpace(netDumperPath))
+                {
+                    tempPath = Path.GetDirectoryName(netDumperPath);
+                }
+            }
+            catch (ArgumentException)
+            {
+                // the path was not correct do nothing
             }
 
+            var tmp = !string.IsNullOrWhiteSpace(tempPath) ? tempPath : Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+
+            Directory.CreateDirectory(tmp);
+
             return tmp;
         }
     }

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs

@@ -16,9 +16,25 @@ public ICrashDumper Create(string targetFramework)
             {
                 EqtTrace.Info($"CrashDumperFactory: This is Windows, returning ProcDumpCrashDumper that uses ProcDump utility.");
                 return new ProcDumpCrashDumper();
+
+                // enable this once crashdump can trigger itself on exceptions that originate from task, then we can avoid using procdump
+                // if (!string.IsNullOrWhiteSpace(targetFramework) && !targetFramework.Contains("v5.0"))
+                // {
+                //     EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework} which is not net5.0 or newer, returning ProcDumpCrashDumper that uses ProcDump utility.");
+                //     return new ProcDumpCrashDumper();
+                // }
+
+                // EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework}, returning the .NETClient dumper which uses env variables to collect crashdumps of testhost and any child process.");
+                // return new NetClientCrashDumper();
+            }
+
+            if (!string.IsNullOrWhiteSpace(targetFramework) && targetFramework.Contains("v5.0"))
+            {
+                EqtTrace.Info($"CrashDumperFactory: This is {RuntimeInformation.OSDescription} on {targetFramework} .NETClient dumper which uses env variables to collect crashdumps of testhost and any child process.");
+                return new NetClientCrashDumper();
             }
 
-            throw new PlatformNotSupportedException($"Unsupported operating system: {RuntimeInformation.OSDescription}");
+            throw new PlatformNotSupportedException($"Unsupported operating system: {RuntimeInformation.OSDescription}, and framework: {targetFramework}.");
         }
     }
 }

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/HangDumperFactory.cs

@@ -28,7 +28,7 @@ public IHangDumper Create(string targetFramework)
                 }
 
                 EqtTrace.Info($"HangDumperFactory: This is Linux netcoreapp3.1 or newer, returning the standard NETClient library dumper.");
-                return new NetClientDumper();
+                return new NetClientHangDumper();
             }
 
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
@@ -41,10 +41,7 @@ public IHangDumper Create(string targetFramework)
                 }
 
                 EqtTrace.Info($"HangDumperFactory: This is OSX on net5.0 or newer, returning the standard NETClient library dumper.");
-
-                // enabling dumps on MacOS needs to be done explicitly https://github.com/dotnet/runtime/pull/40105
-                Environment.SetEnvironmentVariable("COMPlus_DbgEnableElfDumpOnMacOS", "1");
-                return new NetClientDumper();
+                return new NetClientHangDumper();
             }
 
             throw new PlatformNotSupportedException($"Unsupported operating system: {RuntimeInformation.OSDescription}");

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs

@@ -5,7 +5,7 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
     public interface ICrashDumper
     {
-        void AttachToTargetProcess(int processId, string outputFile, DumpTypeOption dumpType);
+        void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType);
 
         void WaitForDumpToFinish();
 

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/IHangDumper.cs

@@ -5,6 +5,6 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
     public interface IHangDumper
     {
-        void Dump(int processId, string outputFile, DumpTypeOption dumpType);
+        void Dump(int processId, string outputDirectory, DumpTypeOption dumpType);
     }
 }

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs

@@ -3,6 +3,8 @@
 
 namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
+    using System.Collections.Generic;
+
     public interface IProcessDumpUtility
     {
         /// <summary>
@@ -11,17 +13,14 @@ public interface IProcessDumpUtility
         /// <returns>
         /// Path of dump file
         /// </returns>
-        string GetDumpFile();
+        IEnumerable<string> GetDumpFiles();
 
         /// <summary>
         /// Launch proc dump process
         /// </summary>
         /// <param name="processId">
         /// Process ID of test host
         /// </param>
-        /// <param name="dumpFileGuid">
-        /// Guid as postfix for dump file, testhost.exe_&lt;guid&gt;.dmp
-        /// </param>
         /// <param name="testResultsDirectory">
         /// Path to TestResults directory
         /// </param>
@@ -31,17 +30,14 @@ public interface IProcessDumpUtility
         /// <param name="targetFramework">
         /// The target framework of the process
         /// </param>
-        void StartTriggerBasedProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump, string targetFramework);
+        void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework);
 
         /// <summary>
         /// Launch proc dump process to capture dump in case of a testhost hang and wait for it to exit
         /// </summary>
         /// <param name="processId">
         /// Process ID of test host
         /// </param>
-        /// <param name="dumpFileGuid">
-        /// Guid as postfix for dump file, testhost.exe_&lt;guid&gt;.dmp
-        /// </param>
         /// <param name="testResultsDirectory">
         /// Path to TestResults directory
         /// </param>
@@ -51,7 +47,7 @@ public interface IProcessDumpUtility
         /// <param name="targetFramework">
         /// The target framework of the process
         /// </param>
-        void StartHangBasedProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump, string targetFramework);
+        void StartHangBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework);
 
         /// <summary>
         /// Detaches the proc dump process from the target process

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs

@@ -0,0 +1,23 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
+{
+    internal class NetClientCrashDumper : ICrashDumper
+    {
+        public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType)
+        {
+            // we don't need to do anything directly here, we setup the env variables
+            // in the dumper configuration, including the path
+        }
+
+        public void DetachFromTargetProcess(int processId)
+        {
+            // here we might consider renaming the files to have timestamp
+        }
+
+        public void WaitForDumpToFinish()
+        {
+        }
+    }
+}