From 8fb25a8b046621ef5fe0626e043b7976a71c0670 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 14 Jun 2023 00:58:17 +1000 Subject: [PATCH] Clone files on OSX-like platforms when possible, instead of copying the whole file (#79243) Co-authored-by: Stephen Toub Co-authored-by: Dan Moseley --- .../Common/src/Interop/OSX/Interop.libc.cs | 5 + .../System.IO.FileSystem/tests/File/Copy.cs | 13 +++ .../System.Private.CoreLib.Shared.projitems | 1 + .../System/IO/FileSystem.TryCloneFile.OSX.cs | 92 +++++++++++++++++++ .../src/System/IO/FileSystem.Unix.cs | 29 ++++-- 5 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.TryCloneFile.OSX.cs diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 44717c56d0ba8..adf70d390872d 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -44,5 +44,10 @@ internal static unsafe int fsetattrlist(SafeHandle handle, AttrList* attrList, v handle.DangerousRelease(); } } + + [LibraryImport(Libraries.libc, EntryPoint = "clonefile", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + internal static unsafe partial int clonefile(string src, string dst, int flags); + + internal const int CLONE_ACL = 0x0004; } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs index 8b1f1c2d4e78c..4ab26c3748877 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs @@ -352,6 +352,19 @@ public void WindowsAlternateDataStreamOverwrite(string defaultStream, string alt Assert.Throws(() => Copy(testFileAlternateStream, testFile2 + alternateStream, overwrite: true)); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))] + public void CopyOntoLockedFile() + { + string testFileSource = GetTestFilePath(); + string testFileDest = GetTestFilePath(); + File.Create(testFileSource).Dispose(); + File.Create(testFileDest).Dispose(); + using (var stream = new FileStream(testFileDest, FileMode.Open, FileAccess.Read, FileShare.None)) + { + Assert.Throws(() => Copy(testFileSource, testFileDest, overwrite: true)); + } + } + [Fact] public void DestinationFileIsTruncatedWhenItsLargerThanSourceFile() { diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index d80ad76e071a2..c7a6eed48f322 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2434,6 +2434,7 @@ Common\Interop\OSX\Interop.libc.cs + diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.TryCloneFile.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.TryCloneFile.OSX.cs new file mode 100644 index 0000000000000..8bfb60414a707 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.TryCloneFile.OSX.cs @@ -0,0 +1,92 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Win32.SafeHandles; +using System.Diagnostics; + +namespace System.IO +{ + internal static partial class FileSystem + { + static partial void TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite, ref bool cloned) + { + // This helper function calls out to clonefile, and returns the error. + static bool TryCloneFile(string sourceFullPath, string destFullPath, int flags, out Interop.Error error) + { + if (Interop.@libc.clonefile(sourceFullPath, destFullPath, flags) == 0) + { + // Success. + error = Interop.Error.SUCCESS; + return true; + } + + error = Interop.Sys.GetLastError(); + return false; + } + + // Try to clone the file immediately, this will only succeed if the + // destination doesn't exist, so we don't worry about locking for this one. + int flags = Interop.@libc.CLONE_ACL; + Interop.Error error; + if (TryCloneFile(sourceFullPath, destFullPath, flags, out error)) + { + cloned = true; + return; + } + + // Some filesystems don't support ACLs, so may fail due to trying to copy ACLs. + // This will disable them and allow trying again (a maximum of 1 time). + if (error == Interop.Error.EINVAL) + { + flags = 0; + if (TryCloneFile(sourceFullPath, destFullPath, flags, out error)) + { + cloned = true; + return; + } + } + + // Try to delete the destination file if we're overwriting. + if (error == Interop.Error.EEXIST && overwrite) + { + // Delete the destination. This should fail on directories. Get a lock to the dest file to ensure we don't copy onto it when + // it's locked by something else, and then delete it. It should also fail if destination == source since it's already locked. + try + { + using SafeFileHandle? dstHandle = SafeFileHandle.Open(destFullPath, FileMode.Open, FileAccess.ReadWrite, + FileShare.None, FileOptions.None, preallocationSize: 0, createOpenException: CreateOpenExceptionForCopyFile); + if (Interop.Sys.Unlink(destFullPath) < 0 && + Interop.Sys.GetLastError() != Interop.Error.ENOENT) + { + // Fall back to standard copy as an unexpected error has occurred. + return; + } + } + catch (FileNotFoundException) + { + // We don't want to throw if it's just the file not existing, since we're trying to delete it. + } + + // Try clonefile now we've deleted the destination file. + if (TryCloneFile(sourceFullPath, destFullPath, flags, out error)) + { + cloned = true; + return; + } + } + + if (error is Interop.Error.ENOTSUP // Check if it's not supported, + or Interop.Error.EXDEV // if files are on different filesystems, + or Interop.Error.EEXIST) // or if the destination file still exists. + { + // Fall back to normal copy. + return; + } + + // Throw the appropriate exception. + Debug.Assert(error != Interop.Error.EINVAL); // We shouldn't fail due to an invalid parameter. + Debug.Assert(error != Interop.Error.SUCCESS); // We shouldn't fail with success. + throw Interop.GetExceptionForIoErrno(error.Info(), destFullPath); + } + } +} diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index f28cd70f08f83..be6505d9bd93f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -28,27 +28,38 @@ internal static partial class FileSystem UnixFileMode.OtherWrite | UnixFileMode.OtherExecute; + static partial void TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite, ref bool cloned); + public static void CopyFile(string sourceFullPath, string destFullPath, bool overwrite) { long fileLength; UnixFileMode filePermissions; using SafeFileHandle src = SafeFileHandle.OpenReadOnly(sourceFullPath, FileOptions.None, out fileLength, out filePermissions); + + // Try to clone the file first. + bool cloned = false; + TryCloneFile(sourceFullPath, destFullPath, overwrite, ref cloned); + if (cloned) + { + return; + } + using SafeFileHandle dst = SafeFileHandle.Open(destFullPath, overwrite ? FileMode.Create : FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize: 0, filePermissions, - CreateOpenException); + CreateOpenExceptionForCopyFile); Interop.CheckIo(Interop.Sys.CopyFile(src, dst, fileLength)); + } - static Exception? CreateOpenException(Interop.ErrorInfo error, Interop.Sys.OpenFlags flags, string path) + private static Exception? CreateOpenExceptionForCopyFile(Interop.ErrorInfo error, Interop.Sys.OpenFlags flags, string path) + { + // If the destination path points to a directory, we throw to match Windows behaviour. + if (error.Error == Interop.Error.EEXIST && DirectoryExists(path)) { - // If the destination path points to a directory, we throw to match Windows behaviour. - if (error.Error == Interop.Error.EEXIST && DirectoryExists(path)) - { - return new IOException(SR.Format(SR.Arg_FileIsDirectory_Name, path)); - } - - return null; // Let SafeFileHandle create the exception for this error. + return new IOException(SR.Format(SR.Arg_FileIsDirectory_Name, path)); } + + return null; // Let SafeFileHandle create the exception for this error. } #pragma warning disable IDE0060