Add automatic temp file tracking and cleanup to FileSystemService #13361

Copilot · 2025-12-06T07:18:43Z

Description

Enhances FileSystemService with automatic tracking and cleanup of temporary files/directories on AppHost shutdown, plus debugging support via configuration.

This PR implements comprehensive temporary file management to prevent temp file leaks and ensure proper cleanup when the AppHost shuts down.

Changes

Automatic tracking: ConcurrentDictionary<string, IDisposable> tracks all allocated temp files/directories using path as key
Shutdown cleanup: FileSystemService implements IDisposable to clean up remaining temp files when AppHost disposes
Dispose integration: TempFile and TempDirectory automatically remove themselves from tracking on dispose
Debug preservation: ASPIRE_PRESERVE_TEMP_FILES configuration setting skips all cleanup for debugging scenarios
Verbose logging: Debug-level logs for allocation and cleanup operations, info-level for preservation
Configuration-based: Uses IConfiguration instead of environment variables for settings
Idempotent dispose: Multiple calls to Dispose() are safe and will not cause errors
Thread-safe allocation: Lock-based synchronization prevents race conditions between allocation and disposal
- TrackItem uses lock to atomically check _disposed flag and add items
- Dispose() uses the same lock to atomically set _disposed flag
- Throws ObjectDisposedException when attempting to allocate temp files/directories after disposal

Implementation Notes

FileSystemService accepts IConfiguration in constructor, passed from DistributedApplicationBuilder
Logger is set via factory lambda during DI registration because FileSystemService is instantiated before logging initialization
Logger typed as ILogger? for flexibility
Thread-safe tracking using ConcurrentDictionary<string, IDisposable> where items untrack themselves on dispose
Lock-based synchronization (_disposeLock) ensures atomic operations between TrackItem and Dispose()
Idempotent dispose pattern with _disposed flag prevents multiple cleanup attempts
ShouldPreserveTempFiles implemented as a property for API consistency

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
- No
Does the change make any security assumptions or guarantees?
- Yes
- No
Does the change require an update in our Aspire docs?
- Yes
- No

Original prompt

As a follow up to the IFileSystemService abstraction. Follow up with:

This is good overall I think. The one thing we should think about adding is a list of allocated temporary files/directories. When a file is disposed it should remove itself from that list - but the idea is that when the apphost is shutting down we can quickly spin over the list and delete any files.

Probably also want an environment varaible that the file system service uses to preserve files for debugging scenarios.

The file system service also needs verbose logging when allocating files/directories as well.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

… 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 <[email protected]>

src/Aspire.Hosting/Utils/FileSystemService.cs

…simplify tracking with ConcurrentDictionary - Replace Environment.GetEnvironmentVariable with IConfiguration["ASPIRE_PRESERVE_TEMP_FILES"] - Pass IConfiguration to FileSystemService constructor from DistributedApplicationBuilder - Use ConcurrentDictionary<string, IDisposable> 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 <[email protected]>

github-actions · 2025-12-06T08:13:03Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13361

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13361"

github-actions · 2025-12-06T08:13:03Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13361

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13361"

Copilot

Pull request overview

This PR enhances the FileSystemService with automatic tracking and cleanup of temporary files and directories when the AppHost shuts down, along with debugging support via configuration settings.

Key changes:

Implements automatic tracking using ConcurrentDictionary<string, IDisposable> to track all allocated temp files/directories
Makes FileSystemService implement IDisposable for cleanup on AppHost shutdown
Adds ASPIRE_PRESERVE_TEMP_FILES configuration option to skip cleanup for debugging scenarios
Integrates logging with debug-level logs for allocation/cleanup and info-level for preservation

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Aspire.Hosting/Utils/FileSystemService.cs	Core implementation: adds IDisposable, tracking dictionary, preserve configuration, logging integration, and cleanup logic in Dispose() method
src/Aspire.Hosting/DistributedApplicationBuilder.cs	Updates DI registration to pass IConfiguration to FileSystemService constructor and sets logger via factory lambda after service provider initialization
tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs	Adds comprehensive test coverage for cleanup functionality, preserve configuration, mixed scenarios, and idempotent dispose
tests/Aspire.Hosting.Tests/UserSecretsParameterDefaultTests.cs	Updates test helper to pass IConfiguration to FileSystemService constructor
tests/Aspire.Hosting.Tests/SecretsStoreTests.cs	Updates test helper to pass IConfiguration to FileSystemService constructor
tests/Aspire.Hosting.Tests/Dcp/DcpHostNotificationTests.cs	Updates test initialization to pass IConfiguration to FileSystemService constructor
tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs	Updates test initialization to pass IConfiguration to FileSystemService constructor
tests/Aspire.Hosting.Tests/Dashboard/DashboardLifecycleHookTests.cs	Updates test initialization to pass IConfiguration to FileSystemService constructor
tests/Aspire.Hosting.Tests/AspireStoreTests.cs	Updates test initialization to pass IConfiguration to FileSystemService constructor

