Fix ObjectDisposedException in GenerateSbom Task under MSBuild Server - #1529
Open
Chet Husk (baronfel) wants to merge 1 commit into
Open
Fix ObjectDisposedException in GenerateSbom Task under MSBuild Server#1529Chet Husk (baronfel) wants to merge 1 commit into
Chet Husk (baronfel) wants to merge 1 commit into
Conversation
Register an invocation-local null-output IAnsiConsole so component detection cannot retain a disposed process Console.Out writer. Add repeated in-process generation coverage that verifies detected package data survives server reuse. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f64399fe-4312-4d2f-94af-9044fb5cf135
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes an
ObjectDisposedExceptioninMicrosoft.Sbom.Targets.GenerateSbomwhen running under a reused MSBuild Server process (e.g., MSBuild Server node reuse across multiple builds in the same VS/CLI session).Root cause, upstream context, and fix rationale are tracked in dotnet/msbuild#14691.
Fixes #712
Root cause
GenerateSbombuilds a fresh DI container on every task invocation viaAddSbomTool(), which calls ComponentDetection'sAddComponentDetection(). ComponentDetection'sDetectorProcessingServicedoes not receive an explicitSpectre.Console.IAnsiConsolefrom the DI container, so it falls back to the process-wide staticAnsiConsole.Consolesingleton, which is bound to whateverConsole.Outwas current the first time it was touched in the process.This is a cardinal sin of MSBuild Tasks - they should not rely on ambient/environmental state. They should only interact with the build environment through the APIs/mechanisms provided by the IBuildEngineX APIs or the new TaskEnvironment contextual parameter.
Under MSBuild Server reuse, build 1 sets up a
RedirectConsoleWriteraroundConsole.Outand disposes it when build 1 finishes. Build 2 reuses the same process; ComponentDetection still holds the (now-disposed) writer via the staticAnsiConsole.Consolesingleton, and any call intoDetectorProcessingService.LogTabularOutput(which renders the "Detection Summary" table via Spectre) throwsObjectDisposedException, producing empty/failed SBOM generation on the second and subsequent invocations in the same process.Fix
AddSbomTool()now registers a local, non-staticIAnsiConsolein the service collection before callingAddComponentDetection(), so ComponentDetection resolves this instance instead of falling back to the static singleton. The console is created viaAnsiConsole.Create(new AnsiConsoleSettings { Out = new AnsiConsoleOutput(TextWriter.Null) })— a fresh, per-invocation instance backed byTextWriter.Nullrather than the process'sConsole.Out.This is intentional and low-risk: the only thing ComponentDetection renders through
IAnsiConsoleis a decorative "Detection Summary" table (gated by!settings.NoSummary) inDetectorProcessingService.LogTabularOutput. The same data is already independently emitted throughILogger, andServiceCollectionExtensions.CreateLogger()in this repo already excludes that verbose per-detector timing/summary data from the console sink on purpose (it's meant for the log file only). Discarding the Spectre summary table viaTextWriter.Nulltherefore has no effect on the actual SBOM manifest, detected packages/files, or logged output — only on a summary table that was already suppressed from the console in this tool's logging configuration.Testing
Microsoft.Sbom.Extensions.DependencyInjection.Testsassert thatAddSbomTool()resolves anIAnsiConsolefrom the container that is not the staticAnsiConsole.Consolesingleton, across repeated container builds.Microsoft.Sbom.Targets.TestsinvokeGenerateSbomtwice in-process, replacing and disposingConsole.Outbetween invocations (simulating MSBuild Server's build-boundaryRedirectConsoleWriterdisposal). Both invocations succeed and produce a valid manifest with non-empty package data, on bothnet8.0andnet472. Before the fix, the second invocation throwsObjectDisposedException.dotnet buildanddotnet teston the affected projects).Files changed
src/Microsoft.Sbom.Extensions.DependencyInjection/ServiceCollectionExtensions.cs— register invocation-localIAnsiConsolebeforeAddComponentDetection().src/Microsoft.Sbom.Extensions.DependencyInjection/Microsoft.Sbom.Extensions.DependencyInjection.csproj— explicitSpectre.Consolepackage reference (central package management).test/Microsoft.Sbom.Extensions.DependencyInjection.Tests/ServiceCollectionExtensionsTests.cs— new DI resolution regression tests.test/Microsoft.Sbom.Targets.Tests/AbstractGenerateSbomTaskTests.cs— new repeated in-process invocation regression test.test/Microsoft.Sbom.Targets.Tests/GeneratedSbomValidator.cs— new lightweight package-data validator assertion used by the regression test.Closes/relates to dotnet/msbuild#14691.