Skip to content

Commit a8222d2

Browse files
authored
Maven combined detector experiment (#1628)
* Maven combined detector experiment * Fix verification test race condition * Added variable to account for race conditions * Fixed tests * Update MavenCommandService * NIt * Force experiment detector to use another temp directory * PR comments * Remove extra code * Nit * Another nit * Update experiment * Perf refactoring * Perf improvement * perf update * Updated tests * Readd semaphore * More improvements * Nit * Remove Maven new detector experiment * Remove unused method overload * Remove unused parameter * More perf improvement * Nit * All cancellation token usage * Added ExecutionDataflowBlockOptions * Build error
1 parent 6748194 commit a8222d2

File tree

7 files changed

+2237
-20
lines changed

7 files changed

+2237
-20
lines changed

src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public interface IMavenCommandService
1111

1212
Task<bool> MavenCLIExistsAsync();
1313

14-
Task GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default);
14+
Task<MavenCliResult> GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default);
1515

1616
void ParseDependenciesFile(ProcessRequest processRequest);
1717
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace Microsoft.ComponentDetection.Detectors.Maven;
2+
3+
/// <summary>
4+
/// Represents the result of executing a Maven CLI command.
5+
/// </summary>
6+
/// <param name="Success">Whether the command completed successfully.</param>
7+
/// <param name="ErrorOutput">The error output from the command, if any.</param>
8+
public record MavenCliResult(bool Success, string? ErrorOutput);

src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
namespace Microsoft.ComponentDetection.Detectors.Maven;
33

44
using System;
5+
using System.Collections.Concurrent;
56
using System.IO;
67
using System.Threading;
78
using System.Threading.Tasks;
@@ -19,6 +20,19 @@ public class MavenCommandService : IMavenCommandService
1920

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

23+
/// <summary>
24+
/// Per-location semaphores to prevent concurrent Maven CLI executions for the same pom.xml.
25+
/// This allows multiple detectors (e.g., MvnCliComponentDetector and MavenWithFallbackDetector)
26+
/// to safely share the same output file without race conditions.
27+
/// </summary>
28+
private readonly ConcurrentDictionary<string, SemaphoreSlim> locationLocks = new();
29+
30+
/// <summary>
31+
/// Tracks locations where dependency generation has completed successfully.
32+
/// Used to skip duplicate executions when multiple detectors process the same pom.xml.
33+
/// </summary>
34+
private readonly ConcurrentDictionary<string, MavenCliResult> completedLocations = new();
35+
2236
private readonly ICommandLineInvocationService commandLineInvocationService;
2337
private readonly IMavenStyleDependencyGraphParserService parserService;
2438
private readonly IEnvironmentVariableService envVarService;
@@ -43,7 +57,57 @@ public async Task<bool> MavenCLIExistsAsync()
4357
return await this.commandLineInvocationService.CanCommandBeLocatedAsync(PrimaryCommand, AdditionalValidCommands, MvnVersionArgument);
4458
}
4559

46-
public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default)
60+
public async Task<MavenCliResult> GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default)
61+
{
62+
var pomFile = processRequest.ComponentStream;
63+
var pomDir = Path.GetDirectoryName(pomFile.Location);
64+
var depsFilePath = Path.Combine(pomDir, this.BcdeMvnDependencyFileName);
65+
66+
// Check the cache before acquiring the semaphore to allow fast-path returns
67+
// even when cancellation has been requested.
68+
if (this.completedLocations.TryGetValue(pomFile.Location, out var cachedResult)
69+
&& cachedResult.Success
70+
&& File.Exists(depsFilePath))
71+
{
72+
this.logger.LogDebug("{DetectorPrefix}: Skipping duplicate \"dependency:tree\" for {PomFileLocation}, already generated", DetectorLogPrefix, pomFile.Location);
73+
return cachedResult;
74+
}
75+
76+
// Use semaphore to prevent concurrent Maven CLI executions for the same pom.xml.
77+
// This allows MvnCliComponentDetector and MavenWithFallbackDetector to safely share the output file.
78+
var semaphore = this.locationLocks.GetOrAdd(pomFile.Location, _ => new SemaphoreSlim(1, 1));
79+
80+
await semaphore.WaitAsync(cancellationToken);
81+
try
82+
{
83+
// Re-check the cache after acquiring the semaphore in case another caller
84+
// completed while we were waiting.
85+
if (this.completedLocations.TryGetValue(pomFile.Location, out cachedResult)
86+
&& cachedResult.Success
87+
&& File.Exists(depsFilePath))
88+
{
89+
this.logger.LogDebug("{DetectorPrefix}: Skipping duplicate \"dependency:tree\" for {PomFileLocation}, already generated", DetectorLogPrefix, pomFile.Location);
90+
return cachedResult;
91+
}
92+
93+
var result = await this.GenerateDependenciesFileCoreAsync(processRequest, cancellationToken);
94+
95+
// Only cache successful results. Failed results should allow retries for transient failures,
96+
// and caching them would waste memory since the cache check requires Success == true anyway.
97+
if (result.Success)
98+
{
99+
this.completedLocations[pomFile.Location] = result;
100+
}
101+
102+
return result;
103+
}
104+
finally
105+
{
106+
semaphore.Release();
107+
}
108+
}
109+
110+
private async Task<MavenCliResult> GenerateDependenciesFileCoreAsync(ProcessRequest processRequest, CancellationToken cancellationToken)
47111
{
48112
var cliFileTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
49113
var timeoutSeconds = -1;
@@ -58,7 +122,7 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, C
58122
var pomFile = processRequest.ComponentStream;
59123
this.logger.LogDebug("{DetectorPrefix}: Running \"dependency:tree\" on {PomFileLocation}", DetectorLogPrefix, pomFile.Location);
60124

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

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

@@ -78,10 +142,13 @@ public async Task GenerateDependenciesFileAsync(ProcessRequest processRequest, C
78142
{
79143
this.logger.LogWarning("{DetectorPrefix}: There was a timeout in {PomFileLocation} file. Increase it using {TimeoutVar} environment variable.", DetectorLogPrefix, pomFile.Location, MvnCLIFileLevelTimeoutSecondsEnvVar);
80144
}
145+
146+
return new MavenCliResult(false, errorMessage);
81147
}
82148
else
83149
{
84150
this.logger.LogDebug("{DetectorPrefix}: Execution of \"dependency:tree\" on {PomFileLocation} completed successfully", DetectorLogPrefix, pomFile.Location);
151+
return new MavenCliResult(true, null);
85152
}
86153
}
87154

0 commit comments

Comments
 (0)