src/Aspire.Hosting/Utils/FileSystemService.cs

Copilot · 2025-12-06T08:23:05Z

tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs

+
+        // Clean up manually
+        Directory.Delete(dirPath, recursive: true);
+        File.Delete(filePath);


The manual cleanup order in this test could cause a failure. The test attempts to delete the file first (line 256) and then the directory (line 255), but if the file still exists in the directory when trying to delete the directory, this could fail depending on the filesystem state.

Suggested fix:

// Clean up manually - delete directory recursively (includes the file) Directory.Delete(dirPath, recursive: true);

Remove the separate File.Delete(filePath) call since deleting the directory recursively will handle the file as well.

Suggested change

File.Delete(filePath);

The test cleanup code is correct as-is. The tempDir (line 239) and tempFile (line 240) are two independent items - the file is NOT inside the directory. tempFile is created with CreateTempFile() (no filename parameter), which creates a standalone temp file, not a file within a subdirectory. Therefore, both need to be deleted separately.

Copilot · 2025-12-06T08:23:06Z

tests/Aspire.Hosting.Tests/FileSystemServiceTests.cs

+        // Clean up manually
+        File.Delete(filePath);


This test's manual cleanup could fail because it tries to delete the file separately instead of relying on the directory deletion. Since CreateTempFile() with a filename creates both a temp directory and a file within it, and the temp file's parent directory should be deleted when the file is disposed (when deleteParentDirectory: true), the cleanup should delete the parent directory recursively.

However, looking at the test scenario, when preserve is enabled, both the file and its parent directory are preserved. The manual cleanup should handle this correctly by getting the parent directory path and deleting it recursively.

Suggested fix:

// Clean up manually - delete the parent directory which contains the file var parentDir = Path.GetDirectoryName(filePath); if (parentDir is not null && Directory.Exists(parentDir)) { Directory.Delete(parentDir, recursive: true); }

Suggested change

// Clean up manually

File.Delete(filePath);

// Clean up manually - delete the parent directory which contains the file

var parentDir = Path.GetDirectoryName(filePath);

if (parentDir is not null && Directory.Exists(parentDir))

{

Directory.Delete(parentDir, recursive: true);

}

The test cleanup code is correct as-is. The tempFile is created with CreateTempFile() without a filename parameter, which creates a standalone temp file directly in the system temp directory (not in a subdirectory). Therefore, File.Delete(filePath) is the appropriate cleanup method. There is no parent directory to delete in this case.

src/Aspire.Hosting/Utils/FileSystemService.cs

…hods - 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 <[email protected]>

mitchdenny · 2025-12-06T09:13:17Z

src/Aspire.Hosting/Utils/FileSystemService.cs

+
+        _logger?.LogDebug("Cleaning up {Count} remaining temporary files/directories", _allocatedItems.Count);
+
+        foreach (var kvp in _allocatedItems)


What happens here if someone adds a tracked file whilst it is disposing.

This is a snapshot, so it will miss cleanup.

@copilot lets mark that we are disposing so that any new allocations throw, so that new files can't be added while disposing. We need to make this thread safe.

Made TrackItem thread-safe by checking the _disposed flag before adding items. If the service is disposed, it now throws ObjectDisposedException preventing new temp files/directories from being allocated during or after disposal. Added tests to verify this behavior. (87cabce)

@copilot There's a super tight race where one thread is about to call _allocatedItems.TryAdd(path, item); (before Dispose is called) and then another thread calls Dispose is called just before we are about to enumerate the _allocatedItems. It maybe unrealstic but it is possible (thread scheduling and all that).

