Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

@copilot can you double-check that the changes will fix the failed scenarios. Here's the console logs for both issues:

https://helixr18s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-heads-main-c069cfeb95dd4cd0bb/System.IO.FileSystem.Tests/1/console.48d39a3e.log?helixlogtype=result

https://helixr18s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-125526-merge-ec064c4db40e4688b9/System.IO.FileSystem.Tests/1/console.d1be5f1e.log?skoid=8eda00af-b5ec-4be9-b69b-0919a2338892&sktid=72f988bf-86f1-41af-91ab-2d7cd011db47&skt=2026-03-16T18%3A03%3A40Z&ske=2026-03-16T19%3A03%3A40Z&sks=b&skv=2026-02-06&sv=2026-02-06&st=2026-03-16T18%3A03%3A40Z&se=2026-03-16T19%3A03%3A40Z&sr=c&sp=r&sig=rCVk68C1DmnrU8bz1t8WLIyZE2pA3lWyTU35hisxPiI%3D

@copilot can you double-check that the changes will fix the failed scenarios. Here's the console logs for both issues:

https://helixr18s23ayyeko0k025g8.blob.core.windows.net/dotn...

The Helix log SAS tokens in your comment have expired (1-hour window) so I can't fetch them, but here's my code-level analysis:

Log 1 — ReparsePoints_MountVolume.runTest (main branch)

The 4 finally blocks had unconditional MountHelper.Unmount(mountedDirName). If MountHelper.Mount fails after CreateDirectory succeeds, the directory exists but is not a mount point → DeleteVolumeMountPoint fails with Win32 error 4390, caught by the scenario catch → s_pass = false. My Directory.Exists guard prevents calling Unmount when the directory doesn't exist (error 3 case). This matches the same pattern PR #125348 applied to Delete_MountVolume scenarios 1–3.2.

