Handle FileOptions.DeleteOnClose on Unix in SafeFileHandle.Dispose (#55153)

* Handle FileOptions.DeleteOnClose on Unix in SafeFileHandle.Dispose

* Move file path into SafeFileHandle on Windows as well

Doing so enables cleaning up a bunch of passing around of the file path, while also enabling the path to be fed to more error paths.

* Update Net5Compat impl as well

Author: stephentoub

src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs

@@ -56,15 +56,18 @@ public void SafeFileHandle_IsAsync_ReturnsCorrectInformation(FileOptions options
             }
         }
 
-        // Unix doesn't directly support DeleteOnClose
-        // For FileStream created out of path, we mimic it by closing the handle first
-        // and then unlinking the path
-        // Since SafeFileHandle does not always have the path and we can't find path for given file descriptor on Unix
-        // this test runs only on Windows
-        [PlatformSpecific(TestPlatforms.Windows)]
         [Theory]
         [InlineData(FileOptions.DeleteOnClose)]
         [InlineData(FileOptions.DeleteOnClose | FileOptions.Asynchronous)]
-        public override void DeleteOnClose_FileDeletedAfterClose(FileOptions options) => base.DeleteOnClose_FileDeletedAfterClose(options);
+        public void DeleteOnClose_FileDeletedAfterSafeHandleDispose(FileOptions options)
+        {
+            string path = GetTestFilePath();
+            Assert.False(File.Exists(path));
+            using (SafeFileHandle sfh = File.OpenHandle(path, FileMode.Create, FileAccess.ReadWrite, FileShare.None, options))
+            {
+                Assert.True(File.Exists(path));
+            }
+            Assert.False(File.Exists(path));
+        }
     }
 }

src/libraries/System.IO.FileSystem/tests/FileStream/Name.cs

@@ -3,6 +3,7 @@
 
 using System.Globalization;
 using System.Tests;
+using Microsoft.Win32.SafeHandles;
 using Xunit;
 
 namespace System.IO.Tests
@@ -35,14 +36,29 @@ public void NameNormalizesPath()
         }
 
         [Fact]
-        public void NameReturnsUnknownForHandle()
+        public void ConstructFileStreamFromHandle_NameMatchesOriginal()
         {
-            using (new ThreadCultureChange(CultureInfo.InvariantCulture))
-            using (FileStream fs = new FileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite))
-            using (FileStream fsh = new FileStream(fs.SafeFileHandle, FileAccess.ReadWrite))
-            {
-                Assert.Equal("[Unknown]", fsh.Name);
-            }
+            string path = GetTestFilePath();
+            using var _ = new ThreadCultureChange(CultureInfo.InvariantCulture);
+
+            using FileStream fs = new FileStream(path, FileMode.Create, FileAccess.ReadWrite);
+            Assert.Equal(path, fs.Name);
+
+            using FileStream fsh = new FileStream(fs.SafeFileHandle, FileAccess.ReadWrite);
+            Assert.Equal(path, fsh.Name);
+        }
+
+        [Fact]
+        public void ConstructFileStreamFromHandleClone_NameReturnsUnknown()
+        {
+            string path = GetTestFilePath();
+            using var _ = new ThreadCultureChange(CultureInfo.InvariantCulture);
+
+            using FileStream fs = new FileStream(path, FileMode.Create, FileAccess.ReadWrite);
+            Assert.Equal(path, fs.Name);
+
+            using FileStream fsh = new FileStream(new SafeFileHandle(fs.SafeFileHandle.DangerousGetHandle(), ownsHandle: false), FileAccess.ReadWrite);
+            Assert.Equal("[Unknown]", fsh.Name);
         }
     }
 }

src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs_buffer_fo.cs

@@ -1,6 +1,7 @@
 // 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 Xunit;
 
 namespace System.IO.Tests
@@ -79,7 +80,7 @@ public void ValidFileOptions_Encrypted(FileOptions option)
         [Theory]
         [InlineData(FileOptions.DeleteOnClose)]
         [InlineData(FileOptions.DeleteOnClose | FileOptions.Asynchronous)]
-        public virtual void DeleteOnClose_FileDeletedAfterClose(FileOptions options)
+        public void DeleteOnClose_FileDeletedAfterClose(FileOptions options)
         {
             string path = GetTestFilePath();
             Assert.False(File.Exists(path));
@@ -89,5 +90,37 @@ public virtual void DeleteOnClose_FileDeletedAfterClose(FileOptions options)
             }
             Assert.False(File.Exists(path));
         }
+
+        [Theory]
+        [InlineData(FileOptions.DeleteOnClose)]
+        [InlineData(FileOptions.DeleteOnClose | FileOptions.Asynchronous)]
+        public void DeleteOnClose_FileDeletedAfterSafeHandleRelease(FileOptions options)
+        {
+            string path = GetTestFilePath();
+            Assert.False(File.Exists(path));
+
+            using (FileStream fs = CreateFileStream(path, FileMode.Create, FileAccess.ReadWrite, FileShare.None, 0x1000, options))
+            {
+                Assert.True(File.Exists(path));
+
+                bool added = false;
+                try
+                {
+                    fs.SafeFileHandle.DangerousAddRef(ref added);
+
+                    fs.Dispose();
+                    Assert.True(File.Exists(path));
+                }
+                finally
+                {
+                    if (added)
+                    {
+                        fs.SafeFileHandle.DangerousRelease();
+                    }
+
+                    Assert.False(File.Exists(path));
+                }
+            }
+        }
     }
 }

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.OverlappedValueTaskSource.Windows.cs

