Use O_APPEND and FILE_APPEND_DATA to allow for atomic file appends

src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs

@@ -27,6 +27,9 @@ internal static partial class Fcntl
 
             [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlGetFD", SetLastError=true)]
             internal static extern int GetFD(IntPtr fd);
+
+            [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetIsAppend")]
+            internal static extern bool GetIsAppend(SafeHandle fd);
         }
     }
 }

src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenFlags.cs

@@ -21,6 +21,7 @@ internal enum OpenFlags
             O_EXCL    = 0x0040,
             O_TRUNC   = 0x0080,
             O_SYNC    = 0x0100,
+            O_APPEND  = 0x0200,
         }
     }
 }

src/libraries/Common/src/Interop/Windows/NtDll/Interop.NtQueryInformationFile.cs

@@ -16,6 +16,7 @@ internal static extern unsafe int NtQueryInformationFile(
             uint Length,
             uint FileInformationClass);
 
+        internal const uint FileAccessInformation = 8;
         internal const uint FileModeInformation = 16;
 
         internal const int STATUS_INVALID_HANDLE = unchecked((int)0xC0000008);

src/libraries/Native/Unix/System.Native/entrypoints.c

@@ -72,6 +72,7 @@ static const Entry s_sysNative[] =
     DllImportEntry(SystemNative_FcntlSetPipeSz)
     DllImportEntry(SystemNative_FcntlSetIsNonBlocking)
     DllImportEntry(SystemNative_FcntlGetIsNonBlocking)
+    DllImportEntry(SystemNative_GetIsAppend)
     DllImportEntry(SystemNative_MkDir)
     DllImportEntry(SystemNative_ChMod)
     DllImportEntry(SystemNative_FChMod)

src/libraries/Native/Unix/System.Native/pal_io.c

@@ -249,7 +249,7 @@ static int32_t ConvertOpenFlags(int32_t flags)
             return -1;
     }
 
-    if (flags & ~(PAL_O_ACCESS_MODE_MASK | PAL_O_CLOEXEC | PAL_O_CREAT | PAL_O_EXCL | PAL_O_TRUNC | PAL_O_SYNC))
+    if (flags & ~(PAL_O_ACCESS_MODE_MASK | PAL_O_CLOEXEC | PAL_O_CREAT | PAL_O_EXCL | PAL_O_TRUNC | PAL_O_SYNC | PAL_O_APPEND))
     {
         assert_msg(false, "Unknown Open flag", (int)flags);
         return -1;
@@ -267,6 +267,8 @@ static int32_t ConvertOpenFlags(int32_t flags)
         ret |= O_TRUNC;
     if (flags & PAL_O_SYNC)
         ret |= O_SYNC;
+    if (flags & PAL_O_APPEND)
+        ret |= O_APPEND;
 
     assert(ret != -1);
     return ret;
@@ -646,6 +648,11 @@ int32_t SystemNative_FcntlGetIsNonBlocking(intptr_t fd, int32_t* isNonBlocking)
     return 0;
 }
 
+int32_t SystemNative_GetIsAppend(intptr_t fd)
+{
+    return fcntl(ToFileDescriptor(fd), F_GETFL) & O_APPEND;
+}
+
 int32_t SystemNative_MkDir(const char* path, int32_t mode)
 {
     int32_t result;

src/libraries/Native/Unix/System.Native/pal_io.h

@@ -154,6 +154,7 @@ enum
     PAL_O_EXCL = 0x0040,    // When combined with CREAT, fails if file already exists
     PAL_O_TRUNC = 0x0080,   // Truncate file to length 0 if it already exists
     PAL_O_SYNC = 0x0100,    // Block writes call will block until physically written
+    PAL_O_APPEND = 0x0200,  // atomic append to the end of file
 };
 
 /**
@@ -468,6 +469,11 @@ PALEXPORT int32_t SystemNative_FcntlSetIsNonBlocking(intptr_t fd, int32_t isNonB
  */
 PALEXPORT int32_t SystemNative_FcntlGetIsNonBlocking(intptr_t fd, int32_t* isNonBlocking);
 
