From 4a3d9891989dcf6308bb4b56779ab753eed0a218 Mon Sep 17 00:00:00 2001 From: Zheng Hao Tang Date: Fri, 13 Mar 2026 13:10:44 -0700 Subject: [PATCH 1/4] Fix maven detector race conditions --- .../maven/IMavenCommandService.cs | 6 ++- .../maven/MavenCommandService.cs | 47 +++++++++++++------ .../maven/MavenWithFallbackDetector.cs | 47 ++++++++++--------- .../maven/MvnCliComponentDetector.cs | 12 ++--- .../MavenWithFallbackDetectorTests.cs | 4 +- 5 files changed, 69 insertions(+), 47 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs index c6adc8a2e..829bd1b82 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs @@ -20,12 +20,14 @@ public interface IMavenCommandService /// This prevents premature deletion by other detectors. /// /// The path to the dependency file being read. - void RegisterFileReader(string dependencyFilePath); + /// The identifier of the detector registering the file reader. + void RegisterFileReader(string dependencyFilePath, string detectorId = null); /// /// Unregisters a detector's active reading of a dependency file and attempts cleanup. /// If no other detectors are reading the file, it will be safely deleted. /// /// The path to the dependency file that was being read. - void UnregisterFileReader(string dependencyFilePath); + /// The identifier of the detector unregistering the file reader. + void UnregisterFileReader(string dependencyFilePath, string detectorId = null); } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs index 9d21971e2..e1a2db678 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs @@ -69,6 +69,9 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest p var pomDir = Path.GetDirectoryName(pomFile.Location); var depsFilePath = Path.Combine(pomDir, this.BcdeMvnDependencyFileName); + // Register as file reader immediately to prevent premature cleanup + this.RegisterFileReader(depsFilePath, "GenerationStep"); + // Check the cache before acquiring the semaphore to allow fast-path returns // even when cancellation has been requested. if (this.completedLocations.TryGetValue(pomFile.Location, out var cachedResult) @@ -154,6 +157,7 @@ private async Task GenerateDependenciesFileCoreAsync(ProcessRequ else { this.logger.LogDebug("{DetectorPrefix}: Execution of \"dependency:tree\" on {PomFileLocation} completed successfully", DetectorLogPrefix, pomFile.Location); + return new MavenCliResult(true, null); } } @@ -171,9 +175,15 @@ public void ParseDependenciesFile(ProcessRequest processRequest) /// This prevents premature deletion by other detectors. /// /// The path to the dependency file being read. - public void RegisterFileReader(string dependencyFilePath) + /// The identifier of the detector registering the file reader. + public void RegisterFileReader(string dependencyFilePath, string detectorId = null) { this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 1, (key, count) => count + 1); + this.logger.LogDebug( + "{DetectorId}: Registered file reader for {DependencyFilePath}, count: {Count}", + detectorId ?? "Unknown", + dependencyFilePath, + this.fileReaderCounts[dependencyFilePath]); } /// @@ -181,10 +191,16 @@ public void RegisterFileReader(string dependencyFilePath) /// If no other detectors are reading the file, it will be safely deleted. /// /// The path to the dependency file that was being read. - public void UnregisterFileReader(string dependencyFilePath) + /// The identifier of the detector unregistering the file reader. + public void UnregisterFileReader(string dependencyFilePath, string detectorId = null) { this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 0, (key, count) => Math.Max(0, count - 1)); - this.TryDeleteDependencyFileIfNotInUse(dependencyFilePath); + this.logger.LogDebug( + "{DetectorId}: Unregistered file reader for {DependencyFilePath}, count: {Count}", + detectorId ?? "Unknown", + dependencyFilePath, + this.fileReaderCounts.TryGetValue(dependencyFilePath, out var currentCount) ? currentCount : 0); + this.TryDeleteDependencyFileIfNotInUse(dependencyFilePath, detectorId); } /// @@ -192,12 +208,15 @@ public void UnregisterFileReader(string dependencyFilePath) /// Uses cross-process coordination to prevent race conditions with other instances. /// /// The path to the dependency file to delete. - private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath) + /// The identifier of the detector requesting the deletion. + private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath, string detectorId = null) { + var detector = detectorId ?? "Unknown"; + // Check if any local readers are using the file if (this.fileReaderCounts.TryGetValue(dependencyFilePath, out var count) && count > 0) { - this.logger.LogDebug("Skipping deletion of {DependencyFilePath} - {Count} local readers still active", dependencyFilePath, count); + this.logger.LogDebug("{DetectorId}: Skipping deletion of {DependencyFilePath} - {Count} local readers still active", detector, dependencyFilePath, count); return; } @@ -215,7 +234,7 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath) writer.Write(processId); } - this.logger.LogDebug("Created coordination file {CoordinationFile} for process {ProcessId}", coordinationFile, processId); + this.logger.LogDebug("{DetectorId}: Created coordination file {CoordinationFile} for process {ProcessId}", detector, coordinationFile, processId); // Give other processes a chance to create their coordination files if needed Thread.Sleep(50); @@ -223,14 +242,14 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath) // Verify we still own the coordination (another process might have deleted and recreated it) if (!File.Exists(coordinationFile)) { - this.logger.LogDebug("Coordination file {CoordinationFile} was deleted by another process", coordinationFile); + this.logger.LogDebug("{DetectorId}: Coordination file {CoordinationFile} was deleted by another process", detector, coordinationFile); return; } var coordinationContent = File.ReadAllText(coordinationFile).Trim(); if (coordinationContent != processId) { - this.logger.LogDebug("Coordination file {CoordinationFile} was taken over by process {OtherProcessId}, aborting deletion", coordinationFile, coordinationContent); + this.logger.LogDebug("{DetectorId}: Coordination file {CoordinationFile} was taken over by process {OtherProcessId}, aborting deletion", detector, coordinationFile, coordinationContent); return; } @@ -238,21 +257,21 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath) if (File.Exists(dependencyFilePath)) { File.Delete(dependencyFilePath); - this.logger.LogDebug("Successfully deleted dependency file {DependencyFilePath}", dependencyFilePath); + this.logger.LogDebug("{DetectorId}: Successfully deleted dependency file {DependencyFilePath}", detector, dependencyFilePath); } else { - this.logger.LogDebug("Dependency file {DependencyFilePath} was already deleted", dependencyFilePath); + this.logger.LogDebug("{DetectorId}: Dependency file {DependencyFilePath} was already deleted", detector, dependencyFilePath); } } catch (IOException ex) when (ex.Message.Contains("already exists") || ex.HResult == -2147024816) { // Another process is handling deletion (File already exists) - this.logger.LogDebug("Another process is already coordinating deletion of {DependencyFilePath}", dependencyFilePath); + this.logger.LogDebug("{DetectorId}: Another process is already coordinating deletion of {DependencyFilePath}", detector, dependencyFilePath); } catch (Exception ex) { - this.logger.LogWarning(ex, "Failed to coordinate deletion of dependency file {DependencyFilePath}", dependencyFilePath); + this.logger.LogWarning(ex, "{DetectorId}: Failed to coordinate deletion of dependency file {DependencyFilePath}", detector, dependencyFilePath); } finally { @@ -265,13 +284,13 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath) if (coordinationContent == Environment.ProcessId.ToString()) { File.Delete(coordinationFile); - this.logger.LogDebug("Cleaned up coordination file {CoordinationFile}", coordinationFile); + this.logger.LogDebug("{DetectorId}: Cleaned up coordination file {CoordinationFile}", detector, coordinationFile); } } } catch (Exception ex) { - this.logger.LogDebug(ex, "Failed to clean up coordination file {CoordinationFile}, will be cleaned up later", coordinationFile); + this.logger.LogDebug(ex, "{DetectorId}: Failed to clean up coordination file {CoordinationFile}, will be cleaned up later", detector, coordinationFile); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs index 99126d319..96cdb6a68 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs @@ -112,11 +112,11 @@ public class MavenWithFallbackDetector : FileComponentDetector, IExperimentalDet private readonly Dictionary documentsLoaded = []; // Track original pom.xml files for potential fallback - private readonly ConcurrentBag originalPomFiles = []; + private readonly ConcurrentQueue originalPomFiles = []; // Track Maven CLI errors for analysis - private readonly ConcurrentBag mavenCliErrors = []; - private readonly ConcurrentBag failedEndpoints = []; + private readonly ConcurrentQueue mavenCliErrors = []; + private readonly ConcurrentQueue failedEndpoints = []; // Telemetry tracking private MavenDetectionMethod usedDetectionMethod = MavenDetectionMethod.None; @@ -185,7 +185,6 @@ protected override async Task> OnPrepareDetectionAsy // Timeout occurred (not user cancellation) this.LogWarning($"OnPrepareDetectionAsync timed out after {PrepareDetectionTimeout.TotalMinutes} minutes. Falling back to static pom.xml parsing."); this.Telemetry["TimedOut"] = "true"; - this.usedDetectionMethod = MavenDetectionMethod.StaticParserOnly; this.fallbackReason = MavenFallbackReason.OtherMvnCliFailure; return processRequests; } @@ -194,7 +193,6 @@ protected override async Task> OnPrepareDetectionAsy // Unexpected error - log and fall back to static parsing this.LogWarning($"OnPrepareDetectionAsync failed with unexpected error: {ex.Message}. Falling back to static pom.xml parsing."); this.Telemetry["PrepareDetectionError"] = ex.GetType().Name; - this.usedDetectionMethod = MavenDetectionMethod.StaticParserOnly; this.fallbackReason = MavenFallbackReason.OtherMvnCliFailure; return processRequests; } @@ -281,7 +279,7 @@ private async Task> RunMavenCliDetectionAsync( CancellationToken cancellationToken) { var results = new ConcurrentQueue(); - var failedDirectories = new ConcurrentBag(); + var failedDirectories = new ConcurrentQueue(); var cliSuccessCount = 0; var cliFailureCount = 0; @@ -295,7 +293,7 @@ private async Task> RunMavenCliDetectionAsync( cancellationToken.ThrowIfCancellationRequested(); // Store original pom.xml for telemetry - this.originalPomFiles.Add(processRequest); + this.originalPomFiles.Enqueue(processRequest); var pomFile = processRequest.ComponentStream; var pomDir = Path.GetDirectoryName(pomFile.Location); @@ -315,13 +313,14 @@ private async Task> RunMavenCliDetectionAsync( // Use existence check to avoid redundant I/O (file will be read during directory scan) if (this.fileUtilityService.Exists(depsFilePath)) { + // File reader registration is now handled in GenerateDependenciesFileAsync Interlocked.Increment(ref cliSuccessCount); } else { // CLI reported success but deps file is missing - treat as failure Interlocked.Increment(ref cliFailureCount); - failedDirectories.Add(pomDir); + failedDirectories.Enqueue(pomDir); this.LogWarning($"Maven CLI succeeded but deps file not found: {depsFilePath}"); } } @@ -329,12 +328,12 @@ private async Task> RunMavenCliDetectionAsync( { // CLI failed - track directory for nested pom.xml scanning Interlocked.Increment(ref cliFailureCount); - failedDirectories.Add(pomDir); + failedDirectories.Enqueue(pomDir); // Capture error output for later analysis if (!string.IsNullOrWhiteSpace(result.ErrorOutput)) { - this.mavenCliErrors.Add(result.ErrorOutput); + this.mavenCliErrors.Enqueue(result.ErrorOutput); } } }, @@ -420,13 +419,7 @@ private void DetermineDetectionMethod(int cliSuccessCount, int cliFailureCount) this.usedDetectionMethod = MavenDetectionMethod.MvnCliOnly; this.LogDebug("All pom.xml files processed successfully with Maven CLI."); } - else if (cliSuccessCount == 0 && cliFailureCount > 0) - { - this.usedDetectionMethod = MavenDetectionMethod.StaticParserOnly; - this.LogWarning("Maven CLI failed for all pom.xml files. Using static parsing fallback."); - this.AnalyzeMvnCliFailure(); - } - else if (cliSuccessCount > 0 && cliFailureCount > 0) + else if (cliFailureCount > 0) { this.usedDetectionMethod = MavenDetectionMethod.Mixed; this.LogWarning($"Maven CLI failed for {cliFailureCount} pom.xml files. Using mixed detection."); @@ -443,16 +436,16 @@ protected override Task OnFileFoundAsync( if (pattern == this.mavenCommandService.BcdeMvnDependencyFileName) { - // Process MvnCli result - register as reader and cleanup when done + // Process MvnCli result - file reader already registered at generation step var depsFilePath = processRequest.ComponentStream.Location; - this.mavenCommandService.RegisterFileReader(depsFilePath); try { this.ProcessMvnCliResult(processRequest); } finally { - this.mavenCommandService.UnregisterFileReader(depsFilePath); + // Unregister file reader after processing to allow cleanup + this.mavenCommandService.UnregisterFileReader(depsFilePath, this.Id); } } else @@ -503,7 +496,7 @@ private void AnalyzeMvnCliFailure() // Extract failed endpoints from error messages foreach (var endpoint in this.mavenCliErrors.SelectMany(this.ExtractFailedEndpoints)) { - this.failedEndpoints.Add(endpoint); + this.failedEndpoints.Enqueue(endpoint); } this.LogAuthErrorGuidance(); @@ -531,6 +524,8 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) var singleFileComponentRecorder = processRequest.SingleFileComponentRecorder; var filePath = file.Location; + this.Logger.LogWarning("FallbackMaven: Processing pom.xml: {FilePath}", filePath); + try { var document = new XmlDocument(); @@ -538,7 +533,11 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) lock (this.documentsLoaded) { - this.documentsLoaded.TryAdd(file.Location, document); + var wasAdded = this.documentsLoaded.TryAdd(file.Location, document); + if (wasAdded) + { + this.Logger.LogWarning("FallbackMaven: Added document to shared dictionary: {FilePath} (Total docs: {Count})", file.Location, this.documentsLoaded.Count); + } } var namespaceManager = new XmlNamespaceManager(document.NameTable); @@ -576,6 +575,7 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) { var component = new MavenComponent(groupId, artifactId, versionString); var detectedComponent = new DetectedComponent(component); + this.Logger.LogWarning("FallbackMaven: Registering component: {ComponentName}@{ComponentVersion} from file: {FileName}", component.Id, component.Version, Path.GetFileName(filePath)); singleFileComponentRecorder.RegisterUsage(detectedComponent); componentsFoundInFile++; } @@ -597,6 +597,7 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) } } + this.Logger.LogWarning("FallbackMaven: File {FileName} contributed {ComponentCount} components", Path.GetFileName(filePath), componentsFoundInFile); Interlocked.Add(ref this.staticParserComponentCount, componentsFoundInFile); } catch (Exception e) @@ -674,6 +675,7 @@ private string ReplaceVariable(string variable, XmlDocument currentDocument, str lock (this.documentsLoaded) { + this.Logger.LogWarning("FallbackMaven: Variable resolution for {Variable} searching through {Count} loaded documents from: {Paths}", variable, this.documentsLoaded.Count, string.Join(", ", this.documentsLoaded.Keys.Select(Path.GetFileName))); foreach (var pathDocumentPair in this.documentsLoaded) { var path = pathDocumentPair.Key; @@ -681,6 +683,7 @@ private string ReplaceVariable(string variable, XmlDocument currentDocument, str result = this.FindVariableInDocument(document, path, variable); if (result != null) { + this.Logger.LogWarning("FallbackMaven: Variable {Variable} resolved from document: {Path}", variable, Path.GetFileName(path)); return result; } } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs index e69d87475..fefc11f28 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs @@ -76,11 +76,10 @@ await this.RemoveNestedPomXmls(processRequests).ForEachAsync(processRequest => { // The file stream is going to be disposed after the iteration is finished // so is necessary to read the content and keep it in memory, for further processing. - // Register as reader for coordination with file deletion - this.mavenCommandService.RegisterFileReader(componentStream.Location); + // File reader registration is now handled in GenerateDependenciesFileAsync using var reader = new StreamReader(componentStream.Stream); var content = reader.ReadToEnd(); - this.mavenCommandService.UnregisterFileReader(componentStream.Location); + return new ProcessRequest { ComponentStream = new ComponentStream @@ -98,17 +97,16 @@ await this.RemoveNestedPomXmls(processRequests).ForEachAsync(processRequest => protected override async Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) { - // Register as reader to coordinate with other detectors + // File reader already registered at generation step, just process and unregister var depsFilePath = processRequest.ComponentStream.Location; - this.mavenCommandService.RegisterFileReader(depsFilePath); try { this.mavenCommandService.ParseDependenciesFile(processRequest); } finally { - // Unregister and attempt coordinated cleanup - this.mavenCommandService.UnregisterFileReader(depsFilePath); + // Cleanup coordination with other detectors + this.mavenCommandService.UnregisterFileReader(depsFilePath, this.Id); } await Task.CompletedTask; diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs index 208cf95a1..cf9338e90 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs @@ -1178,9 +1178,9 @@ public async Task WhenAuthenticationFailsAndParentChildPropertiesUsed_MaintainsC detectorResult.AdditionalTelemetryDetails.Should().ContainKey("FallbackReason"); detectorResult.AdditionalTelemetryDetails["FallbackReason"].Should().Be("AuthenticationFailure"); - // Should have method showing static parser was used + // Should have method showing mixed detection was used (CLI failed but fallback succeeded) detectorResult.AdditionalTelemetryDetails.Should().ContainKey("DetectionMethod"); - detectorResult.AdditionalTelemetryDetails["DetectionMethod"].Should().Be("StaticParserOnly"); + detectorResult.AdditionalTelemetryDetails["DetectionMethod"].Should().Be("Mixed"); } private void SetupMvnCliSuccess(string depsFileContent) From 9fbde104c99ca9db0c2c49ed1fdf621a98faef4c Mon Sep 17 00:00:00 2001 From: Zheng Hao Tang Date: Fri, 13 Mar 2026 13:36:14 -0700 Subject: [PATCH 2/4] PR comments --- .../maven/IMavenCommandService.cs | 3 +-- .../maven/MavenCommandService.cs | 8 +++----- .../maven/MavenWithFallbackDetector.cs | 14 ++++---------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs index 829bd1b82..309cbf40e 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs @@ -20,8 +20,7 @@ public interface IMavenCommandService /// This prevents premature deletion by other detectors. /// /// The path to the dependency file being read. - /// The identifier of the detector registering the file reader. - void RegisterFileReader(string dependencyFilePath, string detectorId = null); + void RegisterFileReader(string dependencyFilePath); /// /// Unregisters a detector's active reading of a dependency file and attempts cleanup. diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs index e1a2db678..d6bcef707 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs @@ -70,7 +70,7 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest p var depsFilePath = Path.Combine(pomDir, this.BcdeMvnDependencyFileName); // Register as file reader immediately to prevent premature cleanup - this.RegisterFileReader(depsFilePath, "GenerationStep"); + this.RegisterFileReader(depsFilePath); // Check the cache before acquiring the semaphore to allow fast-path returns // even when cancellation has been requested. @@ -175,13 +175,11 @@ public void ParseDependenciesFile(ProcessRequest processRequest) /// This prevents premature deletion by other detectors. /// /// The path to the dependency file being read. - /// The identifier of the detector registering the file reader. - public void RegisterFileReader(string dependencyFilePath, string detectorId = null) + public void RegisterFileReader(string dependencyFilePath) { this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 1, (key, count) => count + 1); this.logger.LogDebug( - "{DetectorId}: Registered file reader for {DependencyFilePath}, count: {Count}", - detectorId ?? "Unknown", + "Registered file reader for {DependencyFilePath}, count: {Count}", dependencyFilePath, this.fileReaderCounts[dependencyFilePath]); } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs index 96cdb6a68..7320d5cab 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs @@ -186,6 +186,7 @@ protected override async Task> OnPrepareDetectionAsy this.LogWarning($"OnPrepareDetectionAsync timed out after {PrepareDetectionTimeout.TotalMinutes} minutes. Falling back to static pom.xml parsing."); this.Telemetry["TimedOut"] = "true"; this.fallbackReason = MavenFallbackReason.OtherMvnCliFailure; + this.usedDetectionMethod = MavenDetectionMethod.Mixed; return processRequests; } catch (Exception ex) @@ -194,6 +195,7 @@ protected override async Task> OnPrepareDetectionAsy this.LogWarning($"OnPrepareDetectionAsync failed with unexpected error: {ex.Message}. Falling back to static pom.xml parsing."); this.Telemetry["PrepareDetectionError"] = ex.GetType().Name; this.fallbackReason = MavenFallbackReason.OtherMvnCliFailure; + this.usedDetectionMethod = MavenDetectionMethod.Mixed; return processRequests; } } @@ -524,7 +526,7 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) var singleFileComponentRecorder = processRequest.SingleFileComponentRecorder; var filePath = file.Location; - this.Logger.LogWarning("FallbackMaven: Processing pom.xml: {FilePath}", filePath); + this.Logger.LogDebug("FallbackMaven: Processing pom.xml: {FilePath}", filePath); try { @@ -533,11 +535,7 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) lock (this.documentsLoaded) { - var wasAdded = this.documentsLoaded.TryAdd(file.Location, document); - if (wasAdded) - { - this.Logger.LogWarning("FallbackMaven: Added document to shared dictionary: {FilePath} (Total docs: {Count})", file.Location, this.documentsLoaded.Count); - } + this.documentsLoaded.TryAdd(file.Location, document); } var namespaceManager = new XmlNamespaceManager(document.NameTable); @@ -575,7 +573,6 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) { var component = new MavenComponent(groupId, artifactId, versionString); var detectedComponent = new DetectedComponent(component); - this.Logger.LogWarning("FallbackMaven: Registering component: {ComponentName}@{ComponentVersion} from file: {FileName}", component.Id, component.Version, Path.GetFileName(filePath)); singleFileComponentRecorder.RegisterUsage(detectedComponent); componentsFoundInFile++; } @@ -597,7 +594,6 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) } } - this.Logger.LogWarning("FallbackMaven: File {FileName} contributed {ComponentCount} components", Path.GetFileName(filePath), componentsFoundInFile); Interlocked.Add(ref this.staticParserComponentCount, componentsFoundInFile); } catch (Exception e) @@ -675,7 +671,6 @@ private string ReplaceVariable(string variable, XmlDocument currentDocument, str lock (this.documentsLoaded) { - this.Logger.LogWarning("FallbackMaven: Variable resolution for {Variable} searching through {Count} loaded documents from: {Paths}", variable, this.documentsLoaded.Count, string.Join(", ", this.documentsLoaded.Keys.Select(Path.GetFileName))); foreach (var pathDocumentPair in this.documentsLoaded) { var path = pathDocumentPair.Key; @@ -683,7 +678,6 @@ private string ReplaceVariable(string variable, XmlDocument currentDocument, str result = this.FindVariableInDocument(document, path, variable); if (result != null) { - this.Logger.LogWarning("FallbackMaven: Variable {Variable} resolved from document: {Path}", variable, Path.GetFileName(path)); return result; } } From ed20692dfea094df30dfdb3a57522aa823d626c1 Mon Sep 17 00:00:00 2001 From: Zheng Hao Tang Date: Fri, 13 Mar 2026 13:48:18 -0700 Subject: [PATCH 3/4] Another PR comment --- .../maven/MavenCommandService.cs | 79 ++++--------------- .../maven/MavenWithFallbackDetector.cs | 2 - 2 files changed, 17 insertions(+), 64 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs index d6bcef707..222f84683 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs @@ -203,7 +203,7 @@ public void UnregisterFileReader(string dependencyFilePath, string detectorId = /// /// Attempts to delete a dependency file if no detectors are currently using it. - /// Uses cross-process coordination to prevent race conditions with other instances. + /// Uses atomic counter operations for thread-safe coordination. /// /// The path to the dependency file to delete. /// The identifier of the detector requesting the deletion. @@ -211,47 +211,27 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath, string { var detector = detectorId ?? "Unknown"; - // Check if any local readers are using the file - if (this.fileReaderCounts.TryGetValue(dependencyFilePath, out var count) && count > 0) + // Atomically check if any readers are using the file + var shouldDelete = false; + this.fileReaderCounts.AddOrUpdate( + dependencyFilePath, + 0, + (key, count) => + { + shouldDelete = count == 0; + return count; + }); + + if (!shouldDelete) { - this.logger.LogDebug("{DetectorId}: Skipping deletion of {DependencyFilePath} - {Count} local readers still active", detector, dependencyFilePath, count); + var currentCount = this.fileReaderCounts.TryGetValue(dependencyFilePath, out var count) ? count : 0; + this.logger.LogDebug("{DetectorId}: Skipping deletion of {DependencyFilePath} - {Count} readers still active", detector, dependencyFilePath, currentCount); return; } - var coordinationFile = dependencyFilePath + ".deleting"; - + // Safe to delete - no readers are using the file try { - // Try to create coordination file atomically with our process ID - var processId = Environment.ProcessId.ToString(); - - // Use FileMode.CreateNew to ensure atomic creation (fails if file exists) - using (var fs = new FileStream(coordinationFile, FileMode.CreateNew, FileAccess.Write, FileShare.None)) - using (var writer = new StreamWriter(fs)) - { - writer.Write(processId); - } - - this.logger.LogDebug("{DetectorId}: Created coordination file {CoordinationFile} for process {ProcessId}", detector, coordinationFile, processId); - - // Give other processes a chance to create their coordination files if needed - Thread.Sleep(50); - - // Verify we still own the coordination (another process might have deleted and recreated it) - if (!File.Exists(coordinationFile)) - { - this.logger.LogDebug("{DetectorId}: Coordination file {CoordinationFile} was deleted by another process", detector, coordinationFile); - return; - } - - var coordinationContent = File.ReadAllText(coordinationFile).Trim(); - if (coordinationContent != processId) - { - this.logger.LogDebug("{DetectorId}: Coordination file {CoordinationFile} was taken over by process {OtherProcessId}, aborting deletion", detector, coordinationFile, coordinationContent); - return; - } - - // We own the coordination - proceed with deletion if (File.Exists(dependencyFilePath)) { File.Delete(dependencyFilePath); @@ -262,34 +242,9 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath, string this.logger.LogDebug("{DetectorId}: Dependency file {DependencyFilePath} was already deleted", detector, dependencyFilePath); } } - catch (IOException ex) when (ex.Message.Contains("already exists") || ex.HResult == -2147024816) - { - // Another process is handling deletion (File already exists) - this.logger.LogDebug("{DetectorId}: Another process is already coordinating deletion of {DependencyFilePath}", detector, dependencyFilePath); - } catch (Exception ex) { - this.logger.LogWarning(ex, "{DetectorId}: Failed to coordinate deletion of dependency file {DependencyFilePath}", detector, dependencyFilePath); - } - finally - { - // Clean up our coordination file - try - { - if (File.Exists(coordinationFile)) - { - var coordinationContent = File.ReadAllText(coordinationFile).Trim(); - if (coordinationContent == Environment.ProcessId.ToString()) - { - File.Delete(coordinationFile); - this.logger.LogDebug("{DetectorId}: Cleaned up coordination file {CoordinationFile}", detector, coordinationFile); - } - } - } - catch (Exception ex) - { - this.logger.LogDebug(ex, "{DetectorId}: Failed to clean up coordination file {CoordinationFile}, will be cleaned up later", detector, coordinationFile); - } + this.logger.LogWarning(ex, "{DetectorId}: Failed to delete dependency file {DependencyFilePath}", detector, dependencyFilePath); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs index 7320d5cab..f458727a0 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs @@ -526,8 +526,6 @@ private void ProcessPomFileStatically(ProcessRequest processRequest) var singleFileComponentRecorder = processRequest.SingleFileComponentRecorder; var filePath = file.Location; - this.Logger.LogDebug("FallbackMaven: Processing pom.xml: {FilePath}", filePath); - try { var document = new XmlDocument(); From 73ee5fe8ca86a11cfa1949e8726d0d23539c595e Mon Sep 17 00:00:00 2001 From: Zheng Hao Tang Date: Fri, 13 Mar 2026 14:11:14 -0700 Subject: [PATCH 4/4] Cleanup code --- .../maven/MavenCommandService.cs | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs index 222f84683..66dd17d0b 100644 --- a/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs +++ b/src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs @@ -192,18 +192,22 @@ public void RegisterFileReader(string dependencyFilePath) /// The identifier of the detector unregistering the file reader. public void UnregisterFileReader(string dependencyFilePath, string detectorId = null) { - this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 0, (key, count) => Math.Max(0, count - 1)); + var newCount = this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 0, (key, count) => Math.Max(0, count - 1)); this.logger.LogDebug( "{DetectorId}: Unregistered file reader for {DependencyFilePath}, count: {Count}", detectorId ?? "Unknown", dependencyFilePath, - this.fileReaderCounts.TryGetValue(dependencyFilePath, out var currentCount) ? currentCount : 0); - this.TryDeleteDependencyFileIfNotInUse(dependencyFilePath, detectorId); + newCount); + + // If no readers remain, attempt cleanup + if (newCount == 0) + { + this.TryDeleteDependencyFileIfNotInUse(dependencyFilePath, detectorId); + } } /// /// Attempts to delete a dependency file if no detectors are currently using it. - /// Uses atomic counter operations for thread-safe coordination. /// /// The path to the dependency file to delete. /// The identifier of the detector requesting the deletion. @@ -211,25 +215,7 @@ private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath, string { var detector = detectorId ?? "Unknown"; - // Atomically check if any readers are using the file - var shouldDelete = false; - this.fileReaderCounts.AddOrUpdate( - dependencyFilePath, - 0, - (key, count) => - { - shouldDelete = count == 0; - return count; - }); - - if (!shouldDelete) - { - var currentCount = this.fileReaderCounts.TryGetValue(dependencyFilePath, out var count) ? count : 0; - this.logger.LogDebug("{DetectorId}: Skipping deletion of {DependencyFilePath} - {Count} readers still active", detector, dependencyFilePath, currentCount); - return; - } - - // Safe to delete - no readers are using the file + // Safe to delete - no readers are using the file (count was already verified to be 0) try { if (File.Exists(dependencyFilePath))