TaskAnalyzer: no diagnostic for unsynchronized shared mutable state in [MSBuildMultiThreadableTask] types

[ViktorHofer]

Issue Description

[MSBuildMultiThreadableTask] is an assertion by the task author that the task is safe to run concurrently with other tasks in the same process. The analyzer verifies the ambient process state half of that contract (environment, CWD, Process.Start, temp files), but nothing verifies the shared mutable state half.

Unsynchronized static state is not mentioned anywhere in #14772. It is, however, the failure mode that produces genuinely nondeterministic corruption rather than a deterministic wrong answer, and it caused three real races in the Arcade migration (dotnet/arcade#17381).

Steps to Reproduce

[MSBuildMultiThreadableTask]
public sealed class SharedStateTask : Task, IMultiThreadableTask
{
    public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;

    private static readonly Dictionary<string, string> s_cache = new Dictionary<string, string>();
    private static readonly Random s_random = new Random();
    private static HttpClient s_client;

    public override bool Execute()
    {
        s_cache["k"] = "v";              // unsynchronized write to a static Dictionary
        s_client = new HttpClient();     // overwrites a process-wide field
        Log.LogMessage(s_random.Next().ToString() + s_cache.Count + s_client.Timeout);
        return true;
    }
}

Build with msbuild_task_analyzer.scope = multithreadable_only and --no-incremental.

Expected Behavior

At minimum a diagnostic on the s_cache["k"] = "v" write: mutating a non-thread-safe static collection from a type asserted to be multithreadable. Ideally also on the static field assignment and on Random, which is documented as not thread-safe for instance members.

Actual Behavior

No diagnostic. The only thing reported for this type is MSBuildTask0011 (constructor injection).

Real-World Cases

All three were found by manual review, not by the analyzer, and all three are in code that the analyzer otherwise reported as clean:

1. TargetFrameworkResolver.CreateOrGet — a static readonly Dictionary used as a memoization cache, read and written with no synchronization. Concurrent Execute calls can tear the bucket array. Fixed by switching to ConcurrentDictionary.GetOrAdd.
2. GetPackageDescription — shared a non-thread-safe NuGet object across invocations via a cached index.
3. GetCompatiblePackageTargetFrameworks — same shape.

Case 1 is representative of a very common idiom in build tasks: a static memoization cache added at some point for performance, in code that was single-threaded-by-construction for its entire life until the attribute was applied.

Suggested Fix

A new diagnostic, reported only on types carrying [MSBuildMultiThreadableTask] (so it is opt-in and cannot regress existing task code), covering:

1. Writes to static fields, and mutating calls on static fields of known non-thread-safe types (Dictionary<,>, List<>, HashSet<>, StringBuilder, Random, XmlDocument, …), when not obviously guarded by a lock in the same method.
2. Writes to static fields of any type from within Execute or anything it reaches.
3. Optionally, a "possibly-shared state" informational diagnostic on static fields of types from referenced assemblies, which cannot be judged locally.

Suppression is easy for the intentional cases (lock, Lazy<T>, ConcurrentDictionary, Interlocked, immutable collections, readonly value types), so the rule should be tractable without excessive noise.

Even a limited version restricted to point 1 — mutating a static Dictionary/List/HashSet outside a lock — would have caught all three Arcade cases.

This may be considered out of scope for a task-authoring analyzer, since it is a general concurrency concern. Filing it because the attribute is exactly the signal that makes the rule cheap and precise: it tells the analyzer which types the author has promised are concurrency-safe, and therefore which types are worth checking.

Versions & Configurations

• Microsoft.Build.TaskAuthoring.Analyzer 18.11.0-1.26420.118
• Microsoft.Build.Utilities.Core 18.8.2
• SDK 11.0.0-preview.6.26359.118

Related

• Roslyn analyzers for MSBuild tasks #14772, Multithreading: docs and analyzer should reflect that [MSBuildMultiThreadableTask] is the routing signal, not IMultiThreadableTask #14779, TaskAnalyzer: MSBuildTask0003 code fix produces non-compiling code for nested calls and static methods #14780
• dotnet/arcade#17378, dotnet/arcade#17381