[vs18.3] Fix the lifecycle of task host factory

[ViktorHofer]

If the task host factory is explicitly requested, do not act as a long lived task host. This is important as customers use task host factories for short lived tasks to release potential locks after the build.

This goes back to the previous behavior.
This partially regressed with #12620

Fixes #13013

[ViktorHofer]

@copilot please add a test to validate this scenario

[Copilot]

@ViktorHofer I've opened a new pull request, #13023, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

This PR fixes a regression where tasks with Runtime=NET were incorrectly using long-lived task hosts (with node reuse) even when the task host factory was explicitly requested. The fix ensures that explicitly requested task host factories always use short-lived task hosts, allowing DLLs to be released after build completion.

Changes:

• Removed the || isNetRuntime condition from the useSidecarTaskHost logic, ensuring TaskHostFactoryExplicitlyRequested takes precedence
• Added clear inline comments explaining when sidecar task hosts should and should not be used

[YuliiaKovalova]

could you please publish a matrix or document it somewhere when we expect to have a short/long lived processes?

[baronfel]

@YuliiaKovalova I just asked copilot to make sure we have a test that explicitly covers the matrix in the linked test-creation PR, so that should handle your request too :)

[ViktorHofer]

I just added a table to our own documentation. Thanks for the suggestion @YuliiaKovalova

[YuliiaKovalova]

@YuliiaKovalova I just asked copilot to make sure we have a test that explicitly covers the matrix in the linked test-creation PR, so that should handle your request too :)

Ok, it looks like it will be long-lived only for

  <UsingTask
	    TaskName="ExampleTask"
            AssemblyFile="$(ExampleTaskPath)"
	    Runtime="NET"/>

But here it's short-lived

  <UsingTask
	    TaskName="ExampleTask"
            AssemblyFile="$(ExampleTaskPath)"
            TaskFactory="TaskHostFactory"
	    Runtime="NET"/>

is it correct?

[ViktorHofer]

It's quite easy. Whenever you specify TaskHostFactory, the process is external and short-lived. See the table for all cases.

[ViktorHofer]

OK I see a weird behavior for .NET Framework msbuild.exe launching a TaskFactory="TaskHostFactory" && Runtime="NET" task.
If there's a long running dotnet.exe task already hanging around, it apparently uses that. When there isn't one hanging around, it creates a short live task as expected.

I would be surprised if that's expected as we discussed that TaskHostFactory should always be short-lived. I need to find out why that's the case in the product code.

[ViktorHofer]

OK @YuliiaKovalova explained to me that this is happening because we don't respect the nodeReuse setting here:

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

Line 246 in 446a15b

if (!TryReuseAnyFromPossibleRunningNodes(currentProcessId, nodeId) && !StartNewNode(nodeId))

We might want to fix this in 10.0.1xx as well as it would impact TaskHostFactory there as well. In the when there's already a long running task host running in the process tree.