Fix thread-unsafe Dictionary in telemetry (#12867)

[JanProvaznik]

Summary

Fixes #12867 — Dictionary corruption crash in telemetry when building with /m /mt.

Problem

In out-of-proc mode (/m only), each worker node runs in a separate process with its own TelemetryForwarderProvider singleton, so its WorkerNodeTelemetryData dictionaries are only accessed by one thread. At end-of-build, each node sends a single WorkerNodeTelemetryEventArgs message and the main node merges them one at a time. No contention, no problem.

In in-proc multithreaded mode (/m /mt), all in-proc nodes share a single process and therefore a single TelemetryForwarderProvider singleton. Multiple RequestBuilder instances run on dedicated threads (DedicatedThreadsTaskScheduler) and call AddTask/AddTarget concurrently on the same Dictionary<> fields. This causes Dictionary.Resize corruption — an ArgumentException or silent data loss on every build with enough parallelism.

Reproduction: 20 non-SDK .NET Framework library projects + 1 exe referencing all of them, built with MSBuild.exe Repro.sln /m /mt. Crashed 12/12 times before this fix.

Approach

Rather than switching to ConcurrentDictionary (which has per-operation stripe-lock overhead and requires AddOrUpdate with lambda allocations for the check-then-act pattern), this PR makes the /mt path match the OOP architecture:

Before: RequestBuilder.UpdateStatisticsPostBuild called telemetryForwarder.AddTask()/AddTarget() once per target and once per task — each call individually hitting the shared singleton dictionary. For a project with 300 targets and 15 tasks, that was 315 lock-needing mutations on the shared state per project.

After: UpdateStatisticsPostBuild creates a local WorkerNodeTelemetryData, accumulates all targets and tasks into it (zero contention — single owner, no lock needed), then calls telemetryForwarder.MergeWorkerData(localData) once. This does a single Add() to merge into the shared singleton under one lock acquisition. For 32 projects, that is 32 lock acquisitions total.

Additionally, WorkerNodeTelemetryData.AddTask/AddTarget/Add are now protected by lock to guard the merge path and any remaining direct callers. ProjectTelemetry._msbuildTaskSubclassUsage gets the same treatment.

Changes

File	Change
WorkerNodeTelemetryData.cs	Added lock around AddTask/AddTarget/Add; extracted private Unsafe variants to avoid nested locking
ProjectTelemetry.cs	Added lock around _msbuildTaskSubclassUsage dictionary access
ITelemetryForwarder.cs	Added MergeWorkerData(IWorkerNodeTelemetryData) to interface
TelemetryForwarderProvider.cs	Implemented MergeWorkerData in TelemetryForwarder and NullTelemetryForwarder
RequestBuilder.cs	Refactored UpdateStatisticsPostBuild to local-accumulate + single merge; constructs TaskOrTargetTelemetryKey directly
WorkerNodeTelemetryData_Tests.cs	3 new concurrency regression tests (AddTask, AddTarget, Add merge) using 20 threads + Barrier

Verification

Scenario	Before	After
E2E: 20-project /m /mt incremental build × 20 runs	12/12 crashes	20/20 pass
Unit tests: concurrent AddTask/AddTarget/Add	N/A	3/3 pass

[JanProvaznik]

@AR-May it's assigned on you and this looks promising. ptal if you want to use this or be inspired how it could be fixed

[JanProvaznik]

Suggested change

	private readonly object _subclassUsageLock = new();
	private readonly LockType _subclassUsageLock = new();

[JanProvaznik]

Suggested change

	private readonly object _lock = new();
	private readonly LockType _lock = new();