Skip to content

Commit 725717b

Browse files
Fix submodule bug in MavenFallbackDetector (#1685)
* Fix submodule bug in MavenFallbackDetector * Optimizations for file enumeration * Make results a queue to maintain parent ordering * Add unit test to ensure ordering * Version bump * Fix test failure * Semaphore logic to safeguard file deletion * PR comment * Nit cleanup --------- Co-authored-by: Fernando Rojo <ferojo@microsoft.com>
1 parent f2d7419 commit 725717b

File tree

5 files changed

+375
-121
lines changed

5 files changed

+375
-121
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,18 @@ public interface IMavenCommandService
1414
Task<MavenCliResult> GenerateDependenciesFileAsync(ProcessRequest processRequest, CancellationToken cancellationToken = default);
1515

1616
void ParseDependenciesFile(ProcessRequest processRequest);
17+
18+
/// <summary>
19+
/// Registers that a detector is actively reading a dependency file.
20+
/// This prevents premature deletion by other detectors.
21+
/// </summary>
22+
/// <param name="dependencyFilePath">The path to the dependency file being read.</param>
23+
void RegisterFileReader(string dependencyFilePath);
24+
25+
/// <summary>
26+
/// Unregisters a detector's active reading of a dependency file and attempts cleanup.
27+
/// If no other detectors are reading the file, it will be safely deleted.
28+
/// </summary>
29+
/// <param name="dependencyFilePath">The path to the dependency file that was being read.</param>
30+
void UnregisterFileReader(string dependencyFilePath);
1731
}

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ internal class MavenCommandService : IMavenCommandService
3333
/// </summary>
3434
private readonly ConcurrentDictionary<string, MavenCliResult> completedLocations = new();
3535

36+
/// <summary>
37+
/// Tracks the number of active readers for each dependency file.
38+
/// Used for safe file cleanup coordination between detectors.
39+
/// </summary>
40+
private readonly ConcurrentDictionary<string, int> fileReaderCounts = new();
41+
3642
private readonly ICommandLineInvocationService commandLineInvocationService;
3743
private readonly IMavenStyleDependencyGraphParserService parserService;
3844
private readonly IEnvironmentVariableService envVarService;
@@ -159,4 +165,114 @@ public void ParseDependenciesFile(ProcessRequest processRequest)
159165
var lines = sr.ReadToEnd().Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
160166
this.parserService.Parse(lines, processRequest.SingleFileComponentRecorder);
161167
}
168+
169+
/// <summary>
170+
/// Registers that a detector is actively reading a dependency file.
171+
/// This prevents premature deletion by other detectors.
172+
/// </summary>
173+
/// <param name="dependencyFilePath">The path to the dependency file being read.</param>
174+
public void RegisterFileReader(string dependencyFilePath)
175+
{
176+
this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 1, (key, count) => count + 1);
177+
}
178+
179+
/// <summary>
180+
/// Unregisters a detector's active reading of a dependency file and attempts cleanup.
181+
/// If no other detectors are reading the file, it will be safely deleted.
182+
/// </summary>
183+
/// <param name="dependencyFilePath">The path to the dependency file that was being read.</param>
184+
public void UnregisterFileReader(string dependencyFilePath)
185+
{
186+
this.fileReaderCounts.AddOrUpdate(dependencyFilePath, 0, (key, count) => Math.Max(0, count - 1));
187+
this.TryDeleteDependencyFileIfNotInUse(dependencyFilePath);
188+
}
189+
190+
/// <summary>
191+
/// Attempts to delete a dependency file if no detectors are currently using it.
192+
/// Uses cross-process coordination to prevent race conditions with other instances.
193+
/// </summary>
194+
/// <param name="dependencyFilePath">The path to the dependency file to delete.</param>
195+
private void TryDeleteDependencyFileIfNotInUse(string dependencyFilePath)
196+
{
197+
// Check if any local readers are using the file
198+
if (this.fileReaderCounts.TryGetValue(dependencyFilePath, out var count) && count > 0)
199+
{
200+
this.logger.LogDebug("Skipping deletion of {DependencyFilePath} - {Count} local readers still active", dependencyFilePath, count);
201+
return;
202+
}
203+
204+
var coordinationFile = dependencyFilePath + ".deleting";
205+
206+
try
207+
{
208+
// Try to create coordination file atomically with our process ID
209+
var processId = Environment.ProcessId.ToString();
210+
211+
// Use FileMode.CreateNew to ensure atomic creation (fails if file exists)
212+
using (var fs = new FileStream(coordinationFile, FileMode.CreateNew, FileAccess.Write, FileShare.None))
213+
using (var writer = new StreamWriter(fs))
214+
{
215+
writer.Write(processId);
216+
}
217+
218+
this.logger.LogDebug("Created coordination file {CoordinationFile} for process {ProcessId}", coordinationFile, processId);
219+
220+
// Give other processes a chance to create their coordination files if needed
221+
Thread.Sleep(50);
222+
223+
// Verify we still own the coordination (another process might have deleted and recreated it)
224+
if (!File.Exists(coordinationFile))
225+
{
226+
this.logger.LogDebug("Coordination file {CoordinationFile} was deleted by another process", coordinationFile);
227+
return;
228+
}
229+
230+
var coordinationContent = File.ReadAllText(coordinationFile).Trim();
231+
if (coordinationContent != processId)
232+
{
233+
this.logger.LogDebug("Coordination file {CoordinationFile} was taken over by process {OtherProcessId}, aborting deletion", coordinationFile, coordinationContent);
234+
return;
235+
}
236+
237+
// We own the coordination - proceed with deletion
238+
if (File.Exists(dependencyFilePath))
239+
{
240+
File.Delete(dependencyFilePath);
241+
this.logger.LogDebug("Successfully deleted dependency file {DependencyFilePath}", dependencyFilePath);
242+
}
243+
else
244+
{
245+
this.logger.LogDebug("Dependency file {DependencyFilePath} was already deleted", dependencyFilePath);
246+
}
247+
}
248+
catch (IOException ex) when (ex.Message.Contains("already exists") || ex.HResult == -2147024816)
249+
{
250+
// Another process is handling deletion (File already exists)
251+
this.logger.LogDebug("Another process is already coordinating deletion of {DependencyFilePath}", dependencyFilePath);
252+
}
253+
catch (Exception ex)
254+
{
255+
this.logger.LogWarning(ex, "Failed to coordinate deletion of dependency file {DependencyFilePath}", dependencyFilePath);
256+
}
257+
finally
258+
{
259+
// Clean up our coordination file
260+
try
261+
{
262+
if (File.Exists(coordinationFile))
263+
{
264+
var coordinationContent = File.ReadAllText(coordinationFile).Trim();
265+
if (coordinationContent == Environment.ProcessId.ToString())
266+
{
267+
File.Delete(coordinationFile);
268+
this.logger.LogDebug("Cleaned up coordination file {CoordinationFile}", coordinationFile);
269+
}
270+
}
271+
}
272+
catch (Exception ex)
273+
{
274+
this.logger.LogDebug(ex, "Failed to clean up coordination file {CoordinationFile}, will be cleaned up later", coordinationFile);
275+
}
276+
}
277+
}
162278
}

0 commit comments

Comments
 (0)