Skip to content

Commit

Permalink
FileSystem.Unix: Directory.Delete: remove per item syscall. (#59520)
Browse files Browse the repository at this point in the history
* FileSystem.Unix: Directory.Delete: remove per item syscall.

By recursing using FileSystemEnumerable we know the file type and
can omit the stat calls made by DirectoryInfo.Exists.

For the top level path, we can omit the call also and handle
non-directories when rmdir errno is ENOTDIR.

For the recursive case we can avoid recursion when the top level path rmdir
succeeds immediately.

FileSystemEntry is updated so IsSymbolicLink remembers the file is symbolic link
and does not make a syscall for it.
  • Loading branch information
tmds authored Nov 23, 2021
1 parent 56b5df4 commit a0fdc25
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 47 deletions.
29 changes: 29 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,5 +290,34 @@ public void RecursiveDelete_ShouldThrowIOExceptionIfContainedFileInUse()
}
Assert.True(testDir.Exists);
}

[ConditionalFact(nameof(CanCreateSymbolicLinks))]
public void RecursiveDeletingDoesntFollowLinks()
{
var target = GetTestFilePath();
Directory.CreateDirectory(target);

var fileInTarget = Path.Combine(target, GetTestFileName());
File.WriteAllText(fileInTarget, "");

var linkParent = GetTestFilePath();
Directory.CreateDirectory(linkParent);

var linkPath = Path.Combine(linkParent, GetTestFileName());
Assert.NotNull(Directory.CreateSymbolicLink(linkPath, target));

// Both the symlink and the target exist
Assert.True(Directory.Exists(target), "target should exist");
Assert.True(Directory.Exists(linkPath), "linkPath should exist");
Assert.True(File.Exists(fileInTarget), "fileInTarget should exist");

// Delete the parent folder of the symlink.
Delete(linkParent, true);

// Target should still exist
Assert.True(Directory.Exists(target), "target should still exist");
Assert.False(Directory.Exists(linkPath), "linkPath should no longer exist");
Assert.True(File.Exists(fileInTarget), "fileInTarget should exist");
}
}
}
115 changes: 68 additions & 47 deletions src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.IO.Enumeration;
using System.Text;
using Microsoft.Win32.SafeHandles;

Expand Down Expand Up @@ -429,69 +430,66 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath)

public static void RemoveDirectory(string fullPath, bool recursive)
{
var di = new DirectoryInfo(fullPath);
if (!di.Exists)
// Delete the directory.
// If we're recursing, don't throw when it is not empty, and perform a recursive remove.
if (!RemoveEmptyDirectory(fullPath, topLevel: true, throwWhenNotEmpty: !recursive))
{
throw Interop.GetExceptionForIoErrno(Interop.Error.ENOENT.Info(), fullPath, isDirectory: true);
Debug.Assert(recursive);

RemoveDirectoryRecursive(fullPath);
}
RemoveDirectoryInternal(di, recursive, throwOnTopLevelDirectoryNotFound: true);
}

private static void RemoveDirectoryInternal(DirectoryInfo directory, bool recursive, bool throwOnTopLevelDirectoryNotFound)
private static void RemoveDirectoryRecursive(string fullPath)
{
Exception? firstException = null;

if ((directory.Attributes & FileAttributes.ReparsePoint) != 0)
try
{
DeleteFile(directory.FullName);
return;
}
var fse = new FileSystemEnumerable<(string, bool)>(fullPath,
static (ref FileSystemEntry entry) =>
{
// Don't report symlinks to directories as directories.
bool isRealDirectory = !entry.IsSymbolicLink && entry.IsDirectory;
return (entry.ToFullPath(), isRealDirectory);
},
EnumerationOptions.Compatible);

if (recursive)
{
try
foreach ((string childPath, bool isDirectory) in fse)
{
foreach (string item in Directory.EnumerateFileSystemEntries(directory.FullName))
try
{
if (!ShouldIgnoreDirectory(Path.GetFileName(item)))
if (isDirectory)
{
try
{
var childDirectory = new DirectoryInfo(item);
if (childDirectory.Exists)
{
RemoveDirectoryInternal(childDirectory, recursive, throwOnTopLevelDirectoryNotFound: false);
}
else
{
DeleteFile(item);
}
}
catch (Exception exc)
{
if (firstException != null)
{
firstException = exc;
}
}
RemoveDirectoryRecursive(childPath);
}
else
{
DeleteFile(childPath);
}
}
}
catch (Exception exc)
{
if (firstException != null)
catch (Exception ex)
{
firstException = exc;
firstException ??= ex;
}
}
}
catch (Exception exc)
{
firstException ??= exc;
}

if (firstException != null)
{
throw firstException;
}
if (firstException != null)
{
throw firstException;
}

if (Interop.Sys.RmDir(directory.FullName) < 0)
RemoveEmptyDirectory(fullPath);
}

private static bool RemoveEmptyDirectory(string fullPath, bool topLevel = false, bool throwWhenNotEmpty = true)
{
if (Interop.Sys.RmDir(fullPath) < 0)
{
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
switch (errorInfo.Error)
Expand All @@ -500,17 +498,40 @@ private static void RemoveDirectoryInternal(DirectoryInfo directory, bool recurs
case Interop.Error.EPERM:
case Interop.Error.EROFS:
case Interop.Error.EISDIR:
throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, directory.FullName)); // match Win32 exception
throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, fullPath)); // match Win32 exception
case Interop.Error.ENOENT:
if (!throwOnTopLevelDirectoryNotFound)
// When we're recursing, don't throw for items that go missing.
if (!topLevel)
{
return;
return true;
}
goto default;
case Interop.Error.ENOTDIR:
// When the top-level path is a symlink to a directory, delete the link.
// In other cases, throw because we expect path to be a real directory.
if (topLevel)
{
if (!DirectoryExists(fullPath))
{
throw Interop.GetExceptionForIoErrno(Interop.Error.ENOENT.Info(), fullPath, isDirectory: true);
}

DeleteFile(fullPath);
return true;
}
goto default;
case Interop.Error.ENOTEMPTY:
if (!throwWhenNotEmpty)
{
return false;
}
goto default;
default:
throw Interop.GetExceptionForIoErrno(errorInfo, directory.FullName, isDirectory: true);
throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirectory: true);
}
}

return true;
}

/// <summary>Determines whether the specified directory name should be ignored.</summary>
Expand Down

0 comments on commit a0fdc25

Please sign in to comment.