[LiveLogger] Many exceptions are thrown from the logger

[ladipro]

Issue Description

Throwing and catching exceptions has perf implications and generally makes debugging harder.

Steps to Reproduce

Run msbuild /ll under a debugger.

Expected Behavior

No exceptions.

Actual Behavior

Many occurrences of:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Build.Logging.LiveLogger.ProjectNode.Log() in C:\src\msbuild\src\MSBuild\LiveLogger\ProjectNode.cs:line 156

With an occasional:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at Microsoft.Build.Logging.LiveLogger.TerminalBuffer.Render() in C:\src\msbuild\src\MSBuild\LiveLogger\TerminalBuffer.cs:line 145

Analysis

No response

Versions & Configurations

Commit dfd8f41

[AR-May]

We need to check code for possible race conditions.
Found a race condition in line

msbuild/src/MSBuild/LiveLogger/LiveLogger.cs

Line 78 in dfd8f41

foreach (var project in projects)

The collection is modified in event handlers, for example

msbuild/src/MSBuild/LiveLogger/LiveLogger.cs

Line 145 in dfd8f41

projects[id] = node;

It is different from what @ladipro found, but it is probably for similar reasons.

[rainersigwald]

All of the data structures we update in the event handlers should be thread safe. We can probably just switch to ConcurrentDictionary for that particular case. For Lines in TerminalBuffer we probably need to lock (or a bigger refactor to avoid that concept).

Many occurrences of:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Build.Logging.LiveLogger.ProjectNode.Log() in C:\src\msbuild\src\MSBuild\LiveLogger\ProjectNode.cs:line 156

looks like we have been overzealous with ! . . .