From 2a5394e78d14d9783dfcf88b30bc2b28ad255636 Mon Sep 17 00:00:00 2001 From: Suneel Freimuth Date: Fri, 30 Oct 2026 14:01:14 -0700 Subject: [PATCH 1/2] Don't let one caller's cancellation abort a shared Syft run LinuxScanner deduplicates concurrent Syft runs per image, but the first caller ran the shared scan with its own cancellation token. When that caller was cancelled, the scan failed for every caller waiting on it. LinuxContainerDetector and the experimental LinuxApplicationLayer detector scan the same images, and experimental detectors are capped at 240 seconds. When the experiment started the shared run and Syft needed longer, its timeout cancelled the scan for LinuxContainerDetector as well, which then reported no Linux components for the image even though its own timeout had not expired. Which detector started the run was a race, so large images lost their inventory intermittently. The shared run now has its own cancellation token. A cancelled caller only stops waiting; the run is cancelled once no caller is left waiting for it, and a later caller starts a new run instead of joining one that is being cancelled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../linux/LinuxScanner.cs | 149 +++++++++++++-- .../LinuxScannerTests.cs | 174 ++++++++++++++++++ 2 files changed, 308 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs index 1ba407edb..44590d0fd 100644 --- a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs +++ b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs @@ -51,8 +51,11 @@ internal class LinuxScanner : ILinuxScanner /// Caches in-flight syft runs. /// When multiple detectors scan the same image concurrently, the second /// caller awaits the already-running task instead of launching a new container. + /// A shared run is cancelled only once every caller waiting on it has been cancelled, + /// so one caller's short timeout (such as an experimental detector's) can't abort + /// a scan that other callers still need. /// - private static readonly ConcurrentDictionary<(string Source, LinuxScannerScope Scope, string Binds, string Platform), Task> SyftRunCache = new(); + private static readonly ConcurrentDictionary<(string Source, LinuxScannerScope Scope, string Binds, string Platform), SharedSyftRun> SyftRunCache = new(); private static readonly int SemaphoreTimeout = Convert.ToInt32( TimeSpan.FromHours(1).TotalMilliseconds @@ -303,35 +306,71 @@ private async Task RunSyftAsync( { var bindsKey = string.Join(";", (additionalBinds ?? []).OrderBy(b => b, StringComparer.Ordinal)); var cacheKey = (syftSource, scope, bindsKey, platform ?? string.Empty); - var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var existingTask = SyftRunCache.GetOrAdd(cacheKey, tcs.Task); - if (existingTask != tcs.Task) + while (true) { - // Another caller is already running syft for this image+scope — await their result, - // but allow this caller's cancellation token to abort the wait. - this.logger.LogDebug("Syft run for {SyftSource} (scope={Scope}) is already in-flight, reusing existing result", syftSource, scope); - return await existingTask.WaitAsync(cancellationToken); + var candidate = new SharedSyftRun(); + var run = SyftRunCache.GetOrAdd(cacheKey, candidate); + + if (!run.TryAddCaller()) + { + // Every caller of this run has been cancelled and it is being torn down; replace it. + SyftRunCache.TryRemove(KeyValuePair.Create(cacheKey, run)); + continue; + } + + if (ReferenceEquals(run, candidate)) + { + // We own this cache entry, so start the run. It uses the run's own token rather than + // this caller's, so the other callers still get the result if this caller is cancelled. + _ = this.ExecuteSharedSyftRunAsync(cacheKey, run, syftSource, scope, additionalBinds ?? [], platform, record, syftTelemetryRecord); + } + else + { + // Another caller is already running syft for this image+scope — await their result, + // but allow this caller's cancellation token to abort the wait. + this.logger.LogDebug("Syft run for {SyftSource} (scope={Scope}) is already in-flight, reusing existing result", syftSource, scope); + } + + try + { + return await run.Result.WaitAsync(cancellationToken); + } + finally + { + run.RemoveCaller(); + } } + } - // We own this cache entry — run syft and propagate the result. + /// + /// Runs syft for a shared cache entry and publishes the result to every caller waiting on it. + /// + private async Task ExecuteSharedSyftRunAsync( + (string Source, LinuxScannerScope Scope, string Binds, string Platform) cacheKey, + SharedSyftRun run, + string syftSource, + LinuxScannerScope scope, + IList additionalBinds, + string? platform, + LinuxScannerTelemetryRecord record, + LinuxScannerSyftTelemetryRecord syftTelemetryRecord) + { try { - var result = await this.RunSyftCoreAsync(syftSource, scope, additionalBinds ?? [], platform, record, syftTelemetryRecord, cancellationToken); - tcs.SetResult(result); - return result; + var result = await this.RunSyftCoreAsync(syftSource, scope, additionalBinds, platform, record, syftTelemetryRecord, run.CancellationToken); + run.SetResult(result); } catch (Exception ex) { - tcs.SetException(ex); - throw; + run.SetException(ex); } finally { // Remove the entry once complete. The cache only deduplicates concurrent // in-flight calls — keeping completed entries would leak memory for the // lifetime of the process. - SyftRunCache.TryRemove(cacheKey, out _); + SyftRunCache.TryRemove(KeyValuePair.Create(cacheKey, run)); } } @@ -533,4 +572,84 @@ private static bool IsPackageManagerDatabasePath(string path) // Cover any file under /var/lib/rpm/ (RPM database can use multiple files) return path.StartsWith("/var/lib/rpm/", StringComparison.OrdinalIgnoreCase); } + + /// + /// A syft run shared by the callers that scan the same source concurrently. + /// + private sealed class SharedSyftRun + { + private readonly object gate = new(); + + // Not disposed: it never owns a timer or wait handle, and disposing it could race with + // the Cancel issued by the last caller to stop waiting. + private readonly CancellationTokenSource cancellationTokenSource = new(); + + private readonly TaskCompletionSource completionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); + + private int callers; + + private bool abandoned; + + /// + /// Gets the task that completes with the syft output. + /// + public Task Result => this.completionSource.Task; + + /// + /// Gets the token that cancels the run once no caller is waiting for it. + /// + public CancellationToken CancellationToken => this.cancellationTokenSource.Token; + + /// + /// Registers a caller that will wait for the result. + /// + /// false if every earlier caller has stopped waiting and the run is being cancelled. + public bool TryAddCaller() + { + lock (this.gate) + { + if (this.abandoned) + { + return false; + } + + this.callers++; + return true; + } + } + + /// + /// Unregisters a caller, and cancels the run if no caller is left waiting for an unfinished result. + /// + public void RemoveCaller() + { + lock (this.gate) + { + this.callers--; + if (this.callers > 0 || this.Result.IsCompleted) + { + return; + } + + this.abandoned = true; + } + + this.cancellationTokenSource.Cancel(); + } + + public void SetResult(string stdout) => this.completionSource.TrySetResult(stdout); + + public void SetException(Exception exception) + { + if (exception is OperationCanceledException && this.cancellationTokenSource.IsCancellationRequested) + { + // Nobody is waiting; complete as cancelled so the exception isn't reported as unobserved. + this.completionSource.TrySetCanceled(this.cancellationTokenSource.Token); + } + else + { + this.completionSource.TrySetException(exception); + } + } + } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs index 7db7f5390..fe8bf1441 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs @@ -1545,6 +1545,180 @@ public async Task TestLinuxScanner_CancelledCaller_DoesNotBlockOnInFlightSyftRun result1.Should().NotBeEmpty(); } + [TestMethod] + public async Task TestLinuxScanner_CancelledOwner_DoesNotCancelInFlightRunForOtherCallersAsync() + { + LinuxScanner.ResetCache(); + + // Like the real Docker service, the mock stops the container when its token is cancelled. + var syftCompletionSource = new TaskCompletionSource<(string, string)>(); + + this.mockDockerService.Setup(service => + service.CreateAndRunContainerAsync( + It.IsAny(), + It.IsAny>(), + It.IsAny>(), + It.IsAny() + ) + ) + .Returns((string _, IList _, IList _, CancellationToken token) => + syftCompletionSource.Task.WaitAsync(token)); + + var enabledTypes = new HashSet { ComponentType.Linux }; + + var layers = new[] + { + new DockerLayer + { + LayerIndex = 0, + DiffId = "sha256:f95fc50d21d981f1efe1f04109c2c3287c271794f5d9e4fdf9888851a174a971", + }, + }; + + var scanner1 = new LinuxScanner( + this.mockDockerService.Object, + this.mockLogger.Object, + this.componentFactories, + this.artifactFilters + ); + var scanner2 = new LinuxScanner( + this.mockDockerService.Object, + this.mockLogger.Object, + this.componentFactories, + this.artifactFilters + ); + + // The first caller starts the syft run with a short deadline, like an experimental detector. + using var ownerCts = new CancellationTokenSource(); + var ownerTask = scanner1.ScanLinuxAsync("owner_cancel_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers, ownerCts.Token); + + // The second caller joins the same in-flight run without a deadline. + var waiterTask = scanner2.ScanLinuxAsync("owner_cancel_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers); + + await ownerCts.CancelAsync(); + + try + { + await ownerTask; + Assert.Fail("Expected OperationCanceledException was not thrown"); + } + catch (OperationCanceledException) + { + // Expected — the owner stopped waiting for the run. + } + + // The run must keep going for the caller that is still waiting. + waiterTask.IsCompleted.Should().BeFalse(); + + syftCompletionSource.SetResult((SyftOutputNoAuthorOrLicense, string.Empty)); + var result = await waiterTask; + result.Should().NotBeEmpty(); + result.First().Components.Should().ContainSingle(); + + this.mockDockerService.Verify( + service => + service.CreateAndRunContainerAsync( + It.IsAny(), + It.IsAny>(), + It.IsAny>(), + It.IsAny() + ), + Times.Once + ); + } + + [TestMethod] + public async Task TestLinuxScanner_AllCallersCancelled_CancelsInFlightSyftRunAsync() + { + LinuxScanner.ResetCache(); + + var firstRun = new TaskCompletionSource<(string, string)>(); + var runTokens = new List(); + + this.mockDockerService.Setup(service => + service.CreateAndRunContainerAsync( + It.IsAny(), + It.IsAny>(), + It.IsAny>(), + It.IsAny() + ) + ) + .Returns((string _, IList _, IList _, CancellationToken token) => + { + lock (runTokens) + { + runTokens.Add(token); + return runTokens.Count == 1 + ? firstRun.Task.WaitAsync(token) + : Task.FromResult((SyftOutputNoAuthorOrLicense, string.Empty)); + } + }); + + var enabledTypes = new HashSet { ComponentType.Linux }; + + var layers = new[] + { + new DockerLayer + { + LayerIndex = 0, + DiffId = "sha256:f95fc50d21d981f1efe1f04109c2c3287c271794f5d9e4fdf9888851a174a971", + }, + }; + + var scanner1 = new LinuxScanner( + this.mockDockerService.Object, + this.mockLogger.Object, + this.componentFactories, + this.artifactFilters + ); + var scanner2 = new LinuxScanner( + this.mockDockerService.Object, + this.mockLogger.Object, + this.componentFactories, + this.artifactFilters + ); + + using var cts1 = new CancellationTokenSource(); + using var cts2 = new CancellationTokenSource(); + var task1 = scanner1.ScanLinuxAsync("abandoned_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers, cts1.Token); + var task2 = scanner2.ScanLinuxAsync("abandoned_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers, cts2.Token); + + // One caller gives up: the container keeps running for the other. + await cts1.CancelAsync(); + try + { + await task1; + Assert.Fail("Expected OperationCanceledException was not thrown"); + } + catch (OperationCanceledException) + { + // Expected — the first caller stopped waiting. + } + + runTokens.Should().ContainSingle(); + runTokens[0].IsCancellationRequested.Should().BeFalse(); + + // The last caller gives up: nobody needs the result, so the container is stopped. + await cts2.CancelAsync(); + try + { + await task2; + Assert.Fail("Expected OperationCanceledException was not thrown"); + } + catch (OperationCanceledException) + { + // Expected — the second caller stopped waiting. + } + + runTokens[0].IsCancellationRequested.Should().BeTrue(); + + // A later caller starts a new run instead of joining the cancelled one. + var result = await this.linuxScanner.ScanLinuxAsync("abandoned_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers); + result.Should().NotBeEmpty(); + result.First().Components.Should().ContainSingle(); + runTokens.Should().HaveCount(2); + } + [TestMethod] public void TestLinuxScanner_ProcessSyftOutput_ExcludesPackageManagerDatabasePathsFromLayerAttribution() { From 5bed72b4ea23721799ae22f51a5e75889985582f Mon Sep 17 00:00:00 2001 From: Suneel Freimuth Date: Wed, 4 Nov 2026 23:05:53 -0800 Subject: [PATCH 2/2] Wait for a cancelled shared Syft run to remove its container When the last caller waiting on a shared Syft run gave up, it cancelled the run and returned straight away. The Syft container is removed only after the run observes the cancellation, so the process could exit while the container was still running. Before runs had their own token, the caller that ran Syft awaited that removal itself. The caller that cancels the run now waits for it to stop. The run's result is set only after its container has been removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../linux/LinuxScanner.cs | 20 +++++- .../LinuxScannerTests.cs | 65 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs index 44590d0fd..b9718fc64 100644 --- a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs +++ b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs @@ -338,7 +338,12 @@ private async Task RunSyftAsync( } finally { - run.RemoveCaller(); + if (run.RemoveCaller()) + { + // This caller was the last one waiting, so it cancelled the run. Wait for the run to stop, + // so its container is removed before this scan returns. + await run.WaitForStopAsync(); + } } } } @@ -621,22 +626,31 @@ public bool TryAddCaller() /// /// Unregisters a caller, and cancels the run if no caller is left waiting for an unfinished result. /// - public void RemoveCaller() + /// true if this caller was the last one waiting and cancelled the run. + public bool RemoveCaller() { lock (this.gate) { this.callers--; if (this.callers > 0 || this.Result.IsCompleted) { - return; + return false; } this.abandoned = true; } this.cancellationTokenSource.Cancel(); + return true; } + /// + /// Waits for the run to stop, whatever its outcome. The result is set only after syft's container is removed. + /// + /// A task that completes when the run has stopped. + public Task WaitForStopAsync() => + this.Result.ContinueWith(static _ => { }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); + public void SetResult(string stdout) => this.completionSource.TrySetResult(stdout); public void SetException(Exception exception) diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs index fe8bf1441..65f132417 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs @@ -1719,6 +1719,71 @@ public async Task TestLinuxScanner_AllCallersCancelled_CancelsInFlightSyftRunAsy runTokens.Should().HaveCount(2); } + [TestMethod] + public async Task TestLinuxScanner_LastCallerCancelled_WaitsForContainerRemovalAsync() + { + LinuxScanner.ResetCache(); + + // Like the real Docker service, the mock removes the container before returning when it's cancelled. + var removalStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var removalFinished = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + this.mockDockerService.Setup(service => + service.CreateAndRunContainerAsync( + It.IsAny(), + It.IsAny>(), + It.IsAny>(), + It.IsAny() + ) + ) + .Returns(async (string _, IList _, IList _, CancellationToken token) => + { + try + { + await Task.Delay(Timeout.Infinite, token); + return (SyftOutputNoAuthorOrLicense, string.Empty); + } + finally + { + removalStarted.SetResult(); + await removalFinished.Task.WaitAsync(TimeSpan.FromSeconds(30), CancellationToken.None); + } + }); + + var enabledTypes = new HashSet { ComponentType.Linux }; + + var layers = new[] + { + new DockerLayer + { + LayerIndex = 0, + DiffId = "sha256:f95fc50d21d981f1efe1f04109c2c3287c271794f5d9e4fdf9888851a174a971", + }, + }; + + using var cts = new CancellationTokenSource(); + var scanTask = this.linuxScanner.ScanLinuxAsync("removal_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers, cts.Token); + + // The only caller gives up, which cancels the run and starts removing the container. + await cts.CancelAsync(); + await removalStarted.Task.WaitAsync(TimeSpan.FromSeconds(30)); + + // The scan doesn't return while the container is still being removed. + var firstToFinish = await Task.WhenAny(scanTask, Task.Delay(TimeSpan.FromMilliseconds(200))); + firstToFinish.Should().NotBeSameAs(scanTask); + + removalFinished.SetResult(); + try + { + await scanTask; + Assert.Fail("Expected OperationCanceledException was not thrown"); + } + catch (OperationCanceledException) + { + // Expected — the caller stopped waiting. + } + } + [TestMethod] public void TestLinuxScanner_ProcessSyftOutput_ExcludesPackageManagerDatabasePathsFromLayerAttribution() {