Log 2 — Delete_MountVolume.RunTest (PR #125526 build, still failing after #125348)

Scenarios 3.3–3.5 used if (!Eval(!Directory.Exists(mountedDirName), "Err_XXX!...")) { Unmount; DeleteDir; } in finally. Directory.Delete internally calls DeleteVolumeMountPoint first, then RemoveDirectory. If RemoveDirectory fails (e.g., antivirus holding a handle — exactly the case documented in EnsureDirectoryNotExist's comment), the mount binding is already removed but the directory still exists. The old Eval then fires → s_pass = false → cleanup tries Unmount → fails with Win32 error 4390 (no longer a reparse point) → exception caught → s_pass = false again.

My fix removes the Eval from the finally blocks entirely (no assertion in cleanup), using a plain if (Directory.Exists) guard instead. For scenario 3.3 specifically, the assertion was moved into the try block after Task.Delay(300). This prevents the spurious s_pass = false from cleanup code and matches the established pattern from #125348.

/azp run runtime-nativeaot-outerloop

@copilot the allowlist was updated, can you try #125625 (comment) again.

@copilot the allowlist was updated, can you try #125625 (comment) again.

After fetching the first log, I found the root cause: the Directory.Exists guard from PR #125348 is insufficient. The log (from 2026-03-15, 4 days after PR #125348 merged on 2026-03-11) shows Scenario 1 still failing with Win32 error 4390:

Mounting volume E:\ at C:\LaksMountfpywadjz\   ← no uniqueName printed = GetVolumeNameForVolumeMountPoint failed
Unmounting the volume at C:\LaksMountfpywadjz\  ← Unmount IS called
Err_768lme! Win32 error: 4390

When GetVolumeNameForVolumeMountPoint fails, MountHelper.Mount throws after Directory.CreateDirectory has already created the directory. So Directory.Exists returns true, the guard passes, Unmount is called on a plain directory (not a mount point), and throws 4390.

Fix (commit c5e5852): wrap MountHelper.Unmount in try { } catch { } in all finally cleanup blocks in both files. This handles all cases — Mount failing, partial deletes, race conditions — without letting cleanup exceptions escape to the scenario's outer catch and set s_pass = false.

Changed 11 finally blocks total:

• Delete_MountVolume.cs: scenarios 1, 2, 3.1, 3.2, 3.3, 3.4, 3.5
• ReparsePoints_MountVolume.cs: scenarios 1, 2, 3.1, 3.2

/azp run runtime-nativeaot-outerloop

This is the same root cause fixed for the sibling test Delete_MountVolume in #125348.

Perhaps we should consider changing MountHelper.Unmount to ignore this particular error and accept ...

Done in commit c3186198. MountHelper.Unmount now:

• Accepts a bool deleteDirectory = false parameter — when true, deletes the directory after unmounting (best-effort, logs on failure)
• Silently ignores Win32 errors 4390 (not a reparse point) and 3 (path not found), which are the two expected cleanup-time failures

All call sites updated to use MountHelper.Unmount(path, deleteDirectory: true), eliminating the repetitive if (Directory.Exists) { try { Unmount } catch { } DeleteDir } pattern from ReparsePoints_MountVolume.cs, Delete_MountVolume.cs (scenarios 1–3.2), and Delete.Windows.cs.

The system cannot find the file specified.

Exception Message
System.Exception : Win32 error: 2
CallStack
   at MountHelper.Unmount(String mountPoint, Boolean deleteDirectory) in /_/src/libraries/Common/tests/System/IO/ReparsePointUtilities.cs:line 262
   at System.IO.Tests.Directory_Delete_str_bool.Delete_VolumeMountPoint() in /_/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.Windows.cs:line 85
   at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) in /_/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs:line 230

ReparsePointUtilities.cs is some Windows pinvokes and some cross platform code in a single file. This is why we have raw numbers here and can't use constants out of Interop.Errors.cs. That should be fixed at some point.

Meanwhile I have added error 2 which is morally equivalent to error 3 in this context.

🤖 Copilot Code Review — PR #125625

Holistic Assessment

Motivation: The PR addresses test flakiness in mount volume tests (Fixes #125624) where Unmount in finally blocks throws when the mount point was already cleaned up by the test itself. The problem is real — the ReparsePoints_MountVolume.cs tests called Unmount unconditionally in finally blocks, and this fails with Win32 errors 4390/3/2 when the test already deleted the mount point. This is a valid fix for test infrastructure reliability.

Approach: Centralizing the "unmount + delete directory" pattern into MountHelper.Unmount with error resilience is a reasonable approach. The specific Win32 errors suppressed (ERROR_NOT_A_REPARSE_POINT=4390, ERROR_PATH_NOT_FOUND=3, ERROR_FILE_NOT_FOUND=2) are well-chosen for cleanup scenarios, and other unexpected errors still propagate. The approach is consistent with test cleanup conventions in dotnet/runtime.

Summary: ⚠️ Needs Human Review. The overall direction is good and the code is correct for its stated purpose. However, the PR removes retry logic that existed in the old DeleteDir calls to handle transient filesystem timing issues, which could reintroduce a different class of flakiness. A human reviewer should evaluate whether the retry logic is still needed or whether the error suppression in Unmount makes it unnecessary.

Detailed Findings

⚠️ Lost Retry Logic in Directory Deletion

The old callers in ReparsePoints_MountVolume.cs and Delete_MountVolume.cs used DeleteDir(mountedDirName, true), which contains retry logic with backoff to handle transient IOException during directory removal:

• ReparsePoints_MountVolume.DeleteDir: Retries up to 10 times with 200ms delay on IOException
• Delete_MountVolume.DeleteDir: Retries up to 5 times with 300ms delay on any Exception

The new Unmount(... deleteDirectory: true) calls Directory.Delete(dirPath, recursive: true) directly with no retries. Given that these tests interact with Windows volume mount points where filesystem timing is known to be sensitive (evidenced by the existing Task.Delay(100).Wait() in Mount and WaitForDirectoryGone in other test code), a single Directory.Delete call may fail transiently if the kernel hasn't fully released the directory after the mount point is removed.

This could trade one class of flakiness (Unmount throwing) for another (Directory.Delete throwing on transient lock). A human reviewer should assess whether adding similar retry logic to the deleteDirectory path in Unmount would be warranted, or whether the timing concern is adequately handled by the new error suppression.

Files: ReparsePointUtilities.cs:264-267 vs old ReparsePoints_MountVolume.cs:368-390 and Delete_MountVolume.cs:382-403

✅ Error Suppression — Appropriate for Cleanup

The three suppressed Win32 errors are well-chosen:

• ERROR_FILE_NOT_FOUND (2) / ERROR_PATH_NOT_FOUND (3): Path is already gone — expected in cleanup after a test that deleted the mount point.
• ERROR_NOT_A_REPARSE_POINT (4390): Path exists but is not a mount point — expected after the test already unmounted or Directory.Delete already removed the reparse point.

All other errors (ACCESS_DENIED, DEVICE_BUSY, INVALID_PARAMETER, etc.) still throw. The console logging of suppressed errors aids debugging. This is verified correct for cleanup semantics.

✅ Behavioral Equivalence of Guard Removal

The old callers in Delete.Windows.cs and Delete_MountVolume.cs (scenarios 1–4) had if (Directory.Exists(...)) guards before calling Unmount. The new code calls Unmount unconditionally, relying on the internal error suppression. This is semantically equivalent: if the directory doesn't exist, DeleteVolumeMountPoint fails with ERROR_PATH_NOT_FOUND or ERROR_FILE_NOT_FOUND, which are now suppressed. The deleteDirectory path also checks Directory.Exists before deletion.

✅ Unchanged Scenarios 3.3–3.5

The finally blocks in scenarios 3.3, 3.4, and 3.5 of Delete_MountVolume.cs were intentionally left unchanged. These have different semantics — they are assertion failure cleanup (only execute when !Eval(...) detects the directory still exists when it shouldn't). Their conditional pattern is correct as-is and benefits from Unmount's new error resilience without needing the deleteDirectory parameter.

💡 Named Constants for Error Codes

The Win32 error codes 4390, 3, and 2 are used as bare integers with an inline comment. Consider using local const declarations for clarity:

const int ERROR_FILE_NOT_FOUND = 2;
const int ERROR_PATH_NOT_FOUND = 3;
const int ERROR_NOT_A_REPARSE_POINT = 4390;

This is a minor readability improvement — the existing comment is adequate but named constants are more self-documenting, especially if the list grows.

✅ No Public API Surface Changes

This PR modifies only test infrastructure and test files. No public API changes, no ref/ assembly changes. API approval verification is not required.

Generated by Code Review for issue #125625 · ◷

@copilot is there an issue that ought to have been closed by this fix.