Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public interface IMavenCommandService

Task<bool> MavenCLIExistsAsync();

Task GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default);
Task<MavenCliResult> GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default);

void ParseDependenciesFile(ProcessRequest processRequest);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace Microsoft.ComponentDetection.Detectors.Maven;

/// <summary>
/// Represents the result of executing a Maven CLI command.
/// </summary>
/// <param name="Success">Whether the command completed successfully.</param>
/// <param name="ErrorOutput">The error output from the command, if any.</param>
public record MavenCliResult(bool Success, string? ErrorOutput);
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
namespace Microsoft.ComponentDetection.Detectors.Maven;

using System;
using System.Collections.Concurrent;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -19,6 +20,19 @@ public class MavenCommandService : IMavenCommandService

internal static readonly string[] AdditionalValidCommands = ["mvn.cmd"];

/// <summary>
/// Per-location semaphores to prevent concurrent Maven CLI executions for the same pom.xml.
/// This allows multiple detectors (e.g., MvnCliComponentDetector and MavenWithFallbackDetector)
/// to safely share the same output file without race conditions.
/// </summary>
private readonly ConcurrentDictionary<string, SemaphoreSlim> locationLocks = new();

/// <summary>
/// Tracks locations where dependency generation has completed successfully.
/// Used to skip duplicate executions when multiple detectors process the same pom.xml.
/// </summary>
private readonly ConcurrentDictionary<string, MavenCliResult> completedLocations = new();
Comment on lines +23 to +34
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The per-location locationLocks and completedLocations dictionaries grow monotonically and are never pruned, and the SemaphoreSlim instances are never disposed. Since MavenCommandService is registered as a singleton, repeated scans (or very large repos) can cause unbounded memory growth.

Consider using a bounded/expiring cache and removing/disposing per-location semaphores once generation is complete (or switching to a keyed lock implementation that doesn't retain entries indefinitely).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a clean up method in OnDetectionFinishedAsync to clear cache


private readonly ICommandLineInvocationService commandLineInvocationService;
private readonly IMavenStyleDependencyGraphParserService parserService;
private readonly IEnvironmentVariableService envVarService;
Expand All @@ -43,7 +57,57 @@ public async Task<bool> MavenCLIExistsAsync()
return await this.commandLineInvocationService.CanCommandBeLocatedAsync(PrimaryCommand, AdditionalValidCommands, MvnVersionArgument);
}

public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default)
public async Task<MavenCliResult> GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default)
{
var pomFile = processRequest.ComponentStream;
var pomDir = Path.GetDirectoryName(pomFile.Location);
var depsFilePath = Path.Combine(pomDir, this.BcdeMvnDependencyFileName);

// 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)
&& cachedResult.Success
&& File.Exists(depsFilePath))
{
this.logger.LogDebug("{DetectorPrefix}: Skipping duplicate \"dependency:tree\" for {PomFileLocation}, already generated", DetectorLogPrefix, pomFile.Location);
return cachedResult;
}

// Use semaphore to prevent concurrent Maven CLI executions for the same pom.xml.
// This allows MvnCliComponentDetector and MavenWithFallbackDetector to safely share the output file.
var semaphore = this.locationLocks.GetOrAdd(pomFile.Location, _ => new SemaphoreSlim(1, 1));

await semaphore.WaitAsync(cancellationToken);
try
{
// Re-check the cache after acquiring the semaphore in case another caller
// completed while we were waiting.
if (this.completedLocations.TryGetValue(pomFile.Location, out cachedResult)
&& cachedResult.Success
&& File.Exists(depsFilePath))
{
Comment on lines 23 to 88
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The locationLocks/completedLocations dictionaries are keyed by pomFile.Location using the default (case-sensitive) comparer. On Windows/macOS this can lead to duplicate locks/cache entries for the same file path with different casing, causing extra CLI executions and unbounded growth. Consider normalizing to a full path and using StringComparer.OrdinalIgnoreCase for these dictionaries.

Copilot uses AI. Check for mistakes.
this.logger.LogDebug("{DetectorPrefix}: Skipping duplicate \"dependency:tree\" for {PomFileLocation}, already generated", DetectorLogPrefix, pomFile.Location);
return cachedResult;
}

var result = await this.GenerateDependenciesFileCoreAsync(processRequest, cancellationToken);

// Only cache successful results. Failed results should allow retries for transient failures,
// and caching them would waste memory since the cache check requires Success == true anyway.
if (result.Success)
{
this.completedLocations[pomFile.Location] = result;
}

return result;
Comment on lines 92 to 102
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

completedLocations is described as tracking locations that completed successfully, but the implementation caches all results (including failures) and never evicts entries. Since MavenCommandService is registered as a singleton, this can prevent retries of transient failures in later scans within the same process and can grow memory usage over time.

Only cache successful results and/or add an eviction/clear strategy scoped to a scan run.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to only cache successful result

}
finally
{
semaphore.Release();
}
}

private async Task<MavenCliResult> GenerateDependenciesFileCoreAsync(ProcessRequest processRequest, CancellationToken cancellationToken)
{
var cliFileTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
var timeoutSeconds = -1;
Expand All @@ -58,7 +122,7 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, C
var pomFile = processRequest.ComponentStream;
this.logger.LogDebug("{DetectorPrefix}: Running \"dependency:tree\" on {PomFileLocation}", DetectorLogPrefix, pomFile.Location);

var cliParameters = new[] { "dependency:tree", "-B", $"-DoutputFile={this.BcdeMvnDependencyFileName}", "-DoutputType=text", $"-f{pomFile.Location}" };
string[] cliParameters = ["dependency:tree", "-B", $"-DoutputFile={this.BcdeMvnDependencyFileName}", "-DoutputType=text", $"-f{pomFile.Location}"];

var result = await this.commandLineInvocationService.ExecuteCommandAsync(PrimaryCommand, AdditionalValidCommands, cancellationToken: cliFileTimeout.Token, cliParameters);

Expand All @@ -78,10 +142,13 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, C
{
this.logger.LogWarning("{DetectorPrefix}: There was a timeout in {PomFileLocation} file. Increase it using {TimeoutVar} environment variable.", DetectorLogPrefix, pomFile.Location, MvnCLIFileLevelTimeoutSecondsEnvVar);
}

return new MavenCliResult(false, errorMessage);
}
else
{
this.logger.LogDebug("{DetectorPrefix}: Execution of \"dependency:tree\" on {PomFileLocation} completed successfully", DetectorLogPrefix, pomFile.Location);
return new MavenCliResult(true, null);
}
}

Expand Down
Loading
Loading