diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 54486b367900cc..393154799ce2f4 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -2,10 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Buffers; using System.ComponentModel; using System.Diagnostics; using System.IO; +using System.IO.Strategies; using System.Runtime.InteropServices; using System.Threading; @@ -13,10 +13,10 @@ namespace Microsoft.Win32.SafeHandles { public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid { - internal const FileOptions NoBuffering = (FileOptions)0x20000000; + private const FileOptions UninitializedOptions = (FileOptions)(-1); private long _length = -1; // negative means that hasn't been fetched. private bool _lengthCanBeCached; // file has been opened for reading and not shared for writing. - private volatile FileOptions _fileOptions = (FileOptions)(-1); + private FileOptions _fileOptions = UninitializedOptions; public SafeFileHandle() : base(true) { @@ -106,7 +106,7 @@ public static partial void CreateAnonymousPipe(out SafeFileHandle readHandle, ou public bool IsAsync => (GetFileOptions() & FileOptions.Asynchronous) != 0; - internal bool IsNoBuffering => (GetFileOptions() & NoBuffering) != 0; + internal bool IsNoBuffering => (GetFileOptions() & FileStreamHelpers.NoBuffering) != 0; internal bool CanSeek => !IsClosed && Type == FileHandleType.RegularFile; @@ -280,7 +280,7 @@ private void InitThreadPoolBinding() internal unsafe FileOptions GetFileOptions() { FileOptions fileOptions = _fileOptions; - if (fileOptions != (FileOptions)(-1)) + if (fileOptions != UninitializedOptions) { return fileOptions; } @@ -323,7 +323,7 @@ internal unsafe FileOptions GetFileOptions() } if ((options & Interop.NtDll.CreateOptions.FILE_NO_INTERMEDIATE_BUFFERING) != 0) { - result |= NoBuffering; + result |= FileStreamHelpers.NoBuffering; } return _fileOptions = result; @@ -331,11 +331,22 @@ internal unsafe FileOptions GetFileOptions() internal FileHandleType GetFileTypeCore() { + Debug.Assert(Path is null || _fileOptions != UninitializedOptions, "When Path is set, _fileOptions are also provided."); + int kernelFileType = Interop.Kernel32.GetFileType(this); return kernelFileType switch { Interop.Kernel32.FileTypes.FILE_TYPE_CHAR => FileHandleType.CharacterDevice, Interop.Kernel32.FileTypes.FILE_TYPE_PIPE => GetPipeOrSocketType(), + // GetFileType can return FILE_TYPE_DISK for regular files, directories and symbolic links. + // When Path is not null, it means that the handle was created by SafeFileHandle.Open. + // This method resolves symbolic links, so it can't be a symbolic link. + // However, it accepts FILE_FLAG_BACKUP_SEMANTICS as an option, + // which makes it possible to open a directory. + // So when Path is not null and options don't include FILE_FLAG_BACKUP_SEMANTICS, + // it's a regular file. In such case, we don't need to call GetDiskBasedType() which would be + // an extra sys-call. + Interop.Kernel32.FileTypes.FILE_TYPE_DISK when Path is not null && ((_fileOptions & FileStreamHelpers.BackupOrRestore) == 0) => FileHandleType.RegularFile, Interop.Kernel32.FileTypes.FILE_TYPE_DISK => GetDiskBasedType(), _ => FileHandleType.Unknown }; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 3ece14b5fd82a7..ad3614376ab396 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -448,7 +448,7 @@ internal static void WriteGatherAtOffset(SafeFileHandle handle, IReadOnlyList handle.IsAsync && ((handle.GetFileOptions() & SafeFileHandle.NoBuffering) != 0); + => handle.IsAsync && ((handle.GetFileOptions() & FileStreamHelpers.NoBuffering) != 0); // From the same source: // "Each buffer must be at least the size of a system memory page and must be aligned on a system diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs index e3676a6a4e2835..af2bfd18220908 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs @@ -8,10 +8,13 @@ namespace System.IO.Strategies { internal static partial class FileStreamHelpers { + internal const FileOptions NoBuffering = (FileOptions)0x20000000; + internal const FileOptions BackupOrRestore = (FileOptions)0x02000000; + // NOTE: any change to FileOptions enum needs to be matched here as it's used in the error validation private const FileOptions ValidFileOptions = FileOptions.WriteThrough | FileOptions.Asynchronous | FileOptions.RandomAccess | FileOptions.DeleteOnClose | FileOptions.SequentialScan | FileOptions.Encrypted - | (FileOptions)0x20000000 /* NoBuffering */ | (FileOptions)0x02000000 /* BackupOrRestore */; + | NoBuffering | BackupOrRestore; /// Caches whether Serialization Guard has been disabled for file writes private static int s_cachedSerializationSwitch; diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs index 00fba057c13d09..90ad33818e2f1d 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs @@ -12,20 +12,29 @@ namespace System.IO.Tests [PlatformSpecific(TestPlatforms.Windows)] public class SafeFileHandle_GetFileType_Windows : FileSystemTest { - [Fact] - public unsafe void GetFileType_Directory() + [Theory] + [InlineData(true)] + [InlineData(false)] + public unsafe void GetFileType_Directory(bool usePublicAPI) { string path = GetTestFilePath(); Directory.CreateDirectory(path); - using SafeFileHandle handle = Interop.Kernel32.CreateFile( - path, - Interop.Kernel32.GenericOperations.GENERIC_READ, - FileShare.ReadWrite, - null, - FileMode.Open, - Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS, - IntPtr.Zero); + using SafeFileHandle handle = usePublicAPI + ? File.OpenHandle( + path, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + (FileOptions)Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS) + : Interop.Kernel32.CreateFile( + path, + Interop.Kernel32.GenericOperations.GENERIC_READ, + FileShare.ReadWrite, + null, + FileMode.Open, + Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS, + IntPtr.Zero); Assert.False(handle.IsInvalid); Assert.Equal(FileHandleType.Directory, handle.Type);