Fixed the race condition by adding a _disposeLock object. Both TrackItem and the beginning of Dispose() now use lock (_disposeLock) to atomically check and set the _disposed flag. This ensures that once Dispose() sets _disposed = true, no thread can add items to the dictionary. (a0db49c)

@davidfowl

…isposed - 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 <[email protected]>

davidfowl · 2025-12-06T09:34:58Z

src/Aspire.Hosting/Utils/FileSystemService.cs

+    /// <summary>
+    /// Gets whether temporary files should be preserved for debugging.
+    /// </summary>
+    internal bool ShouldPreserveTempFiles() => _preserveTempFiles;


nit: Make this a property

Changed ShouldPreserveTempFiles() from a method to a property by removing the parentheses. Updated all callers to use property syntax. (a0db49c)

davidfowl · 2025-12-06T09:40:52Z

src/Aspire.Hosting/Utils/FileSystemService.cs

+    /// <summary>
+    /// Gets the logger for this service, if set.
+    /// </summary>
+    internal ILogger<FileSystemService>? Logger => _logger;


Type this as ILogger instead of ILogger<FileSystemService>?

Changed Logger property type from ILogger<FileSystemService>? to ILogger? and updated the _logger field type accordingly. (a0db49c)

@davidfowl

- 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<FileSystemService>? to ILogger? for flexibility - Change _logger field type from ILogger<FileSystemService>? 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 <[email protected]>

Initial plan

cd764e5

Copilot AI assigned Copilot and davidfowl Dec 6, 2025

Copilot started work on behalf of davidfowl December 6, 2025 07:19 View session

Copilot AI changed the title ~~[WIP] Add temporary files management to IFileSystemService~~ Add automatic temp file tracking and cleanup to FileSystemService Dec 6, 2025

Copilot AI requested a review from davidfowl December 6, 2025 07:47

Copilot finished work on behalf of davidfowl December 6, 2025 07:47

davidfowl reviewed Dec 6, 2025

View reviewed changes

src/Aspire.Hosting/Utils/FileSystemService.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of davidfowl December 6, 2025 07:52 View session

Copilot AI requested a review from davidfowl December 6, 2025 08:09

Copilot finished work on behalf of davidfowl December 6, 2025 08:09

davidfowl marked this pull request as ready for review December 6, 2025 08:12

davidfowl requested a review from mitchdenny as a code owner December 6, 2025 08:12

Copilot AI review requested due to automatic review settings December 6, 2025 08:12

Copilot started reviewing on behalf of davidfowl December 6, 2025 08:13 View session

Copilot AI reviewed Dec 6, 2025

View reviewed changes

Copilot started work on behalf of davidfowl December 6, 2025 08:29 View session

Copilot finished work on behalf of davidfowl December 6, 2025 08:38

mitchdenny reviewed Dec 6, 2025

View reviewed changes

Copilot started work on behalf of davidfowl December 6, 2025 09:26 View session

Copilot finished work on behalf of davidfowl December 6, 2025 09:33

davidfowl reviewed Dec 6, 2025

View reviewed changes

davidfowl mentioned this pull request Dec 6, 2025

Various fixes to aspire update #13364

Open

2 tasks

Copilot started work on behalf of davidfowl December 6, 2025 10:09 View session

Copilot AI requested a review from davidfowl December 6, 2025 10:17

Copilot finished work on behalf of davidfowl December 6, 2025 10:17

-        // Clean up manually
-        File.Delete(filePath);
+        // Clean up manually - delete the parent directory which contains the file
+        var parentDir = Path.GetDirectoryName(filePath);
+        if (parentDir is not null && Directory.Exists(parentDir))
+        {
+            Directory.Delete(parentDir, recursive: true);
+        }


		_logger?.LogDebug("Cleaning up {Count} remaining temporary files/directories", _allocatedItems.Count);

		foreach (var kvp in _allocatedItems)

Add automatic temp file tracking and cleanup to FileSystemService #13361

Are you sure you want to change the base?

Add automatic temp file tracking and cleanup to FileSystemService #13361

Conversation

Copilot AI commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Implementation Notes

Checklist

Uh oh!

Uh oh!

github-actions bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mitchdenny Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

davidfowl Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

davidfowl Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

davidfowl Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

davidfowl Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 6, 2025 •

edited

Loading

github-actions bot commented Dec 6, 2025 •

edited

Loading

davidfowl Dec 6, 2025 •

edited

Loading