Fix hangdump running into crashdump

[nohwnd]

Description

This fixes issue that happens when both hang and crash dumps are enabled, which can be common in CI. The issue results in no crash or hang dump being created.

When process crashes and procdump starts crashdumping it takes it some time to write all the memory to disk. During that time it is possible that hangdump timer will reach the threshold and initiate hang dump. This will try to cancel procdump, and before this fix we simply sent Ctlr+C to procdump, and immediately went to kill it. Killing attached procdump almost always results in testhost (to which it is attached) crashing with it. This will kill testhost, and you will have no crashdump, nor hangdump.

To solve this, we look at the output that procdump streams to us and try to detect if it is currently dumping a process, and wait for that to finish. When it finishes dumping, or after a given timeout (90 seconds, or "VSTEST_CONNECTION_TIMEOUT"), we cancel and if needed kill procdump.

Then we progress to hang dumping the process. This is needed because procdump is configurable by environment variables, and it is possible to set dumpcount that is higher than 1, so it is totally possible to create 7 crashdumps, and continue running, and then reach a place where we hang.

Additionally we also write errors from procdump into log.

Testing this in automated tests is quite fragile, so I did not add any tests. But reproducing it locally is quite easy by making crash.dll allocate a bunch of memory (10GB roughly), which makes it write the dump for 10 seconds on my computer, and then fine tune the hang dump timeout to 5-20 seconds to the process has enough time to crash, but not finish crash dumping.

Modified this test to also create hang dumps after 10 seconds:

    [TestMethod]
    [NetCoreRunner("net462;net472;netcoreapp3.1;net5.0")]
    // should make no difference, keeping for easy debug
    // [NetFrameworkRunner("net462;net472;netcoreapp3.1;net5.0")]
    public void CrashDumpOnStackOverflow(RunnerInfo runnerInfo)
    {
        SetTestEnvironment(_testEnvironment, runnerInfo);
        var assemblyPaths = GetAssetFullPath("crash.dll");
        var arguments = PrepareArguments(assemblyPaths, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
        arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}");
        arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;TestTimeout=10s;CollectDump;DumpType=full"" /diag:S:\t\logs\l1\");

        var env = new Dictionary<string, string?>
        {
            ["PROCDUMP_PATH"] = _procDumpPath
        };

        InvokeVsTest(arguments, env);

        ValidateDump();
    }

Allocate some random strings into an array to put stuff in memory:

// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.VisualStudio.TestTools.UnitTesting;

using System;
using System.Collections.Generic;
using System.Linq;

#pragma warning disable IDE1006 // Naming Styles
namespace timeout
#pragma warning restore IDE1006 // Naming Styles
{
    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            var list = new List<a>();
            list.AddRange(Enumerable.Range(0, 100000).Select(a => new a()));
            // stack overflow
            Span<byte> s = stackalloc byte[int.MaxValue];
        }
    }

    public class a
    {
        public static Random random = new Random();
        public a()
        {
            abc = new string(((char)(byte)random.Next(0, 255)), random.Next(10000, 100_000)); 
        }
        public string abc;
    }
}

Output in diag logs, where crashdump is orange and initiates the dump, and hangdump is red, and waits for it to finish:

I also tried it when the hangdump is too fast (e.g. 3s), and it simply collects just hangdump, or when it is too slow (e.g. 100s), and then it collects just crashdump.

Fix #4351

[Evangelink]

Nitpick: I saw you fixed some messages to use procdump instead of proc dump. We are mixing procdump and ProcDump, could we use only one or the other (I would favoir latter).

I see quite some code added but no unit test code added. Would it make sense to have some more added? Thinking mainly about ProcDumpDumper.cs and DetachFromTargetProcess.

[Evangelink]

Same 2 remarks (UtcNow + IClock) here.

[nohwnd]

Switching back to draft, I will roll back some of this, I can't reliably differentiate between creating a dump on crash and on exit, and it outputs crash dumps when there is no crash which is undesirable, and would make users wonder what crashed where.

[nohwnd]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).