@@ -44,7 +44,7 @@ internal sealed unsafe class OverlappedValueTaskSource : IValueTaskSource<int>,
             internal static readonly IOCompletionCallback s_ioCallback = IOCallback;
 
             internal readonly PreAllocatedOverlapped _preallocatedOverlapped;
-            private readonly SafeFileHandle _fileHandle;
+            internal readonly SafeFileHandle _fileHandle;
             internal MemoryHandle _memoryHandle;
             internal ManualResetValueTaskSourceCore<int> _source; // mutable struct; do not make this readonly
             private NativeOverlapped* _overlapped;

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

@@ -12,6 +12,7 @@ public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
         // not using bool? as it's not thread safe
         private volatile NullableBool _canSeek = NullableBool.Undefined;
+        private bool _deleteOnClose;
 
         public SafeFileHandle() : this(ownsHandle: true)
         {
@@ -36,6 +37,7 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int
         {
             Debug.Assert(path != null);
             SafeFileHandle handle = Interop.Sys.Open(path, flags, mode);
+            handle._path = path;
 
             if (handle.IsInvalid)
             {
@@ -51,7 +53,7 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int
 
                 bool isDirectory = (error.Error == Interop.Error.ENOENT) &&
                     ((flags & Interop.Sys.OpenFlags.O_CREAT) != 0
-                    || !DirectoryExists(Path.GetDirectoryName(Path.TrimEndingDirectorySeparator(path!))!));
+                    || !DirectoryExists(System.IO.Path.GetDirectoryName(System.IO.Path.TrimEndingDirectorySeparator(path!))!));
 
                 Interop.CheckIo(
                     error.Error,
@@ -117,6 +119,17 @@ protected override bool ReleaseHandle()
             // problem trying to unlock it.)
             Interop.Sys.FLock(handle, Interop.Sys.LockOperations.LOCK_UN); // ignore any errors
 
+            // If DeleteOnClose was requested when constructed, delete the file now.
+            // (Unix doesn't directly support DeleteOnClose, so we mimic it here.)
+            if (_deleteOnClose)
+            {
+                // Since we still have the file open, this will end up deleting
+                // it (assuming we're the only link to it) once it's closed, but the
+                // name will be removed immediately.
+                Debug.Assert(_path is not null);
+                Interop.Sys.Unlink(_path); // ignore errors; it's valid that the path may no longer exist
+            }
+
             // Close the descriptor. Although close is documented to potentially fail with EINTR, we never want
             // to retry, as the descriptor could actually have been closed, been subsequently reassigned, and
             // be in use elsewhere in the process.  Instead, we simply check whether the call was successful.
@@ -234,6 +247,7 @@ private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mo
         private void Init(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize)
         {
             IsAsync = (options & FileOptions.Asynchronous) != 0;
+            _deleteOnClose = (options & FileOptions.DeleteOnClose) != 0;
 
             // Lock the file if requested via FileShare.  This is only advisory locking. FileShare.None implies an exclusive
             // lock on the file and all other modes use a shared lock.  While this is not as granular as Windows, not mandatory,

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs

@@ -103,6 +103,7 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode,
                 throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath);
             }
 
+            fileHandle._path = fullPath;
             fileHandle._fileOptions = options;
             return fileHandle;
         }

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.cs

@@ -7,9 +7,13 @@ namespace Microsoft.Win32.SafeHandles
 {
     public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
+        private string? _path;
+
         public SafeFileHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
         {
             SetHandle(preexistingHandle);
         }
+
+        internal string? Path => _path;
     }
 }

src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs

@@ -17,10 +17,10 @@ public static partial class RandomAccess
         // that get stackalloced in the Linux kernel.
         private const int IovStackThreshold = 8;
 
-        internal static long GetFileLength(SafeFileHandle handle, string? path)
+        internal static long GetFileLength(SafeFileHandle handle)
         {
             int result = Interop.Sys.FStat(handle, out Interop.Sys.FileStatus status);
-            FileStreamHelpers.CheckFileCall(result, path);
+            FileStreamHelpers.CheckFileCall(result, handle.Path);
             return status.Size;
         }
 
@@ -29,7 +29,7 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer
             fixed (byte* bufPtr = &MemoryMarshal.GetReference(buffer))
             {
                 int result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset);
-                FileStreamHelpers.CheckFileCall(result, path: null);
+                FileStreamHelpers.CheckFileCall(result, handle.Path);
                 return result;
             }
         }
@@ -64,7 +64,7 @@ internal static unsafe long ReadScatterAtOffset(SafeFileHandle handle, IReadOnly
                 }
             }
 
