pr feedback

Author: pauld-msft

src/Microsoft.ComponentDetection.Common/DockerService.cs

@@ -117,9 +117,9 @@ public async Task<bool> TryPullImageAsync(string image, CancellationToken cancel
 
         if (existingTask != tcs.Task)
         {
-            // Another caller is already pulling this image — await their result.
+            // Another caller is already pulling this image — await their result
             this.logger.LogDebug("Image {Image} is already being pulled by another caller, waiting", image);
-            return await existingTask;
+            return await existingTask.WaitAsync(cancellationToken);
         }
 
         // We own this cache entry — perform the actual pull.
@@ -128,6 +128,12 @@ public async Task<bool> TryPullImageAsync(string image, CancellationToken cancel
             this.logger.LogDebug("Pulling image {Image}...", image);
             var result = await this.PullImageCoreAsync(image, cancellationToken);
             this.logger.LogDebug("Pull of image {Image} completed (success={Success})", image, result);
+            if (!result)
+            {
+                // Non-exception failure — remove the entry so later callers can retry.
+                PullCache.TryRemove(image, out _);
+            }
+
             tcs.SetResult(result);
             return result;
         }

src/Microsoft.ComponentDetection.Detectors/linux/LinuxScanner.cs

@@ -33,7 +33,7 @@ internal class LinuxScanner : ILinuxScanner
     private static readonly SemaphoreSlim ContainerSemaphore = new SemaphoreSlim(2);
 
     /// <summary>
-    /// Caches in-flight and completed syft runs keyed by (source, scope).
+    /// Caches in-flight and completed syft runs.
     /// When multiple detectors scan the same image concurrently, the second
     /// caller awaits the already-running task instead of launching a new container.
     /// </summary>
@@ -261,8 +261,7 @@ private IEnumerable<LayerMappedLinuxComponents> ProcessSyftOutputWithTelemetry(
 
     /// <summary>
     /// Runs the Syft scanner container and returns the stdout output.
-    /// Results are cached by (source, scope, binds) so that concurrent callers
-    /// with identical parameters share a single container run.
+    /// Results are cached so that callers with identical parameters share a single container run.
     /// </summary>
     private async Task<string> RunSyftAsync(
         string syftSource,
@@ -279,8 +278,9 @@ private async Task<string> RunSyftAsync(
 
         if (existingTask != tcs.Task)
         {
-            // Another caller is already running syft for this image+scope — await their result.
-            return await existingTask;
+            // Another caller is already running syft for this image+scope — await their result,
+            // but allow this caller's cancellation token to abort the wait.
+            return await existingTask.WaitAsync(cancellationToken);
         }
 
         // We own this cache entry — run syft and propagate the result.

test/Microsoft.ComponentDetection.Detectors.Tests/LinuxScannerTests.cs

@@ -1274,13 +1274,13 @@ public async Task TestLinuxScanner_SyftCacheKey_BindOrderDoesNotMatterAsync()
 
         var result1 = await scanner1.GetSyftOutputAsync(
             "oci-dir:/img",
-            new List<string> { "/host/a:/container/a:ro", "/host/b:/container/b:ro" },
+            ["/host/a:/container/a:ro", "/host/b:/container/b:ro"],
             LinuxScannerScope.AllLayers
         );
 
         var result2 = await scanner2.GetSyftOutputAsync(
             "oci-dir:/img",
-            new List<string> { "/host/b:/container/b:ro", "/host/a:/container/a:ro" },
+            ["/host/b:/container/b:ro", "/host/a:/container/a:ro"],
             LinuxScannerScope.AllLayers
         );
 
@@ -1375,4 +1375,77 @@ await this.linuxScanner.ScanLinuxAsync(
             Times.Exactly(2)
         );
     }
+
+    [TestMethod]
+    public async Task TestLinuxScanner_CancelledCaller_DoesNotBlockOnInFlightSyftRunAsync()
+    {
+        LinuxScanner.ResetCache();
+
+        // Use a TCS to control when the syft container "completes",
+        // so the first caller's run stays in-flight while we cancel the second.
+        var syftCompletionSource = new TaskCompletionSource<(string, string)>();
+
+        this.mockDockerService.Setup(service =>
+                service.CreateAndRunContainerAsync(
+                    It.IsAny<string>(),
+                    It.IsAny<List<string>>(),
+                    It.IsAny<IList<string>>(),
+                    It.IsAny<CancellationToken>()
+                )
+            )
+            .Returns(syftCompletionSource.Task);
+
+        var enabledTypes = new HashSet<ComponentType> { 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
+        );
+
+        // First caller starts the syft run (it will block on syftCompletionSource).
+        var task1 = scanner1.ScanLinuxAsync("cancel_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers);
+
+        // Second caller with a cancellable token joins the same in-flight run.
+        using var cts = new CancellationTokenSource();
+        var task2 = scanner2.ScanLinuxAsync("cancel_hash", layers, 0, enabledTypes, LinuxScannerScope.AllLayers, cts.Token);
+
+        // Cancel the second caller while the first is still running.
+        await cts.CancelAsync();
+
+        // The second caller should throw OperationCanceledException promptly.
+        try
+        {
+            await task2;
+            Assert.Fail("Expected OperationCanceledException was not thrown");
+        }
+        catch (OperationCanceledException)
+        {
+            // Expected — the second caller was cancelled while waiting for the in-flight run.
+        }
+
+        // The first caller should still be running (not cancelled).
+        task1.IsCompleted.Should().BeFalse();
+
+        // Now let the first caller complete normally.
+        syftCompletionSource.SetResult((SyftOutputNoAuthorOrLicense, string.Empty));
+        var result1 = await task1;
+        result1.Should().NotBeEmpty();
+    }
 }