Comparing changes

…proc node (#13445) ## Summary Related to https://github.com/dotnet/msbuild/pull/13332/changes `HandleBuildResultAsync` in `ProjectCacheService` crashes with `InternalErrorException("Project unexpectedly null")` when a build result arrives for a configuration whose `ProjectInstance` was never loaded on the in-proc node. ## Root Cause In VS (and global plugin) scenarios, `ShouldUseCache` returns `true` without checking `IsLoaded`. This is by design - it's needed for the **pre-build** cache request path (`BuildManager.ExecuteSubmission`), where the project hasn't been evaluated yet and `PostCacheRequest` loads it via `EvaluateProjectIfNecessary`. However, `ShouldUseCache` is also called from the **post-build** path (`BuildManager.HandleResult`) to decide whether to notify cache plugins about the result. When a project was built on an out-of-proc worker node, the in-proc config cache entry never gets a `ProjectInstance` assigned - `ProjectInstance` is intentionally not transferred back from worker nodes because it's too large to serialize across the named pipe (only `BuildResult` with target outcomes is returned). This means `HandleBuildResultAsync` is called with a configuration where `IsLoaded` is false and accessing `.Project` would crash. ## Fix Replace the `VerifyThrowInternalNull(requestConfiguration.Project)` assert with an `!requestConfiguration.IsLoaded` early return. **Why `IsLoaded` instead of `Project == null`**: The `Project` getter has an internal assert (`!IsCached`) that throws if the configuration happens to be in cached-to-disk state. `IsLoaded` is a safe read-only check (`_project?.IsLoaded == true`) with no side effects or asserts. It also correctly handles the edge case of a partially-initialized `ProjectInstance` (deserialized but not yet `LateInitialize`'d). Skipping is safe because: - The build has already completed successfully at this point - The cache plugin's `HandleProjectFinishedAsync` requires a loaded `ProjectInstance` to determine applicable plugins and provide project context - No build correctness is affected — only the cache plugin notification is skipped for this configuration ## Validation Diagnostic telemetry (`ProjectCacheState`) deployed in a prior iteration confirmed the root cause across multiple VS repo users: - `IsLoaded=False`, `IsCached=False`, `WasGeneratedByNode=False`, `IsVsScenario=True` - Configuration created from `BuildRequestData` without `ProjectInstance` (VS submits by path) - Project built on out-of-proc worker node, `ProjectInstance` never transferred back

## Thread-Safe Task Analyzer Adds a Roslyn analyzer that detects unsafe API usage in MSBuild task implementations, guiding task authors toward thread-safe patterns required for multithreaded task execution (`IMultiThreadableTask`). ### Diagnostic Rules | ID | Default Severity | Title | |----|-----------------|-------| | **MSBuildTask0001** | Error | API is never safe in MSBuild tasks (Console.*, Environment.Exit, ThreadPool, etc.) | | **MSBuildTask0002** | Warning / Info ¹ | API requires TaskEnvironment alternative | | **MSBuildTask0003** | Warning / Info ¹ | File system API requires absolute path | | **MSBuildTask0004** | Warning / Info ¹ | API may cause issues (Assembly.Load, Activator, etc.) | | **MSBuildTask0005** | Warning / Info ¹ | Transitive unsafe API in call chain (source-level BFS) | ¹ Warning for `IMultiThreadableTask` / `[MSBuildMultiThreadableTask]`, Info for plain `ITask`. Users can configure via `.editorconfig`. ### What's included - **Core analyzer** (`MultiThreadableTaskAnalyzer`): Per-type scoped analysis via `RegisterSymbolStartAction` - **Transitive analyzer** (`TransitiveCallChainAnalyzer`): Source-level BFS call graph traces unsafe APIs through helper methods - **Code fix provider**: Auto-fixes for MSBuildTask0002 (TaskEnvironment replacements) and MSBuildTask0003 (`GetAbsolutePath` wrapping) - **Shared helpers** (`SharedAnalyzerHelpers`): Deduplicated path safety analysis and banned API resolution - **109 unit tests** passing ### Build integration - Added to `MSBuild.slnx` - `/p:BuildAnalyzer=true` enables analyzer on Tasks.csproj - Produces 56 warnings on Tasks project (34 env + 12 path + 10 transitive), 0 false positives ### Safe pattern recognition (zero false positives verified) - `ITaskItem.GetMetadata("FullPath")` - `Path.Combine(absolutePath, ...)` - `Path.GetDirectoryName(absolutePath)` - `AbsolutePath` type (implicit conversion) - `File.WriteAllText(path, contents)` — only path param flagged ### What's NOT included (preserved in `threadsafe-analyzer` branch) - IL decompiler (`ILCallGraphExtender.cs`) — 1,011 lines of raw IL byte parsing with 6 verified opcode bugs - Demo project (~1,600 lines) - Cross-assembly IL tests (~870 lines) ### Testing - Stress-tested on MSBuild Tasks project, external task libraries, and SDK-style task patterns - Zero false positives across all test scenarios --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

### Context With the discovery of the "direct instantiation of the task" use case, tasks must explicitly set a default task environment in order to work correctly. Task authors may encounter the same issue, without having benefit of having framework internals visible to them and thus without ability to create one without re-implementing the driver themselves. So, we need to provide a public API for `TaskEnvironment` construction to enable setting the default task environment. Would be helpful for sdk as well. ### Changes Made Adds two public API members to `TaskEnvironment` so that task authors using `IMultiThreadableTask` can obtain or create a `TaskEnvironment` without depending on internal types: - `TaskEnvironment.Default`: static singleton that delegates to the process-level environment (multi-process mode). - `TaskEnvironment.CreateMultiThreaded(projectDirectory, environmentVariables?)`: factory method that creates an isolated environment for multithreaded execution mode. ### Testing Unit tests ### Note Depends on #13380

Fixes #13451 ### Context Version bump check didn't work. This change should fix that. ### Changes Made Node selection logic was changed. ### Testing Change tested in release branch: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1355155&view=logs&j=6fe8bf02-8a7a-56a7-e3fd-913e2045a877&t=e87b56af-cb98-594d-acd9-14509c9c5689 ### Notes --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Adds ETW instrumentation around the node create-or-connect path in `NodeProviderOutOfProcBase.GetNodes`, to help chase down a possible regression from #13175. ## New ETW events | Event | ID | Parameters | Description | |---|---|---|---| | `NodeConnectStart` | 99 | `nodeId` | Begin acquiring a node (reuse or launch) | | `NodeConnectStop` | 100 | `nodeId`, `processId`, `isReused` | Node acquired successfully | | `NodeReuseScanStart` | 101 | | Begin scanning for reusable node processes | | `NodeReuseScanStop` | 102 | `candidateCount` | Scan complete | | `NodeLaunchStart` | 103 | `nodeId` | Begin OS process creation | | `NodeLaunchStop` | 104 | `nodeId`, `processId` | Process created | | `NodePipeConnectStart` | 105 | `nodeId`, `processId` | Begin named pipe connect + handshake | | `NodePipeConnectStop` | 106 | `nodeId`, `processId`, `success` | Pipe connect complete | ## Timeline (new node) ``` NodeReuseScanStart / NodeReuseScanStop(candidates) NodeConnectStart(nodeId) NodeLaunchStart(nodeId) NodeLaunchStop(nodeId, pid) NodePipeConnectStart(nodeId, pid) NodePipeConnectStop(nodeId, pid, success) NodeConnectStop(nodeId, pid, isReused=false) ``` ## Timeline (reused node) ``` NodeReuseScanStart / NodeReuseScanStop(candidates) NodeConnectStart(nodeId) NodePipeConnectStart(nodeId, candidatePid) NodePipeConnectStop(nodeId, candidatePid, success) NodeConnectStop(nodeId, pid, isReused=true) ``` Both `NodeProviderOutOfProc` and `NodeProviderOutOfProcTaskHost` flow through `GetNodes`. No behavioral change. Pure observability addition. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Jan Provazník <janpro@janpro.dev> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…os (#13452) PR #13175 introduced the MSBuild AppHost (`MSBuild.exe`). After this change, `dotnet build` spawns worker nodes as `MSBuild.exe` processes. ETL trace analysis of OrchardCore NuGet restore shows a **~16% regression** (61s -> 72s): The root cause of the performance difference is not fully understood. Reverting worker nodes to `dotnet MSBuild.dll` in CLI scenarios restores baseline performance.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comparing changes

Open a pull request

Commits on Mar 30, 2026

This comparison is taking too long to generate.

Uh oh!