17.12 VS hang on solution close

[JanKrivanek]

Context

microsoft.visualstudio.projectservices.dll!Microsoft.VisualStudio.ProjectServices.DesignTimeBuildService.SolutionClosedEventHandler
https://prism.vsdata.io/failure/?query=ch%3Drelease%20r%3D17.12&eventType=hang&failureHash=cdfda8b0-fdb8-70cd-57c9-ad606708a21f
116 hits in the wild - ranks as top 14 hang

It seems related to previously fixed case: #10709 (comment) (thoug it seems slightly different)

FYI @davkean (pointed this case out)

[JanKrivanek]

It looks like we lost information about worker node shutdown and hence keep waiting for it (nodeId is 2 - pointing out to OutOfProcNode).
Otherwise the submission is completed with build cancelled result - so this is distinct from #10709

[JanKrivanek]

Through ComponentHost DI I got to the PID of the out of proc node that the BuildManager was waiting for - its: 10396:

From the case ETL

I can see that that that process did not terminate:

So it might be possible that shutdown request wasn't even properly sent out

[JanKrivanek]

I can see that the main node requested that node to be shutdown

But without the worker node dump it's hard to tell why that got stuck.

In any case - we might want to asynchronously wait abit after signaling shutdown request and just close the connection if we do not hear back:

msbuild/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

Lines 107 to 137 in 04ef516

	protected void ShutdownConnectedNodes(List<NodeContext> contextsToShutDown, bool enableReuse)
	{
	// Send the build completion message to the nodes, causing them to shutdown or reset.
	_processesToIgnore.Clear();
	// We wait for child nodes to exit to avoid them changing the terminal
	// after this process terminates.
	bool waitForExit = !enableReuse &&
	!Console.IsInputRedirected &&
	Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout;
	Task[] waitForExitTasks = waitForExit && contextsToShutDown.Count > 0 ? new Task[contextsToShutDown.Count] : null;
	int i = 0;
	var loggingService = _componentHost.LoggingService;
	foreach (NodeContext nodeContext in contextsToShutDown)
	{
	if (nodeContext is null)
	{
	continue;
	}
	nodeContext.SendData(new NodeBuildComplete(enableReuse));
	if (waitForExit)
	{
	waitForExitTasks[i++] = nodeContext.WaitForExitAsync(loggingService);
	}
	}
	if (waitForExitTasks != null)
	{
	Task.WaitAll(waitForExitTasks);
	}
	}

We do similar under some conditions (however the wait is 30 seconds - which is too long for UI actions):

msbuild/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs

Lines 829 to 870 in 04ef516

	public async Task WaitForExitAsync(ILoggingService loggingService)
	{
	if (_exitPacketState == ExitPacketState.ExitPacketQueued)
	{
	// Wait up to 100ms until all remaining packets are sent.
	// We don't need to wait long, just long enough for the Task to start running on the ThreadPool.
	await Task.WhenAny(_packetWriteDrainTask, Task.Delay(100));
	}
	if (_exitPacketState == ExitPacketState.ExitPacketSent)
	{
	CommunicationsUtilities.Trace("Waiting for node with pid = {0} to exit", _process.Id);
	// .NET 5 introduces a real WaitForExitAsyc.
	// This is a poor man's implementation that uses polling.
	int timeout = TimeoutForWaitForExit;
	int delay = 5;
	while (timeout > 0)
	{
	bool exited = _process.WaitForExit(milliseconds: 0);
	if (exited)
	{
	return;
	}
	timeout -= delay;
	await Task.Delay(delay).ConfigureAwait(false);
	// Double delay up to 500ms.
	delay = Math.Min(delay * 2, 500);
	}
	}
	// Kill the child and do a blocking wait.
	loggingService?.LogWarning(
	BuildEventContext.Invalid,
	null,
	BuildEventFileInfo.Empty,
	"KillingProcessWithPid",
	_process.Id);
	CommunicationsUtilities.Trace("Killing node with pid = {0}", _process.Id);
	_process.KillTree(timeoutMilliseconds: 5000);
	}

But we do not need to necessarily kill the process - we can first just close the connection - node should augument NodeShutdown packet in such case.

[YuliiaKovalova]

reported bug with the same problem: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2342674