Deadlock at ExecuteSubmission vs LoggingService during shutdown

[rokonec]

Issue Description

This deadlock has been detected while investigating
This bug has surfaced as hang at BuildManager.EndBuild.

Steps to Reproduce

This race condition triggered deadlock is very hard to reproduce.
However there are many CABs which includes devenv process dump which helped during issue investigation.

I have used this CAB during investigation.

Analysis

During execution BuildManager.ExecuteSubmission if build is canceled at particular time and following if branch is entered

msbuild/src/Build/BackEnd/BuildManager/BuildManager.cs

Lines 1132 to 1141 in 38eec6f

	if (_shuttingDown)
	{
	// We were already canceled!
	AddBuildRequestToSubmission(submission, resolvedConfiguration.ConfigurationId);
	BuildResult result = new BuildResult(submission.BuildRequest, new BuildAbortedException());
	submission.CompleteResults(result);
	submission.CompleteLogging(true);
	CheckSubmissionCompletenessAndRemove(submission);
	return;
	}

Line

msbuild/src/Build/BackEnd/BuildManager/BuildManager.cs

Line 1138 in 38eec6f

submission.CompleteLogging(true);

will block inside _syncLock and wait for all messages of logging service to be processed. Processing messages of logging service is performed on thread pool and it could callback BuildManager, for example:

msbuild/src/Build/BackEnd/BuildManager/BuildManager.cs

Lines 2815 to 2818 in 38eec6f

	private void OnProjectFinished(object sender, ProjectFinishedEventArgs e)
	{
	lock (_syncLock)
	{

Locking _syncLock in callback from LoggingService concludes this deadlock.

Prove:

Analysis

Code as is, is very fragile and hard to maintain. We shall strategically avoid, if possible, infinite lock-blocking callbacks from LoggingService into BuildManager
Proposed solution:

• Code in OnProjectStarting/Finishing seems to be dead. Nothing uses its results. I recommend to delete it and stop propagating project started and finished to BuildManager.
• OnThreadException
  • shall either somehow detect possible deadlock and throws with inner exception
  • or use lock timeout and throws with inner exception after timeout

Versions & Configurations

This error might have been there for ages, at least since 2018 commit 764fe79.