+/**
+ * Gets whether or not a file was opened with 0_APPEND. Returns false on failure.
+*/
+PALEXPORT int32_t SystemNative_GetIsAppend(intptr_t fd);
+
 /**
  * Create a directory. Implemented as a shim to mkdir(2).
  *

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

@@ -30,13 +30,6 @@ public override void NegativePreallocationSizeThrows()
                 () => File.OpenHandle("validPath", FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize: -1));
         }
 
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/53432")]
-        [Theory, MemberData(nameof(StreamSpecifiers))]
-        public override void FileModeAppendExisting(string streamSpecifier)
-        {
-            _ = streamSpecifier; // to keep the xUnit analyser happy
-        }
-
         [Theory]
         [InlineData(FileOptions.None)]
         [InlineData(FileOptions.Asynchronous)]

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

@@ -28,6 +28,7 @@ public void SetPositionAppendModify()
                 Assert.Equal(length + 1, fs.Position);
 
                 fs.Write(TestBuffer);
+                length = fs.Length;
                 fs.Position = length + 1;
                 Assert.Equal(length + 1, fs.Position);
             }

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

@@ -34,7 +34,7 @@ public void SeekAppendModifyThrows()
                 Assert.Equal(length, fs.Position);
 
                 fs.Write(TestBuffer);
-                Assert.Equal(length, fs.Seek(length, SeekOrigin.Begin));
+                Assert.Equal(fs.Position, fs.Seek(fs.Length, SeekOrigin.Begin));
             }
         }
     }

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

@@ -27,8 +27,10 @@ public void SetLengthAppendModifyThrows()
                 fs.Write(TestBuffer);
                 Assert.Equal(length + TestBuffer.Length, fs.Length);
 
-                fs.SetLength(length);
-                Assert.Equal(length, fs.Length);
+                if (PlatformDetection.IsNet5CompatFileStreamDisabled)
+                {
+                    Assert.Throws<IOException>(() => fs.SetLength(fs.Length - 1));
+                }
             }
         }
 

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

@@ -223,14 +223,13 @@ public virtual void FileModeAppend(string streamSpecifier)
                 fs.Write(Encoding.ASCII.GetBytes("abcde"));
                 Assert.Equal(5, fs.Length);
                 Assert.Equal(5, fs.Position);
-                Assert.Equal(1, fs.Seek(1, SeekOrigin.Begin));
 
                 fs.Write(Encoding.ASCII.GetBytes("xyz"));
-                Assert.Equal(4, fs.Position);
-                Assert.Equal(5, fs.Length);
+                Assert.Equal(8, fs.Position);
+                Assert.Equal(8, fs.Length);
             }
 
-            Assert.Equal("axyze", File.ReadAllText(fileName));
+            Assert.Equal("abcdexyz", File.ReadAllText(fileName));
         }
 
         [Theory, MemberData(nameof(StreamSpecifiers))]
@@ -262,12 +261,11 @@ public virtual void FileModeAppendExisting(string streamSpecifier)
                 fs.Write(Encoding.ASCII.GetBytes("abcde"));
                 Assert.Equal(position + 5, fs.Position);
 
-                Assert.Equal(position, fs.Seek(position, SeekOrigin.Begin));
-                Assert.Equal(position + 1, fs.Seek(1, SeekOrigin.Current));
                 fs.Write(Encoding.ASCII.GetBytes("xyz"));
+                Assert.Equal(position + 8, fs.Position);
             }
 
-            Assert.Equal(initialContents + "axyze", File.ReadAllText(fileName));
+            Assert.Equal(initialContents + "abcdexyz", File.ReadAllText(fileName));
         }
     }
 }

src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj

@@ -9,6 +9,8 @@
   <ItemGroup>
     <Compile Include="..\**\*.cs" />
     <Compile Remove="..\DisabledFileLockingTests\DisabledFileLockingSwitchTests.cs" />
+    <!-- .NET 5 did not properly support FileStreams created out of handle opened with APPEND -->
+    <Compile Remove="..\File\OpenHandle.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
     <Compile Remove="..\**\*.Windows.cs" />

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

@@ -47,6 +47,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;
+            handle._isAppend = (flags & Interop.Sys.OpenFlags.O_APPEND) != 0 ? NullableBool.True : NullableBool.False;
 
             if (handle.IsInvalid)
             {
@@ -216,7 +217,6 @@ private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mo
                     }
                     break;
 
-                case FileMode.Append: // Append is the same as OpenOrCreate, except that we'll also separately jump to the end later
                 case FileMode.OpenOrCreate:
                     flags |= Interop.Sys.OpenFlags.O_CREAT;
                     break;
@@ -232,6 +232,14 @@ private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mo
                 case FileMode.CreateNew:
                     flags |= (Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL);
                     break;
+
+                case FileMode.Append:
+                    flags |= Interop.Sys.OpenFlags.O_CREAT;
+                    if (!FileStreamHelpers.UseNet5CompatStrategy) // we used to seek to the end of file in .NET 5
+                    {
+                        flags |= Interop.Sys.OpenFlags.O_APPEND;
+                    }
+                    break;
             }
 
             // Translate FileAccess.  All possible values map cleanly to corresponding values for open.
@@ -389,11 +397,18 @@ private bool GetCanSeek()
             return canSeek == NullableBool.True;
         }
 
-        private enum NullableBool
+        private bool GetIsAppend()
         {
-            Undefined = 0,
-            False = -1,
-            True = 1
+            Debug.Assert(!IsClosed);
+            Debug.Assert(!IsInvalid);
+
+            NullableBool isAppend = _isAppend;
+            if (isAppend == NullableBool.Undefined)
+            {
+                _isAppend = isAppend = Interop.Sys.Fcntl.GetIsAppend(this) ? NullableBool.True : NullableBool.False;
+            }
+
+            return isAppend == NullableBool.True;
         }
     }
 }

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

@@ -69,10 +69,19 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode,
             // the security attributes class.  Don't leave this bit set.
             share &= ~FileShare.Inheritable;
 
-            // Must use a valid Win32 constant here...
-            if (mode == FileMode.Append)
+            if (!FileStreamHelpers.UseNet5CompatStrategy)
             {
-                mode = FileMode.OpenOrCreate;
+                if (mode == FileMode.Append)
+                {
+                    fAccess |= (int)Interop.NtDll.DesiredAccess.FILE_APPEND_DATA;
+                }
+                else
+                {
+                    // FILE_GENERIC_WRITE consists of FILE_APPEND_DATA amongst many other flags
+                    // we don't want to recognize all FILE_GENERIC_WRITE handles as opened for APPEND
+                    // otherwise we would not be able to recognize them in GetIsAppend
+                    fAccess &= ~(int)Interop.NtDll.DesiredAccess.FILE_APPEND_DATA;
+                }
             }
 
             int flagsAndAttributes = (int)options;
@@ -83,7 +92,8 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode,
             // (note that this is the effective default on CreateFile2)
             flagsAndAttributes |= (Interop.Kernel32.SecurityOptions.SECURITY_SQOS_PRESENT | Interop.Kernel32.SecurityOptions.SECURITY_ANONYMOUS);
 
-            SafeFileHandle fileHandle = Interop.Kernel32.CreateFile(fullPath, fAccess, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero);
+            FileMode windowsMode = mode == FileMode.Append ? FileMode.OpenOrCreate : mode; // Must use a valid Win32 constant here..
+            SafeFileHandle fileHandle = Interop.Kernel32.CreateFile(fullPath, fAccess, share, &secAttrs, windowsMode, flagsAndAttributes, IntPtr.Zero);
             if (fileHandle.IsInvalid)
             {
                 // Return a meaningful exception with the full path.
@@ -103,6 +113,7 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode,
 
             fileHandle._path = fullPath;
             fileHandle._fileOptions = options;
+            fileHandle._isAppend = mode == FileMode.Append ? NullableBool.True : NullableBool.False;
             return fileHandle;
         }
 
@@ -252,5 +263,33 @@ internal int GetFileType()
 
             return fileType;
         }
+
+        private unsafe bool GetIsAppend()
+        {
+            NullableBool isAppend = _isAppend;
+            if (isAppend == NullableBool.Undefined)
+            {
+                Interop.NtDll.DesiredAccess access;
+                int ntStatus = Interop.NtDll.NtQueryInformationFile(
+                    FileHandle: this,
+                    IoStatusBlock: out _,
+                    FileInformation: &access,
+                    Length: sizeof(Interop.NtDll.DesiredAccess),
+                    FileInformationClass: Interop.NtDll.FileModeInformation);
+
+                if (ntStatus != Interop.StatusOptions.STATUS_SUCCESS) // we have failed, so we assume it's false
+                {
+                    _isAppend = isAppend = NullableBool.False;
+                }
+                else
+                {
+                    // FILE_GENERIC_WRITE consists of FILE_APPEND_DATA amongst many other flags
+                    // we don't want to recognize all FILE_GENERIC_WRITE handles as opened for APPEND
+                    _isAppend = isAppend = (access & Interop.NtDll.DesiredAccess.FILE_APPEND_DATA) != 0 && (access & Interop.NtDll.DesiredAccess.FILE_GENERIC_WRITE) == 0 ? NullableBool.True : NullableBool.False;
+                }
+            }
+
+            return isAppend == NullableBool.True;
+        }
     }
 }

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

@@ -7,6 +7,7 @@ namespace Microsoft.Win32.SafeHandles
 {
     public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
+        private volatile NullableBool _isAppend = NullableBool.Undefined;
         private string? _path;
 
         /// <summary>
@@ -20,5 +21,14 @@ public SafeFileHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHand
         }
 
         internal string? Path => _path;
+
+        internal bool IsAppend => GetIsAppend();
+
+        private enum NullableBool
+        {
+            Undefined = 0,
+            False = -1,
+            True = 1
+        }
     }
 }

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

@@ -457,6 +457,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Enumeration\FileSystemEnumerable.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Enumeration\FileSystemEnumerableFactory.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Enumeration\FileSystemName.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\AppendFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\BufferedFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\DerivedFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamHelpers.cs" />
@@ -1939,6 +1940,9 @@
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.UnixFileSystemTypes.cs">
       <Link>Common\Interop\Unix\System.Native\Interop.UnixFileSystemTypes.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Fcntl.cs">
+      <Link>Common\Interop\Unix\System.Native\Interop.Fcntl.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.FLock.cs">
       <Link>Common\Interop\Unix\System.Native\Interop.FLock.cs</Link>
     </Compile>

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

@@ -90,10 +90,12 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
                     // The Windows implementation uses WriteFile, which ignores the offset if the handle
                     // isn't seekable.  We do the same manually with PWrite vs Write, in order to enable
                     // the function to be used by FileStream for all the same situations.
-                    int bytesWritten = handle.CanSeek ?
-                        Interop.Sys.PWrite(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length), fileOffset) :
-                        Interop.Sys.Write(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length));
 
+                    // POSIX requires that pwrite should respect provided offset even for handles opened with O_APPEND.
+                    // But Linux and BSD don't do that, moreover their behaviour is different. So we always use write for O_APPEND.
+                    int bytesWritten = handle.CanSeek && !handle.IsAppend
+                       ? Interop.Sys.PWrite(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length), fileOffset)
+                       : Interop.Sys.Write(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length));
                     FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path);
                     if (bytesWritten == buffer.Length)
                     {