Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ public override void Initialize(
if (_collectProcessDumpOnHang)
{
_inactivityTimer ??= new InactivityTimer(CollectDumpAndAbortTesthost);
ResetInactivityTimer();
// Don't start the timer here — wait until TestHostLaunched so we know
// which process to dump. ResetInactivityTimer is called from TestHostLaunchedHandler.
}
Comment on lines 200 to 205
Comment on lines 200 to 205
Comment on lines +203 to 205
}

Expand Down Expand Up @@ -244,6 +245,10 @@ private void CollectDumpAndAbortTesthost()
EqtTrace.Verbose("Inactivity timer is already disposed.");
}

// If testhost has not launched yet, the timer should not have been started,
// so this callback should not fire. Assert to catch unexpected paths.
TPDebug.Assert(_testHostProcessId != 0, "CollectDumpAndAbortTesthost called but testhost has not launched yet.");

Comment on lines +248 to +251
Comment on lines +248 to +251
if (_collectProcessDumpOnCrash)
{
// Detach the dumper from the testhost process to prevent crashing testhost process. When the dumper is procdump.exe
Expand Down Expand Up @@ -630,9 +635,9 @@ private void SessionEndedHandler(object? sender, SessionEndEventArgs args)
/// <param name="args">TestHostLaunchedEventArgs</param>
private void TestHostLaunchedHandler(object? sender, TestHostLaunchedEventArgs args)
{
ResetInactivityTimer();
_testHostProcessId = args.TestHostProcessId;
Interlocked.Exchange(ref _testHostProcessId, args.TestHostProcessId);
_testHostProcessName = _processHelper.GetProcessName(args.TestHostProcessId);
ResetInactivityTimer();

if (!_collectProcessDumpOnCrash)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="cs">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="de">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="es">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="fr">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="it">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="ja">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="ko">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="pl">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="pt-BR">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="ru">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="tr">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="zh-Hans">
<body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" original="../Resources.resx" target-language="zh-Hant">
<body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,46 @@ public void HangDumpChildProcesses(RunnerInfo runnerInfo)
ValidateDump(2);
}

[TestMethod]
[DoNotParallelize] // Modifies the test asset's runtimeconfig.json on disk.
[NetCoreTargetFrameworkDataSource]
public void HangDumpShouldNotHangWhenTestHostFailsToStart(RunnerInfo runnerInfo)
Comment on lines +241 to +244
{
// When testhost can't start (e.g. wrong runtime version), the inactivity timer
// must not try to dump PID 0. It should exit cleanly because the timer only starts
// after TestHostLaunched, which never fires here.
SetTestEnvironment(_testEnvironment, runnerInfo);
const string testAssetProjectName = "SimpleTestProjectMessedUpTargetFramework";
var assemblyPath = GetTestDllForFramework(testAssetProjectName + ".dll", Core11TargetFramework);

// Make testhost fail immediately by targeting a non-existent runtime.
var runtimeConfigJson = Path.Combine(Path.GetDirectoryName(assemblyPath)!, testAssetProjectName + ".runtimeconfig.json");
var originalContent = File.ReadAllText(runtimeConfigJson);
try
{
var updatedContent = Regex.Replace(originalContent, @"""version""\s*:\s*""[\d.]+""", @"""version"": ""9999.0.0""");
File.WriteAllText(runtimeConfigJson, updatedContent);

Comment on lines +258 to +260
var arguments = PrepareArguments(assemblyPath, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}");
arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;HangDumpType=mini;TestTimeout=30s""");
InvokeVsTest(arguments);

// vstest should exit with failure (testhost didn't start), but not hang and not crash.
ExitCodeEquals(1);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try ensuring harder the crash was because we did not find the runtime.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — added Assert.MatchesRegex after ExitCodeEquals(1) that checks the combined stdout+stderr for the runtime-not-found message (matches "framework.*9999.0.0", "9999.0.0.*not found", or "compatible framework version"). This verifies the failure was specifically due to the missing runtime, not some other error.

🔧 Iterated by PR Iteration Agent 🔧

// Verify the failure was specifically because the runtime wasn't found, not some other error.
// The .NET runtime error can appear in stderr or stdout depending on the platform/host.
Assert.IsTrue(
Regex.IsMatch(StdErr, "9999\\.0\\.0") || Regex.IsMatch(StdOut, "9999\\.0\\.0"),
$"Expected '9999.0.0' in output.\nStdErr: {StdErr}\nStdOut: {StdOut}");
Assert.DoesNotContain(".dmp", StdOut, "no dump should be collected when testhost never launched");
}
finally
{
File.WriteAllText(runtimeConfigJson, originalContent);
}
}
Comment on lines +243 to +279

[TestMethod]
[TestCategory("Windows-Review")]
[DoNotParallelize] // Installs/uninstalls procdump as machine-wide postmortem debugger via HKLM registry.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void InitializeWithDumpForHangDisabledShouldNotInitializeInactivityTimerO

/// <summary>
/// Initializing with collect dump for hang should configure the timer with the right values and should
/// not call the reset method if no events are received.
/// start the timer when testhost launches (not during Initialize).
/// </summary>
[TestMethod]
public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndCallResetOnce()
Expand All @@ -127,12 +127,17 @@ public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndCallReset
_mockLogger.Object,
_context);

Assert.AreEqual(1, resetCalledCount, "Should have called InactivityTimer.Reset exactly once since no events were received");
Assert.AreEqual(0, resetCalledCount, "Should not have called InactivityTimer.Reset during Initialize — timer starts on TestHostLaunched");

// Simulate testhost launching — this should start the timer.
_mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234));

Assert.AreEqual(1, resetCalledCount, "Should have called InactivityTimer.Reset exactly once after TestHostLaunched");
}

/// <summary>
/// Initializing with collect dump for hang should configure the timer with the right values and should
/// reset for each event received
/// reset for each event received (including TestHostLaunched which starts the timer).
/// </summary>
[TestMethod]
public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndResetForEachEventReceived()
Expand All @@ -151,6 +156,9 @@ public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndResetForE
_mockLogger.Object,
_context);

// Simulate testhost launching — this starts the timer (1st reset).
_mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234));

TestCase testcase = new("TestProject.UnitTest.TestMethod", new Uri("test:/abc"), "abc.dll");

_mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(testcase));
Expand Down Expand Up @@ -182,13 +190,16 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
_mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set());

_blameDataCollector.Initialize(
GetDumpConfigurationElement(false, false, true, 0),
GetDumpConfigurationElement(false, false, true, 50),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moot — with the timer starting only after TestHostLaunched, there's no race between the timer and the PID assignment. The 50ms timeout is fine.

_mockDataColectionEvents.Object,
_mockDataCollectionSink.Object,
_mockLogger.Object,
_context);

hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken);
// Simulate testhost launching before the timer fires.
_mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234));

hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken);
_mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
_mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny<bool>()), Times.Once);
_mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
Expand Down Expand Up @@ -216,13 +227,16 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet
_mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny<bool>())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception"));

_blameDataCollector.Initialize(
GetDumpConfigurationElement(false, false, true, 0),
GetDumpConfigurationElement(false, false, true, 50),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — no race with the new approach.

_mockDataColectionEvents.Object,
_mockDataCollectionSink.Object,
_mockLogger.Object,
_context);

hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken);
// Simulate testhost launching before the timer fires.
_mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234));

hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken);
_mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
_mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny<bool>()), Times.Once);
}
Expand Down Expand Up @@ -251,18 +265,52 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt
_mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception"));

_blameDataCollector.Initialize(
GetDumpConfigurationElement(false, false, true, 0),
GetDumpConfigurationElement(false, false, true, 50),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — no race with the new approach.

_mockDataColectionEvents.Object,
_mockDataCollectionSink.Object,
_mockLogger.Object,
_context);

hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken);
// Simulate testhost launching before the timer fires.
_mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234));

hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken);
_mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
_mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny<bool>()), Times.Once);
_mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
}

/// <summary>
/// If testhost has not launched, the inactivity timer should not be started, so no hang dump
/// should be attempted even after the configured timeout elapses.
/// </summary>
[TestMethod]
public void InitializeWithDumpForHangShouldNotStartTimerIfTestHostHasNotLaunchedYet()
{
_blameDataCollector = new TestableBlameCollector(
_mockBlameReaderWriter.Object,
_mockProcessDumpUtility.Object,
null,
_mockFileHelper.Object,
_mockProcessHelper.Object);

_blameDataCollector.Initialize(
GetDumpConfigurationElement(false, false, true, 0),
_mockDataColectionEvents.Object,
_mockDataCollectionSink.Object,
_mockLogger.Object,
_context);

// Do NOT raise TestHostLaunched — timer should never start.
// Wait long enough that a started timer with timeout 0 would have fired.
Thread.Sleep(100);
Comment on lines +304 to +306

// The hang dump must not have been attempted.
_mockProcessDumpUtility.Verify(
x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()),
Times.Never);
}

/// <summary>
/// The trigger session ended handler should write to file if test start count is greater.
/// </summary>
Expand Down
Loading