diff --git a/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs b/src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs index 1ba407edb..b9718fc64 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,76 @@ 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 + { + 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(); + } + } } + } - // 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 +577,93 @@ 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. + /// + /// 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 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) + { + 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..65f132417 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs @@ -1545,6 +1545,245 @@ 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 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() {