-            return FileStreamHelpers.CheckFileCall(result, path: null);
+            return FileStreamHelpers.CheckFileCall(result, handle.Path);
         }
 
         private static ValueTask<int> ReadAtOffsetAsync(SafeFileHandle handle, Memory<byte> buffer, long fileOffset, CancellationToken cancellationToken)
@@ -79,7 +79,7 @@ internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<byt
             fixed (byte* bufPtr = &MemoryMarshal.GetReference(buffer))
             {
                 int result = Interop.Sys.PWrite(handle, bufPtr, buffer.Length, fileOffset);
-                FileStreamHelpers.CheckFileCall(result, path: null);
+                FileStreamHelpers.CheckFileCall(result, handle.Path);
                 return  result;
             }
         }
@@ -114,7 +114,7 @@ internal static unsafe long WriteGatherAtOffset(SafeFileHandle handle, IReadOnly
                 }
             }
 
-            return FileStreamHelpers.CheckFileCall(result, path: null);
+            return FileStreamHelpers.CheckFileCall(result, handle.Path);
         }
 
         private static ValueTask<int> WriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory<byte> buffer, long fileOffset, CancellationToken cancellationToken)

src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs

@@ -16,23 +16,23 @@ public static partial class RandomAccess
     {
         private static readonly IOCompletionCallback s_callback = AllocateCallback();
 
-        internal static unsafe long GetFileLength(SafeFileHandle handle, string? path)
+        internal static unsafe long GetFileLength(SafeFileHandle handle)
         {
             Interop.Kernel32.FILE_STANDARD_INFO info;
 
             if (!Interop.Kernel32.GetFileInformationByHandleEx(handle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO)))
             {
-                throw Win32Marshal.GetExceptionForLastWin32Error(path);
+                throw Win32Marshal.GetExceptionForLastWin32Error(handle.Path);
             }
 
             return info.EndOfFile;
         }
 
-        internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer, long fileOffset, string? path = null)
+        internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer, long fileOffset)
         {
             if (handle.IsAsync)
             {
-                return ReadSyncUsingAsyncHandle(handle, buffer, fileOffset, path);
+                return ReadSyncUsingAsyncHandle(handle, buffer, fileOffset);
             }
 
             NativeOverlapped overlapped = GetNativeOverlappedForSyncHandle(handle, fileOffset);
@@ -55,12 +55,12 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer
                         // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe.
                         return 0;
                     default:
-                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
+                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path);
                 }
             }
         }
 
-        private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<byte> buffer, long fileOffset, string? path)
+        private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<byte> buffer, long fileOffset)
         {
             handle.EnsureThreadPoolBindingInitialized();
 
@@ -105,7 +105,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<b
                             return 0;
 
                         default:
-                            throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
+                            throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path);
                     }
                 }
             }
@@ -120,11 +120,11 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<b
             }
         }
 
-        internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<byte> buffer, long fileOffset, string? path = null)
+        internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<byte> buffer, long fileOffset)
         {
             if (handle.IsAsync)
             {
-                return WriteSyncUsingAsyncHandle(handle, buffer, fileOffset, path);
+                return WriteSyncUsingAsyncHandle(handle, buffer, fileOffset);
             }
 
             NativeOverlapped overlapped = GetNativeOverlappedForSyncHandle(handle, fileOffset);
@@ -141,12 +141,12 @@ internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<byt
                     case Interop.Errors.ERROR_NO_DATA: // EOF on a pipe
                         return 0;
                     default:
-                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
+                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path);
                 }
             }
         }
 
-        private static unsafe int WriteSyncUsingAsyncHandle(SafeFileHandle handle, ReadOnlySpan<byte> buffer, long fileOffset, string? path)
+        private static unsafe int WriteSyncUsingAsyncHandle(SafeFileHandle handle, ReadOnlySpan<byte> buffer, long fileOffset)
         {
             handle.EnsureThreadPoolBindingInitialized();
 
@@ -193,7 +193,7 @@ private static unsafe int WriteSyncUsingAsyncHandle(SafeFileHandle handle, ReadO
                             throw new IOException(SR.IO_FileTooLong);
 
                         default:
-                            throw Win32Marshal.GetExceptionForWin32Error(errorCode, path);
+                            throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path);
                     }
                 }
             }
@@ -469,7 +469,7 @@ private static unsafe ValueTask<int> ReadFileScatterAsync(SafeFileHandle handle,
                         default:
                             // Error. Callback will not be called.
                             vts.Dispose();
-                            return ValueTask.FromException<int>(Win32Marshal.GetExceptionForWin32Error(errorCode));
+                            return ValueTask.FromException<int>(Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path));
                     }
                 }
             }

src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs

@@ -23,7 +23,7 @@ public static long GetLength(SafeFileHandle handle)
         {
             ValidateInput(handle, fileOffset: 0);
 
-            return GetFileLength(handle, path: null);
+            return GetFileLength(handle);
         }
 
         /// <summary>