Don't hang builds on BuildManager internal errors

[ladipro]

The logic to handle exceptions thrown on logging and communication threads was trying to make the build submission fail with the exception but it didn't always complete it. This manifested as the build hanging, for example when building with MSBUILDNODECONNECTIONTIMEOUT = 0, which is almost guaranteed to throw InternalErrorException in InstantiateNode.

This PR fixes it by making sure that all submissions are completed in OnThreadException. Unlike the original code, proper care is taken to avoid race conditions - i.e. the build result is mutated only if hasn't already been passed to the completion handler.

Fixes #5911

[rainersigwald]

Doesn't submission.BuildResult?.Exception == null mean the same thing as submission.BuildResult != null && submission.BuildResult.Exception == null? SharpLab shows an IL difference but it appears to just not be loading the BuildResult field twice in the ?. version.

[Forgind]

If submission.BuildResult is null, submission.BuildResult?.Exception is null; ? is like <thing before> == null ? null : <thing before>.<thing after>.

(I ran this.)

[ladipro]

With value type fields, one can write something like submission.BuildResult?.SomeFlag == false and this is equivalent to submission.BuildResult != null && submission.BuildResult.SomeFlag == false. But it doesn't work with reference type fields as the type of submission.BuildResult?.Exception is just Exception and if it evaluates to null, you can't tell which of the two is null.

[rainersigwald]

You're right; it's the null-to-bool comparison that saves the former case which doesn't apply here.

[ladipro]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ladipro]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).