dotnet / dotnet/msbuild

NRE under `BuildManager.CancelAllSubmissions`

Open
#14,359 0 comments 1 reaction 2 assignees Claimed by @rainersigwald View on GitHub
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

I was mucking with the debugger (so wildly changing process-start times/successes) and I started `dotnet build` on a single project, (did something debuggery or killed processes or somehthing, not sure exactly what), then canceled, and got this stack.

```
❯ dotnet build
MSBUILD : error MSB1025: An internal failure occurred while running MSBuild.
System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.Build.BackEnd.BuildComponentFactoryCollection.BuildComponentEntry.GetInstance(IBuildComponentHost host)
at Microsoft.Build.BackEnd.BuildComponentFactoryCollection.GetComponent(BuildComponentType type)
at Microsoft.Build.Execution.BuildManager.Microsoft.Build.BackEnd.IBuildComponentHost.get_LoggingService()
at Microsoft.Build.Execution.BuildManager.CancelAllSubmissions(Boolean async)
at Microsoft.Build.Experimental.OutOfProcServerNode.RunInternal(Exception& shutdownException, ServerNodeHandshake handshake)
at Microsoft.Build.Experimental.OutOfProcServerNode.Run(Exception& shutdownException)
at Microsoft.Build.CommandLine.MSBuildApp.StartLocalNode(CommandLineSwitches commandLineSwitches, Boolean lowpriority)
at Microsoft.Build.CommandLine.MSBuildApp.ProcessCommandLineSwitches(CommandLineSwitches switchesFromAutoResponseFile, CommandLineSwitches switchesNotFromAutoResponseFile, String& projectFile, String[]& targets, String& toolsVersion, Dictionary`2& globalProperties, ILogger[]& loggers, LoggerVerbosity& verbosity, LoggerVerbosity& originalVerbosity, List`1& distributedLoggerRecords, Int32& cpuCount, Boolean& multiThreaded, Boolean& enableNodeReuse, TextWriter& preprocessWriter, TextWriter& targetsWriter, Boolean& detailedSummary, ISet`1& warningsAsErrors, ISet`1& warningsNotAsErrors, ISet`1& warningsAsMessages, Boolean& enableRestore, Boolean& interactive, ProfilerLogger& profilerLogger, Boolean& enableProfiler, Dictionary`2& restoreProperties, ProjectIsolationMode& isolateProjects, GraphBuildOptions& graphBuild, String[]& inputResultsCaches, String& outputResultsCache, Boolean& lowPriority, Boolean& question, Boolean& isTaskInputLoggingRequired, Boolean& isBuildCheckEnabled, String[]& getProperty, String[]& getItem, String[]& getTargetResult, String& getResultOutputFile, Boolean recursing, String commandLine, Boolean switchesAlreadyGathered)
at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine, CommandLineSwitches preGatheredSwitchesFromAutoResponseFile, CommandLineSwitches preGatheredSwitchesNotFromAutoResponseFile)
```

Copilot analysis:

> **Likely cause — high confidence:** `BuildManager.CancelAllSubmissions` unconditionally resolves `LoggingService` before checking whether a build is active. Outside the active-build lifetime, the logging component is intentionally registered as a null singleton with no factory. `BuildComponentEntry.GetInstance` then calls that null factory, producing this exact NRE.
>
> The server creates two race windows: it starts the build with `Task.Run` while continuing to process packets, so a cancel can arrive before `BeginBuild` initializes logging; and `EndBuild` tears logging down before returning the manager to `Idle`, so a late cancel can arrive during teardown. The regression traces to `9dcc8cd8e5` (#10055), which added the unconditional `LogBuildCanceled()` call.
>
> **Proposed fix:** guard logging with the BuildManager lifecycle lock and only resolve it while the state is `Building`, while leaving the existing queued cancellation logic unchanged:
>
> ```csharp
> lock (_syncLock)
> {
> if (_buildManagerState == BuildManagerState.Building)
> {
> ((IBuildComponentHost)this).LoggingService.LogBuildCanceled();
> }
> }
> ```
>
> This preserves synchronous cancellation logging for active builds and prevents access before initialization or after teardown. I would not make component lookup nullable or catch `NullReferenceException`; that would hide lifecycle misuse. Add deterministic tests for cancellation while idle, after `EndBuild`, and during an active build (including exactly one `BuildCanceled` event), plus a server test sending build/cancel packets back-to-back. If that last test exposes a lost early cancellation, fix it separately by latching cancellation in `OutOfProcServerNode`; do not make `BuildManager` cancel future builds requested while idle.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.