From cd764e55a5a8bc0f92f51b1a879f22ca69184a02 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 07:18:42 +0000
Subject: [PATCH 1/7] Initial plan
From 0a7b67c7db5f4b9ab30f2d54fd0bf577de45c235 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 07:43:13 +0000
Subject: [PATCH 2/7] Implement IFileSystemService enhancements with tracking,
cleanup, and logging
- Add tracking list for allocated temporary files/directories
- Implement removal from tracking list when TempFile/TempDirectory are disposed
- Add IDisposable to FileSystemService for cleanup on apphost shutdown
- Add environment variable ASPIRE_PRESERVE_TEMP_FILES to skip cleanup for debugging
- Add verbose logging when allocating files/directories and during cleanup
- Add 21 comprehensive tests covering all new functionality
- Update DI registration to set logger via factory lambda
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
---
.../DistributedApplicationBuilder.cs | 8 +-
src/Aspire.Hosting/Utils/FileSystemService.cs | 154 +++++++++++++-
.../FileSystemServiceTests.cs | 197 ++++++++++++++++++
3 files changed, 350 insertions(+), 9 deletions(-)
diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs
index 5f0a83cf547..b5c5eab8ea4 100644
--- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs
+++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs
@@ -67,7 +67,7 @@ public class DistributedApplicationBuilder : IDistributedApplicationBuilder
private readonly DistributedApplicationOptions _options;
private readonly HostApplicationBuilder _innerBuilder;
private readonly IUserSecretsManager _userSecretsManager;
- private readonly IFileSystemService _directoryService;
+ private readonly FileSystemService _directoryService;
///
public IHostEnvironment Environment => _innerBuilder.Environment;
@@ -304,7 +304,11 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
// Core things
// Create and register the directory service (first, so it can be used by other services)
_directoryService = new FileSystemService();
- _innerBuilder.Services.AddSingleton(_directoryService);
+ _innerBuilder.Services.AddSingleton(sp =>
+ {
+ _directoryService.SetLogger(sp.GetRequiredService>());
+ return _directoryService;
+ });
// Create and register the user secrets manager
var userSecretsFactory = new UserSecretsManagerFactory(_directoryService);
diff --git a/src/Aspire.Hosting/Utils/FileSystemService.cs b/src/Aspire.Hosting/Utils/FileSystemService.cs
index 0d8c3b8e82f..7edb8fa2f3d 100644
--- a/src/Aspire.Hosting/Utils/FileSystemService.cs
+++ b/src/Aspire.Hosting/Utils/FileSystemService.cs
@@ -3,28 +3,132 @@
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
+using System.Collections.Concurrent;
+using Microsoft.Extensions.Logging;
+
namespace Aspire.Hosting;
///
/// Default implementation of .
///
-internal sealed class FileSystemService : IFileSystemService
+internal sealed class FileSystemService : IFileSystemService, IDisposable
{
- private readonly TempFileSystemService _tempDirectory = new();
+ private readonly TempFileSystemService _tempDirectory;
+ private ILogger? _logger;
+ private readonly bool _preserveTempFiles;
+
+ public FileSystemService()
+ {
+ // Check environment variable to preserve temp files for debugging
+ _preserveTempFiles = Environment.GetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES") is not null;
+
+ _tempDirectory = new TempFileSystemService(this);
+ }
+
+ ///
+ /// Sets the logger for this service. Called after service provider is built.
+ ///
+ ///
+ /// The logger cannot be injected via constructor because the FileSystemService
+ /// is allocated before logging is fully initialized in the DistributedApplicationBuilder.
+ ///
+ internal void SetLogger(ILogger logger)
+ {
+ _logger = logger;
+ }
///
public ITempFileSystemService TempDirectory => _tempDirectory;
+ // Track allocated temp files and directories
+ private readonly ConcurrentDictionary _allocatedPaths = new();
+
+ internal void TrackAllocatedPath(string path, bool isDirectory)
+ {
+ _allocatedPaths.TryAdd(path, isDirectory);
+
+ if (_logger?.IsEnabled(LogLevel.Debug) == true)
+ {
+ _logger.LogDebug("Allocated temporary {Type}: {Path}", isDirectory ? "directory" : "file", path);
+ }
+ }
+
+ internal void UntrackPath(string path)
+ {
+ _allocatedPaths.TryRemove(path, out _);
+ }
+
+ internal bool ShouldPreserveTempFiles() => _preserveTempFiles;
+
+ ///
+ /// Cleans up any remaining temporary files and directories.
+ ///
+ public void Dispose()
+ {
+ if (_preserveTempFiles)
+ {
+ _logger?.LogInformation("Skipping cleanup of {Count} temporary files/directories due to ASPIRE_PRESERVE_TEMP_FILES environment variable", _allocatedPaths.Count);
+ return;
+ }
+
+ if (_allocatedPaths.IsEmpty)
+ {
+ return;
+ }
+
+ _logger?.LogDebug("Cleaning up {Count} remaining temporary files/directories", _allocatedPaths.Count);
+
+ foreach (var kvp in _allocatedPaths)
+ {
+ var path = kvp.Key;
+ var isDirectory = kvp.Value;
+
+ try
+ {
+ if (isDirectory)
+ {
+ if (Directory.Exists(path))
+ {
+ Directory.Delete(path, recursive: true);
+ _logger?.LogDebug("Cleaned up temporary directory: {Path}", path);
+ }
+ }
+ else
+ {
+ if (File.Exists(path))
+ {
+ File.Delete(path);
+ _logger?.LogDebug("Cleaned up temporary file: {Path}", path);
+ }
+ }
+ }
+ catch (Exception ex)
+ {
+ _logger?.LogWarning(ex, "Failed to clean up temporary {Type}: {Path}", isDirectory ? "directory" : "file", path);
+ }
+ }
+
+ _allocatedPaths.Clear();
+ }
+
///
/// Implementation of .
///
private sealed class TempFileSystemService : ITempFileSystemService
{
+ private readonly FileSystemService _parent;
+
+ public TempFileSystemService(FileSystemService parent)
+ {
+ _parent = parent;
+ }
+
///
public TempDirectory CreateTempSubdirectory(string? prefix = null)
{
var path = Directory.CreateTempSubdirectory(prefix ?? "aspire").FullName;
- return new DefaultTempDirectory(path);
+ _parent.TrackAllocatedPath(path, isDirectory: true);
+ return new DefaultTempDirectory(path, _parent);
}
///
@@ -33,14 +137,16 @@ public TempFile CreateTempFile(string? fileName = null)
if (fileName is null)
{
var tempFile = Path.GetTempFileName();
- return new DefaultTempFile(tempFile, deleteParentDirectory: false);
+ _parent.TrackAllocatedPath(tempFile, isDirectory: false);
+ return new DefaultTempFile(tempFile, deleteParentDirectory: false, _parent);
}
// Create a temp subdirectory and place the named file inside it
var tempDir = Directory.CreateTempSubdirectory("aspire").FullName;
var filePath = Path.Combine(tempDir, fileName);
File.Create(filePath).Dispose();
- return new DefaultTempFile(filePath, deleteParentDirectory: true);
+ _parent.TrackAllocatedPath(filePath, isDirectory: false);
+ return new DefaultTempFile(filePath, deleteParentDirectory: true, _parent);
}
}
@@ -50,16 +156,33 @@ public TempFile CreateTempFile(string? fileName = null)
private sealed class DefaultTempDirectory : TempDirectory
{
private readonly string _path;
+ private readonly FileSystemService _parent;
+ private bool _disposed;
- public DefaultTempDirectory(string path)
+ public DefaultTempDirectory(string path, FileSystemService parent)
{
_path = path;
+ _parent = parent;
}
public override string Path => _path;
public override void Dispose()
{
+ if (_disposed)
+ {
+ return;
+ }
+
+ _disposed = true;
+ _parent.UntrackPath(_path);
+
+ // Skip deletion if preserve flag is set
+ if (_parent.ShouldPreserveTempFiles())
+ {
+ return;
+ }
+
try
{
if (Directory.Exists(_path))
@@ -81,17 +204,34 @@ private sealed class DefaultTempFile : TempFile
{
private readonly string _path;
private readonly bool _deleteParentDirectory;
+ private readonly FileSystemService _parent;
+ private bool _disposed;
- public DefaultTempFile(string path, bool deleteParentDirectory)
+ public DefaultTempFile(string path, bool deleteParentDirectory, FileSystemService parent)
{
_path = path;
_deleteParentDirectory = deleteParentDirectory;
+ _parent = parent;
}
public override string Path => _path;
public override void Dispose()
{
+ if (_disposed)
+ {
+ return;
+ }
+
+ _disposed = true;
+ _parent.UntrackPath(_path);
+
+ // Skip deletion if preserve flag is set
+ if (_parent.ShouldPreserveTempFiles())
+ {
+ return;
+ }
+
try
{
if (File.Exists(_path))
diff --git a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
index 3f2eba2abfd..ac8f65e0db8 100644
--- a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
+++ b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
@@ -3,6 +3,8 @@
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
+using Microsoft.Extensions.Logging;
+
namespace Aspire.Hosting.Tests;
public class FileSystemServiceTests
@@ -194,4 +196,199 @@ public void Dispose_CalledMultipleTimes_DoesNotThrow()
tempDir.Dispose();
tempFile.Dispose();
}
+
+ [Fact]
+ public void ServiceDispose_CleansUpUndisposedFiles()
+ {
+ var service = new FileSystemService();
+
+ // Create temp files/dirs without disposing them
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var dirPath = tempDir.Path;
+ var filePath = tempFile.Path;
+
+ Assert.True(Directory.Exists(dirPath));
+ Assert.True(File.Exists(filePath));
+
+ // Dispose the service - should clean up remaining files
+ service.Dispose();
+
+ Assert.False(Directory.Exists(dirPath));
+ Assert.False(File.Exists(filePath));
+ }
+
+ [Fact]
+ public void ServiceDispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ {
+ try
+ {
+ // Set environment variable to preserve files
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
+
+ var service = new FileSystemService();
+
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var dirPath = tempDir.Path;
+ var filePath = tempFile.Path;
+
+ Assert.True(Directory.Exists(dirPath));
+ Assert.True(File.Exists(filePath));
+
+ // Dispose the service - should NOT clean up files
+ service.Dispose();
+
+ // Files should still exist
+ Assert.True(Directory.Exists(dirPath));
+ Assert.True(File.Exists(filePath));
+
+ // Clean up manually
+ Directory.Delete(dirPath, recursive: true);
+ File.Delete(filePath);
+ }
+ finally
+ {
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
+ }
+ }
+
+ [Fact]
+ public void TempDirectory_Dispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ {
+ try
+ {
+ // Set environment variable to preserve files
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
+
+ var service = new FileSystemService();
+
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var dirPath = tempDir.Path;
+
+ Assert.True(Directory.Exists(dirPath));
+
+ // Dispose the temp directory - should NOT clean up
+ tempDir.Dispose();
+
+ // Directory should still exist
+ Assert.True(Directory.Exists(dirPath));
+
+ // Clean up manually
+ Directory.Delete(dirPath, recursive: true);
+ }
+ finally
+ {
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
+ }
+ }
+
+ [Fact]
+ public void TempFile_Dispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ {
+ try
+ {
+ // Set environment variable to preserve files
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
+
+ var service = new FileSystemService();
+
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var filePath = tempFile.Path;
+
+ Assert.True(File.Exists(filePath));
+
+ // Dispose the temp file - should NOT clean up
+ tempFile.Dispose();
+
+ // File should still exist
+ Assert.True(File.Exists(filePath));
+
+ // Clean up manually
+ File.Delete(filePath);
+ }
+ finally
+ {
+ Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
+ }
+ }
+
+ [Fact]
+ public void ServiceDispose_WithMixedDisposedAndUndisposedItems_CleansUpOnlyUndisposed()
+ {
+ var service = new FileSystemService();
+
+ // Create multiple temp items
+ var tempDir1 = service.TempDirectory.CreateTempSubdirectory();
+ var tempDir2 = service.TempDirectory.CreateTempSubdirectory();
+ var tempFile1 = service.TempDirectory.CreateTempFile();
+ var tempFile2 = service.TempDirectory.CreateTempFile();
+
+ var dir1Path = tempDir1.Path;
+ var dir2Path = tempDir2.Path;
+ var file1Path = tempFile1.Path;
+ var file2Path = tempFile2.Path;
+
+ // Dispose some of them
+ tempDir1.Dispose();
+ tempFile1.Dispose();
+
+ Assert.False(Directory.Exists(dir1Path));
+ Assert.False(File.Exists(file1Path));
+ Assert.True(Directory.Exists(dir2Path));
+ Assert.True(File.Exists(file2Path));
+
+ // Dispose the service - should clean up remaining undisposed items
+ service.Dispose();
+
+ Assert.False(Directory.Exists(dir2Path));
+ Assert.False(File.Exists(file2Path));
+ }
+
+ [Fact]
+ public void ServiceDispose_EmptyTracking_DoesNotThrow()
+ {
+ var service = new FileSystemService();
+
+ // Dispose without creating any temp items
+ service.Dispose();
+
+ // Dispose again should also not throw
+ service.Dispose();
+ }
+
+ [Fact]
+ public void CreateTempFile_WithFileName_TracksOnlyFile()
+ {
+ var service = new FileSystemService();
+
+ var tempFile = service.TempDirectory.CreateTempFile("test.txt");
+ var filePath = tempFile.Path;
+ var parentDir = Path.GetDirectoryName(filePath)!;
+
+ // Both file and parent dir should exist
+ Assert.True(File.Exists(filePath));
+ Assert.True(Directory.Exists(parentDir));
+
+ // Dispose the temp file - should delete both file and parent directory
+ tempFile.Dispose();
+
+ Assert.False(File.Exists(filePath));
+ Assert.False(Directory.Exists(parentDir));
+ }
+
+ [Fact]
+ public void SetLogger_CanBeCalledWithoutError()
+ {
+ var service = new FileSystemService();
+ var loggerFactory = LoggerFactory.Create(builder => builder.AddConsole());
+ var logger = loggerFactory.CreateLogger();
+
+ // Should not throw
+ service.SetLogger(logger);
+
+ // Should still work normally
+ using var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ Assert.True(Directory.Exists(tempDir.Path));
+ }
}
From ecd7a6fd31fcf3b0af7373df315d516308f341a9 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 08:06:23 +0000
Subject: [PATCH 3/7] Use IConfiguration instead of
Environment.GetEnvironmentVariable and simplify tracking with
ConcurrentDictionary
- Replace Environment.GetEnvironmentVariable with IConfiguration["ASPIRE_PRESERVE_TEMP_FILES"]
- Pass IConfiguration to FileSystemService constructor from DistributedApplicationBuilder
- Use ConcurrentDictionary to track temp items instead of ConcurrentBag
- Items automatically untrack themselves on dispose via UntrackItem
- Update all tests to pass IConfiguration to FileSystemService constructor
- Add necessary using statements to test files
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
---
.../DistributedApplicationBuilder.cs | 2 +-
src/Aspire.Hosting/Utils/FileSystemService.cs | 91 +++++-----
.../Aspire.Hosting.Tests/AspireStoreTests.cs | 3 +-
.../Dashboard/DashboardLifecycleHookTests.cs | 2 +-
.../Dcp/DcpExecutorTests.cs | 2 +-
.../Dcp/DcpHostNotificationTests.cs | 3 +-
.../FileSystemServiceTests.cs | 161 ++++++++----------
.../Aspire.Hosting.Tests/SecretsStoreTests.cs | 2 +-
.../UserSecretsParameterDefaultTests.cs | 2 +-
9 files changed, 123 insertions(+), 145 deletions(-)
diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs
index b5c5eab8ea4..cac4a160f38 100644
--- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs
+++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs
@@ -303,7 +303,7 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options)
// Core things
// Create and register the directory service (first, so it can be used by other services)
- _directoryService = new FileSystemService();
+ _directoryService = new FileSystemService(_innerBuilder.Configuration);
_innerBuilder.Services.AddSingleton(sp =>
{
_directoryService.SetLogger(sp.GetRequiredService>());
diff --git a/src/Aspire.Hosting/Utils/FileSystemService.cs b/src/Aspire.Hosting/Utils/FileSystemService.cs
index 7edb8fa2f3d..7864f84c533 100644
--- a/src/Aspire.Hosting/Utils/FileSystemService.cs
+++ b/src/Aspire.Hosting/Utils/FileSystemService.cs
@@ -4,6 +4,7 @@
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
using System.Collections.Concurrent;
+using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
namespace Aspire.Hosting;
@@ -17,10 +18,15 @@ internal sealed class FileSystemService : IFileSystemService, IDisposable
private ILogger? _logger;
private readonly bool _preserveTempFiles;
- public FileSystemService()
+ // Track allocated temp files and directories as disposable objects using path as key
+ private readonly ConcurrentDictionary _allocatedItems = new();
+
+ public FileSystemService(IConfiguration configuration)
{
- // Check environment variable to preserve temp files for debugging
- _preserveTempFiles = Environment.GetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES") is not null;
+ ArgumentNullException.ThrowIfNull(configuration);
+
+ // Check configuration to preserve temp files for debugging
+ _preserveTempFiles = configuration["ASPIRE_PRESERVE_TEMP_FILES"] is not null;
_tempDirectory = new TempFileSystemService(this);
}
@@ -40,26 +46,20 @@ internal void SetLogger(ILogger logger)
///
public ITempFileSystemService TempDirectory => _tempDirectory;
- // Track allocated temp files and directories
- private readonly ConcurrentDictionary _allocatedPaths = new();
-
- internal void TrackAllocatedPath(string path, bool isDirectory)
+ internal void TrackItem(string path, IDisposable item)
{
- _allocatedPaths.TryAdd(path, isDirectory);
-
- if (_logger?.IsEnabled(LogLevel.Debug) == true)
- {
- _logger.LogDebug("Allocated temporary {Type}: {Path}", isDirectory ? "directory" : "file", path);
- }
+ _allocatedItems.TryAdd(path, item);
}
- internal void UntrackPath(string path)
+ internal void UntrackItem(string path)
{
- _allocatedPaths.TryRemove(path, out _);
+ _allocatedItems.TryRemove(path, out _);
}
internal bool ShouldPreserveTempFiles() => _preserveTempFiles;
+ internal ILogger? Logger => _logger;
+
///
/// Cleans up any remaining temporary files and directories.
///
@@ -67,48 +67,28 @@ public void Dispose()
{
if (_preserveTempFiles)
{
- _logger?.LogInformation("Skipping cleanup of {Count} temporary files/directories due to ASPIRE_PRESERVE_TEMP_FILES environment variable", _allocatedPaths.Count);
+ _logger?.LogInformation("Skipping cleanup of {Count} temporary files/directories due to ASPIRE_PRESERVE_TEMP_FILES configuration", _allocatedItems.Count);
return;
}
- if (_allocatedPaths.IsEmpty)
+ if (_allocatedItems.IsEmpty)
{
return;
}
- _logger?.LogDebug("Cleaning up {Count} remaining temporary files/directories", _allocatedPaths.Count);
+ _logger?.LogDebug("Cleaning up {Count} remaining temporary files/directories", _allocatedItems.Count);
- foreach (var kvp in _allocatedPaths)
+ foreach (var kvp in _allocatedItems)
{
- var path = kvp.Key;
- var isDirectory = kvp.Value;
-
try
{
- if (isDirectory)
- {
- if (Directory.Exists(path))
- {
- Directory.Delete(path, recursive: true);
- _logger?.LogDebug("Cleaned up temporary directory: {Path}", path);
- }
- }
- else
- {
- if (File.Exists(path))
- {
- File.Delete(path);
- _logger?.LogDebug("Cleaned up temporary file: {Path}", path);
- }
- }
+ kvp.Value.Dispose();
}
catch (Exception ex)
{
- _logger?.LogWarning(ex, "Failed to clean up temporary {Type}: {Path}", isDirectory ? "directory" : "file", path);
+ _logger?.LogWarning(ex, "Failed to clean up temporary item");
}
}
-
- _allocatedPaths.Clear();
}
///
@@ -127,8 +107,9 @@ public TempFileSystemService(FileSystemService parent)
public TempDirectory CreateTempSubdirectory(string? prefix = null)
{
var path = Directory.CreateTempSubdirectory(prefix ?? "aspire").FullName;
- _parent.TrackAllocatedPath(path, isDirectory: true);
- return new DefaultTempDirectory(path, _parent);
+ var tempDir = new DefaultTempDirectory(path, _parent);
+ _parent.TrackItem(path, tempDir);
+ return tempDir;
}
///
@@ -137,16 +118,18 @@ public TempFile CreateTempFile(string? fileName = null)
if (fileName is null)
{
var tempFile = Path.GetTempFileName();
- _parent.TrackAllocatedPath(tempFile, isDirectory: false);
- return new DefaultTempFile(tempFile, deleteParentDirectory: false, _parent);
+ var file = new DefaultTempFile(tempFile, deleteParentDirectory: false, _parent);
+ _parent.TrackItem(tempFile, file);
+ return file;
}
// Create a temp subdirectory and place the named file inside it
var tempDir = Directory.CreateTempSubdirectory("aspire").FullName;
var filePath = Path.Combine(tempDir, fileName);
File.Create(filePath).Dispose();
- _parent.TrackAllocatedPath(filePath, isDirectory: false);
- return new DefaultTempFile(filePath, deleteParentDirectory: true, _parent);
+ var tempFileObj = new DefaultTempFile(filePath, deleteParentDirectory: true, _parent);
+ _parent.TrackItem(filePath, tempFileObj);
+ return tempFileObj;
}
}
@@ -163,6 +146,8 @@ public DefaultTempDirectory(string path, FileSystemService parent)
{
_path = path;
_parent = parent;
+
+ _parent.Logger?.LogDebug("Allocated temporary directory: {Path}", path);
}
public override string Path => _path;
@@ -175,7 +160,9 @@ public override void Dispose()
}
_disposed = true;
- _parent.UntrackPath(_path);
+
+ // Remove from tracking
+ _parent.UntrackItem(_path);
// Skip deletion if preserve flag is set
if (_parent.ShouldPreserveTempFiles())
@@ -188,6 +175,7 @@ public override void Dispose()
if (Directory.Exists(_path))
{
Directory.Delete(_path, recursive: true);
+ _parent.Logger?.LogDebug("Cleaned up temporary directory: {Path}", _path);
}
}
catch
@@ -212,6 +200,8 @@ public DefaultTempFile(string path, bool deleteParentDirectory, FileSystemServic
_path = path;
_deleteParentDirectory = deleteParentDirectory;
_parent = parent;
+
+ _parent.Logger?.LogDebug("Allocated temporary file: {Path}", path);
}
public override string Path => _path;
@@ -224,7 +214,9 @@ public override void Dispose()
}
_disposed = true;
- _parent.UntrackPath(_path);
+
+ // Remove from tracking
+ _parent.UntrackItem(_path);
// Skip deletion if preserve flag is set
if (_parent.ShouldPreserveTempFiles())
@@ -237,6 +229,7 @@ public override void Dispose()
if (File.Exists(_path))
{
File.Delete(_path);
+ _parent.Logger?.LogDebug("Cleaned up temporary file: {Path}", _path);
}
if (_deleteParentDirectory)
diff --git a/tests/Aspire.Hosting.Tests/AspireStoreTests.cs b/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
index e1f98a4ea0e..f7a3111f851 100644
--- a/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
+++ b/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
@@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
using Aspire.Hosting.Utils;
+using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
namespace Aspire.Hosting.Tests;
@@ -123,7 +124,7 @@ public void GetOrCreateFileWithContent_ShouldNotRecreateFile()
[InlineData("obj/")]
public void AspireStoreConstructor_ShouldThrow_IfNotAbsolutePath(string? basePath)
{
- var directoryService = new FileSystemService();
+ var directoryService = new FileSystemService(new ConfigurationBuilder().Build());
Assert.ThrowsAny(() => new AspireStore(basePath!, directoryService));
}
diff --git a/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs b/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
index 8dcb0040f27..1437986ab5e 100644
--- a/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
+++ b/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
@@ -523,7 +523,7 @@ private static DashboardEventHandlers CreateHook(
new TestHostApplicationLifetime(),
new Hosting.Eventing.DistributedApplicationEventing(),
rewriter,
- new FileSystemService()
+ new FileSystemService(new ConfigurationBuilder().Build())
);
}
diff --git a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
index afdd17f6ec0..dcef98110f3 100644
--- a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
+++ b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
@@ -2086,7 +2086,7 @@ private static DcpExecutor CreateAppExecutor(
new TestDcpDependencyCheckService(),
new DcpNameGenerator(configuration, Options.Create(dcpOptions)),
events ?? new DcpExecutorEvents(),
- new Locations(new FileSystemService()),
+ new Locations(new FileSystemService(new ConfigurationBuilder().Build())),
developerCertificateService);
#pragma warning restore ASPIRECERTIFICATES001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
}
diff --git a/tests/Aspire.Hosting.Tests/Dcp/DcpHostNotificationTests.cs b/tests/Aspire.Hosting.Tests/Dcp/DcpHostNotificationTests.cs
index ea21c987b51..e03ef806cd1 100644
--- a/tests/Aspire.Hosting.Tests/Dcp/DcpHostNotificationTests.cs
+++ b/tests/Aspire.Hosting.Tests/Dcp/DcpHostNotificationTests.cs
@@ -5,6 +5,7 @@
using Aspire.Hosting.Dcp;
using Aspire.Hosting.Resources;
using Microsoft.AspNetCore.InternalTesting;
+using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
@@ -18,7 +19,7 @@ public sealed class DcpHostNotificationTests
{
private static Locations CreateTestLocations()
{
- var directoryService = new FileSystemService();
+ var directoryService = new FileSystemService(new ConfigurationBuilder().Build());
return new Locations(directoryService);
}
diff --git a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
index ac8f65e0db8..0705ec4e1a7 100644
--- a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
+++ b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
@@ -3,16 +3,29 @@
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
+using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
namespace Aspire.Hosting.Tests;
public class FileSystemServiceTests
{
+ private static IConfiguration CreateConfiguration(bool preserveTempFiles = false)
+ {
+ var configDict = new Dictionary();
+ if (preserveTempFiles)
+ {
+ configDict["ASPIRE_PRESERVE_TEMP_FILES"] = "true";
+ }
+ return new ConfigurationBuilder()
+ .AddInMemoryCollection(configDict)
+ .Build();
+ }
+
[Fact]
public void CreateTempSubdirectory_CreatesDirectory()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempDir = service.TempDirectory.CreateTempSubdirectory();
@@ -23,7 +36,7 @@ public void CreateTempSubdirectory_CreatesDirectory()
[Fact]
public void CreateTempSubdirectory_WithPrefix_CreatesDirectoryWithPrefix()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempDir = service.TempDirectory.CreateTempSubdirectory("test-prefix");
@@ -35,7 +48,7 @@ public void CreateTempSubdirectory_WithPrefix_CreatesDirectoryWithPrefix()
[Fact]
public void CreateTempSubdirectory_Dispose_DeletesDirectory()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempDir = service.TempDirectory.CreateTempSubdirectory();
var path = tempDir.Path;
@@ -49,7 +62,7 @@ public void CreateTempSubdirectory_Dispose_DeletesDirectory()
[Fact]
public void CreateTempFile_CreatesFile()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempFile = service.TempDirectory.CreateTempFile();
@@ -60,7 +73,7 @@ public void CreateTempFile_CreatesFile()
[Fact]
public void CreateTempFile_WithFileName_CreatesNamedFile()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempFile = service.TempDirectory.CreateTempFile("config.json");
@@ -72,7 +85,7 @@ public void CreateTempFile_WithFileName_CreatesNamedFile()
[Fact]
public void CreateTempFile_Dispose_DeletesFile()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempFile = service.TempDirectory.CreateTempFile();
var path = tempFile.Path;
@@ -86,7 +99,7 @@ public void CreateTempFile_Dispose_DeletesFile()
[Fact]
public void CreateTempFile_WithFileName_Dispose_DeletesFileAndParentDirectory()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempFile = service.TempDirectory.CreateTempFile("test-file.txt");
var filePath = tempFile.Path;
var parentDir = Path.GetDirectoryName(filePath);
@@ -105,7 +118,7 @@ public void PathExtractionPattern_DirectoryPersistsAfterScopeEnds()
{
// This test verifies the intentional pattern of extracting .Path
// without disposing, which is common in the codebase
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
string? extractedPath;
// Simulate the common pattern: extract path, let object go out of scope
@@ -127,7 +140,7 @@ public void PathExtractionPattern_FilePersistsAfterScopeEnds()
{
// This test verifies the intentional pattern of extracting .Path
// without disposing, which is common in the codebase
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
string? extractedPath;
// Simulate the common pattern: extract path, let object go out of scope
@@ -147,7 +160,7 @@ public void PathExtractionPattern_FilePersistsAfterScopeEnds()
[Fact]
public void TempDirectory_Property_ReturnsSameInstance()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempDir1 = service.TempDirectory;
var tempDir2 = service.TempDirectory;
@@ -158,7 +171,7 @@ public void TempDirectory_Property_ReturnsSameInstance()
[Fact]
public void CreateTempSubdirectory_MultipleCallsCreateDifferentDirectories()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempDir1 = service.TempDirectory.CreateTempSubdirectory();
using var tempDir2 = service.TempDirectory.CreateTempSubdirectory();
@@ -171,7 +184,7 @@ public void CreateTempSubdirectory_MultipleCallsCreateDifferentDirectories()
[Fact]
public void CreateTempFile_MultipleCallsCreateDifferentFiles()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
using var tempFile1 = service.TempDirectory.CreateTempFile();
using var tempFile2 = service.TempDirectory.CreateTempFile();
@@ -184,7 +197,7 @@ public void CreateTempFile_MultipleCallsCreateDifferentFiles()
[Fact]
public void Dispose_CalledMultipleTimes_DoesNotThrow()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempDir = service.TempDirectory.CreateTempSubdirectory();
var tempFile = service.TempDirectory.CreateTempFile();
@@ -200,7 +213,7 @@ public void Dispose_CalledMultipleTimes_DoesNotThrow()
[Fact]
public void ServiceDispose_CleansUpUndisposedFiles()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
// Create temp files/dirs without disposing them
var tempDir = service.TempDirectory.CreateTempSubdirectory();
@@ -219,104 +232,74 @@ public void ServiceDispose_CleansUpUndisposedFiles()
}
[Fact]
- public void ServiceDispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ public void ServiceDispose_WithPreserveConfiguration_SkipsCleanup()
{
- try
- {
- // Set environment variable to preserve files
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
-
- var service = new FileSystemService();
-
- var tempDir = service.TempDirectory.CreateTempSubdirectory();
- var tempFile = service.TempDirectory.CreateTempFile();
- var dirPath = tempDir.Path;
- var filePath = tempFile.Path;
+ var service = new FileSystemService(CreateConfiguration(preserveTempFiles: true));
+
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var dirPath = tempDir.Path;
+ var filePath = tempFile.Path;
- Assert.True(Directory.Exists(dirPath));
- Assert.True(File.Exists(filePath));
+ Assert.True(Directory.Exists(dirPath));
+ Assert.True(File.Exists(filePath));
- // Dispose the service - should NOT clean up files
- service.Dispose();
+ // Dispose the service - should NOT clean up files
+ service.Dispose();
- // Files should still exist
- Assert.True(Directory.Exists(dirPath));
- Assert.True(File.Exists(filePath));
+ // Files should still exist
+ Assert.True(Directory.Exists(dirPath));
+ Assert.True(File.Exists(filePath));
- // Clean up manually
- Directory.Delete(dirPath, recursive: true);
- File.Delete(filePath);
- }
- finally
- {
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
- }
+ // Clean up manually
+ Directory.Delete(dirPath, recursive: true);
+ File.Delete(filePath);
}
[Fact]
- public void TempDirectory_Dispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ public void TempDirectory_Dispose_WithPreserveConfiguration_SkipsCleanup()
{
- try
- {
- // Set environment variable to preserve files
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
-
- var service = new FileSystemService();
-
- var tempDir = service.TempDirectory.CreateTempSubdirectory();
- var dirPath = tempDir.Path;
+ var service = new FileSystemService(CreateConfiguration(preserveTempFiles: true));
+
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var dirPath = tempDir.Path;
- Assert.True(Directory.Exists(dirPath));
+ Assert.True(Directory.Exists(dirPath));
- // Dispose the temp directory - should NOT clean up
- tempDir.Dispose();
+ // Dispose the temp directory - should NOT clean up
+ tempDir.Dispose();
- // Directory should still exist
- Assert.True(Directory.Exists(dirPath));
+ // Directory should still exist
+ Assert.True(Directory.Exists(dirPath));
- // Clean up manually
- Directory.Delete(dirPath, recursive: true);
- }
- finally
- {
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
- }
+ // Clean up manually
+ Directory.Delete(dirPath, recursive: true);
}
[Fact]
- public void TempFile_Dispose_WithPreserveEnvironmentVariable_SkipsCleanup()
+ public void TempFile_Dispose_WithPreserveConfiguration_SkipsCleanup()
{
- try
- {
- // Set environment variable to preserve files
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", "1");
-
- var service = new FileSystemService();
-
- var tempFile = service.TempDirectory.CreateTempFile();
- var filePath = tempFile.Path;
+ var service = new FileSystemService(CreateConfiguration(preserveTempFiles: true));
+
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var filePath = tempFile.Path;
- Assert.True(File.Exists(filePath));
+ Assert.True(File.Exists(filePath));
- // Dispose the temp file - should NOT clean up
- tempFile.Dispose();
+ // Dispose the temp file - should NOT clean up
+ tempFile.Dispose();
- // File should still exist
- Assert.True(File.Exists(filePath));
+ // File should still exist
+ Assert.True(File.Exists(filePath));
- // Clean up manually
- File.Delete(filePath);
- }
- finally
- {
- Environment.SetEnvironmentVariable("ASPIRE_PRESERVE_TEMP_FILES", null);
- }
+ // Clean up manually
+ File.Delete(filePath);
}
[Fact]
public void ServiceDispose_WithMixedDisposedAndUndisposedItems_CleansUpOnlyUndisposed()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
// Create multiple temp items
var tempDir1 = service.TempDirectory.CreateTempSubdirectory();
@@ -348,7 +331,7 @@ public void ServiceDispose_WithMixedDisposedAndUndisposedItems_CleansUpOnlyUndis
[Fact]
public void ServiceDispose_EmptyTracking_DoesNotThrow()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
// Dispose without creating any temp items
service.Dispose();
@@ -360,7 +343,7 @@ public void ServiceDispose_EmptyTracking_DoesNotThrow()
[Fact]
public void CreateTempFile_WithFileName_TracksOnlyFile()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var tempFile = service.TempDirectory.CreateTempFile("test.txt");
var filePath = tempFile.Path;
@@ -380,7 +363,7 @@ public void CreateTempFile_WithFileName_TracksOnlyFile()
[Fact]
public void SetLogger_CanBeCalledWithoutError()
{
- var service = new FileSystemService();
+ var service = new FileSystemService(CreateConfiguration());
var loggerFactory = LoggerFactory.Create(builder => builder.AddConsole());
var logger = loggerFactory.CreateLogger();
diff --git a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
index 18a4e967bf5..f5dc1adf4a9 100644
--- a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
+++ b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
@@ -16,7 +16,7 @@ public class SecretsStoreTests
{
private static readonly ConstructorInfo s_userSecretsIdAttrCtor = typeof(UserSecretsIdAttribute).GetConstructor([typeof(string)])!;
- private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService());
+ private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService(new ConfigurationBuilder().Build()));
[Fact]
public void GetOrSetUserSecret_SavesValueToUserSecrets()
diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
index 40360a87eda..fa5f14f349d 100644
--- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
+++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
@@ -18,7 +18,7 @@ public class UserSecretsParameterDefaultTests
{
private static readonly ConstructorInfo s_userSecretsIdAttrCtor = typeof(UserSecretsIdAttribute).GetConstructor([typeof(string)])!;
- private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService());
+ private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService(new ConfigurationBuilder().Build()));
[Fact]
public void UserSecretsParameterDefault_GetDefaultValue_SavesValueInAppHostUserSecrets()
From 72164347ef6ac7b08d07e6c76bc42a0a39010db7 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 08:35:03 +0000
Subject: [PATCH 4/7] Add idempotent dispose pattern and XML documentation for
internal methods
- Add _disposed flag to FileSystemService.Dispose() to prevent multiple disposals
- Add XML documentation to internal methods: TrackItem, UntrackItem, ShouldPreserveTempFiles, and Logger
- Add test for idempotent dispose behavior
- Addresses bot feedback while keeping test cleanup code correct as-is
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
---
src/Aspire.Hosting/Utils/FileSystemService.cs | 21 +++++++++++++++++++
.../FileSystemServiceTests.cs | 21 +++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/src/Aspire.Hosting/Utils/FileSystemService.cs b/src/Aspire.Hosting/Utils/FileSystemService.cs
index 7864f84c533..25f72e2a8b7 100644
--- a/src/Aspire.Hosting/Utils/FileSystemService.cs
+++ b/src/Aspire.Hosting/Utils/FileSystemService.cs
@@ -46,25 +46,46 @@ internal void SetLogger(ILogger logger)
///
public ITempFileSystemService TempDirectory => _tempDirectory;
+ ///
+ /// Tracks a temporary item for cleanup on service disposal.
+ ///
internal void TrackItem(string path, IDisposable item)
{
_allocatedItems.TryAdd(path, item);
}
+ ///
+ /// Removes a temporary item from tracking.
+ ///
internal void UntrackItem(string path)
{
_allocatedItems.TryRemove(path, out _);
}
+ ///
+ /// Gets whether temporary files should be preserved for debugging.
+ ///
internal bool ShouldPreserveTempFiles() => _preserveTempFiles;
+ ///
+ /// Gets the logger for this service, if set.
+ ///
internal ILogger? Logger => _logger;
+ private bool _disposed;
+
///
/// Cleans up any remaining temporary files and directories.
///
public void Dispose()
{
+ if (_disposed)
+ {
+ return;
+ }
+
+ _disposed = true;
+
if (_preserveTempFiles)
{
_logger?.LogInformation("Skipping cleanup of {Count} temporary files/directories due to ASPIRE_PRESERVE_TEMP_FILES configuration", _allocatedItems.Count);
diff --git a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
index 0705ec4e1a7..f7b795bcfee 100644
--- a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
+++ b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
@@ -340,6 +340,27 @@ public void ServiceDispose_EmptyTracking_DoesNotThrow()
service.Dispose();
}
+ [Fact]
+ public void ServiceDispose_CalledMultipleTimes_IsIdempotent()
+ {
+ var service = new FileSystemService(CreateConfiguration());
+
+ // Create temp items
+ var tempDir = service.TempDirectory.CreateTempSubdirectory();
+ var tempFile = service.TempDirectory.CreateTempFile();
+ var dirPath = tempDir.Path;
+ var filePath = tempFile.Path;
+
+ // First dispose - should clean up
+ service.Dispose();
+
+ Assert.False(Directory.Exists(dirPath));
+ Assert.False(File.Exists(filePath));
+
+ // Second dispose - should be a no-op and not throw
+ service.Dispose();
+ }
+
[Fact]
public void CreateTempFile_WithFileName_TracksOnlyFile()
{
From 87cabce95e8ca2bb114e050a1f57ee3c52e625ae Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 09:31:23 +0000
Subject: [PATCH 5/7] Make TrackItem thread-safe by throwing
ObjectDisposedException when disposed
- TrackItem now checks _disposed flag and throws ObjectDisposedException if service is disposed
- Prevents new temp files/directories from being allocated during or after disposal
- Add tests verifying ObjectDisposedException is thrown when creating temp files/directories after disposal
- Addresses thread safety concern from @davidfowl and @mitchdenny
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
---
src/Aspire.Hosting/Utils/FileSystemService.cs | 5 ++++
.../FileSystemServiceTests.cs | 24 +++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/src/Aspire.Hosting/Utils/FileSystemService.cs b/src/Aspire.Hosting/Utils/FileSystemService.cs
index 25f72e2a8b7..4b870af93f2 100644
--- a/src/Aspire.Hosting/Utils/FileSystemService.cs
+++ b/src/Aspire.Hosting/Utils/FileSystemService.cs
@@ -51,6 +51,11 @@ internal void SetLogger(ILogger logger)
///
internal void TrackItem(string path, IDisposable item)
{
+ if (_disposed)
+ {
+ throw new ObjectDisposedException(nameof(FileSystemService), "Cannot allocate temporary files after the service has been disposed.");
+ }
+
_allocatedItems.TryAdd(path, item);
}
diff --git a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
index f7b795bcfee..f621cda9631 100644
--- a/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
+++ b/tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs
@@ -381,6 +381,30 @@ public void CreateTempFile_WithFileName_TracksOnlyFile()
Assert.False(Directory.Exists(parentDir));
}
+ [Fact]
+ public void CreateTempFile_AfterServiceDisposed_ThrowsObjectDisposedException()
+ {
+ var service = new FileSystemService(CreateConfiguration());
+
+ // Dispose the service
+ service.Dispose();
+
+ // Attempting to create a temp file after disposal should throw
+ Assert.Throws(() => service.TempDirectory.CreateTempFile());
+ }
+
+ [Fact]
+ public void CreateTempDirectory_AfterServiceDisposed_ThrowsObjectDisposedException()
+ {
+ var service = new FileSystemService(CreateConfiguration());
+
+ // Dispose the service
+ service.Dispose();
+
+ // Attempting to create a temp directory after disposal should throw
+ Assert.Throws(() => service.TempDirectory.CreateTempSubdirectory());
+ }
+
[Fact]
public void SetLogger_CanBeCalledWithoutError()
{
From a0db49ceeb74a80d0d1bb23cf860166a72da6dc0 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Dec 2025 10:15:14 +0000
Subject: [PATCH 6/7] Fix race condition with lock and improve type safety
- Add lock (_disposeLock) around _disposed check and TryAdd in TrackItem to prevent race condition
- Add lock in Dispose() to atomically set _disposed flag
- Change Logger property type from ILogger? to ILogger? for flexibility
- Change _logger field type from ILogger? to ILogger?
- Convert ShouldPreserveTempFiles() method to property (remove parentheses)
- Update all callers of ShouldPreserveTempFiles to use property syntax
- Addresses thread safety concerns from @davidfowl and feedback on property/type improvements
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
---
src/Aspire.Hosting/Utils/FileSystemService.cs | 53 +++++++++++--------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/src/Aspire.Hosting/Utils/FileSystemService.cs b/src/Aspire.Hosting/Utils/FileSystemService.cs
index 4b870af93f2..0099c70f066 100644
--- a/src/Aspire.Hosting/Utils/FileSystemService.cs
+++ b/src/Aspire.Hosting/Utils/FileSystemService.cs
@@ -15,7 +15,7 @@ namespace Aspire.Hosting;
internal sealed class FileSystemService : IFileSystemService, IDisposable
{
private readonly TempFileSystemService _tempDirectory;
- private ILogger? _logger;
+ private ILogger? _logger;
private readonly bool _preserveTempFiles;
// Track allocated temp files and directories as disposable objects using path as key
@@ -46,17 +46,33 @@ internal void SetLogger(ILogger logger)
///
public ITempFileSystemService TempDirectory => _tempDirectory;
+ ///
+ /// Gets whether temporary files should be preserved for debugging.
+ ///
+ internal bool ShouldPreserveTempFiles => _preserveTempFiles;
+
+ ///
+ /// Gets the logger for this service, if set.
+ ///
+ internal ILogger? Logger => _logger;
+
+ private bool _disposed;
+ private readonly object _disposeLock = new();
+
///
/// Tracks a temporary item for cleanup on service disposal.
///
internal void TrackItem(string path, IDisposable item)
{
- if (_disposed)
+ lock (_disposeLock)
{
- throw new ObjectDisposedException(nameof(FileSystemService), "Cannot allocate temporary files after the service has been disposed.");
- }
+ if (_disposed)
+ {
+ throw new ObjectDisposedException(nameof(FileSystemService), "Cannot allocate temporary files after the service has been disposed.");
+ }
- _allocatedItems.TryAdd(path, item);
+ _allocatedItems.TryAdd(path, item);
+ }
}
///
@@ -67,29 +83,20 @@ internal void UntrackItem(string path)
_allocatedItems.TryRemove(path, out _);
}
- ///
- /// Gets whether temporary files should be preserved for debugging.
- ///
- internal bool ShouldPreserveTempFiles() => _preserveTempFiles;
-
- ///
- /// Gets the logger for this service, if set.
- ///
- internal ILogger? Logger => _logger;
-
- private bool _disposed;
-
///
/// Cleans up any remaining temporary files and directories.
///
public void Dispose()
{
- if (_disposed)
+ lock (_disposeLock)
{
- return;
- }
+ if (_disposed)
+ {
+ return;
+ }
- _disposed = true;
+ _disposed = true;
+ }
if (_preserveTempFiles)
{
@@ -191,7 +198,7 @@ public override void Dispose()
_parent.UntrackItem(_path);
// Skip deletion if preserve flag is set
- if (_parent.ShouldPreserveTempFiles())
+ if (_parent.ShouldPreserveTempFiles)
{
return;
}
@@ -245,7 +252,7 @@ public override void Dispose()
_parent.UntrackItem(_path);
// Skip deletion if preserve flag is set
- if (_parent.ShouldPreserveTempFiles())
+ if (_parent.ShouldPreserveTempFiles)
{
return;
}
From 58b2518793946c12c83fd433abce5daddfa70c4f Mon Sep 17 00:00:00 2001
From: Mitch Denny
Date: Mon, 8 Dec 2025 09:46:17 +1100
Subject: [PATCH 7/7] Minor tweaks.
---
tests/Aspire.Hosting.Tests/AspireStoreTests.cs | 2 ++
.../Dashboard/DashboardLifecycleHookTests.cs | 2 +-
tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs | 2 +-
tests/Aspire.Hosting.Tests/SecretsStoreTests.cs | 7 ++++++-
.../UserSecretsParameterDefaultTests.cs | 7 ++++++-
5 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/tests/Aspire.Hosting.Tests/AspireStoreTests.cs b/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
index f7a3111f851..aebdfc2fdc7 100644
--- a/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
+++ b/tests/Aspire.Hosting.Tests/AspireStoreTests.cs
@@ -120,8 +120,10 @@ public void GetOrCreateFileWithContent_ShouldNotRecreateFile()
[InlineData(null)]
[InlineData("")]
[InlineData("./folder")]
+ [InlineData(".\\folder")]
[InlineData("folder")]
[InlineData("obj/")]
+ [InlineData("obj\\")]
public void AspireStoreConstructor_ShouldThrow_IfNotAbsolutePath(string? basePath)
{
var directoryService = new FileSystemService(new ConfigurationBuilder().Build());
diff --git a/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs b/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
index 1437986ab5e..4dda4322ff4 100644
--- a/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
+++ b/tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs
@@ -523,7 +523,7 @@ private static DashboardEventHandlers CreateHook(
new TestHostApplicationLifetime(),
new Hosting.Eventing.DistributedApplicationEventing(),
rewriter,
- new FileSystemService(new ConfigurationBuilder().Build())
+ new FileSystemService(configuration)
);
}
diff --git a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
index dcef98110f3..067f58eb723 100644
--- a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
+++ b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs
@@ -2086,7 +2086,7 @@ private static DcpExecutor CreateAppExecutor(
new TestDcpDependencyCheckService(),
new DcpNameGenerator(configuration, Options.Create(dcpOptions)),
events ?? new DcpExecutorEvents(),
- new Locations(new FileSystemService(new ConfigurationBuilder().Build())),
+ new Locations(new FileSystemService(configuration ?? new ConfigurationBuilder().Build())),
developerCertificateService);
#pragma warning restore ASPIRECERTIFICATES001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
}
diff --git a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
index f5dc1adf4a9..bf88084a0d9 100644
--- a/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
+++ b/tests/Aspire.Hosting.Tests/SecretsStoreTests.cs
@@ -16,7 +16,12 @@ public class SecretsStoreTests
{
private static readonly ConstructorInfo s_userSecretsIdAttrCtor = typeof(UserSecretsIdAttribute).GetConstructor([typeof(string)])!;
- private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService(new ConfigurationBuilder().Build()));
+ private static UserSecretsManagerFactory CreateFactory()
+ {
+ return new UserSecretsManagerFactory(
+ new FileSystemService(
+ new ConfigurationBuilder().Build()));
+ }
[Fact]
public void GetOrSetUserSecret_SavesValueToUserSecrets()
diff --git a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
index fa5f14f349d..9e6b01fa173 100644
--- a/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
+++ b/tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs
@@ -18,7 +18,12 @@ public class UserSecretsParameterDefaultTests
{
private static readonly ConstructorInfo s_userSecretsIdAttrCtor = typeof(UserSecretsIdAttribute).GetConstructor([typeof(string)])!;
- private static UserSecretsManagerFactory CreateFactory() => new UserSecretsManagerFactory(new FileSystemService(new ConfigurationBuilder().Build()));
+ private static UserSecretsManagerFactory CreateFactory()
+ {
+ return new UserSecretsManagerFactory(
+ new FileSystemService(
+ new ConfigurationBuilder().Build()));
+ }
[Fact]
public void UserSecretsParameterDefault_GetDefaultValue_SavesValueInAppHostUserSecrets()