Ensure FileStream.CanSeek works for PIPE_ACCESS_OUTBOUND pipes

[Copilot]

fixes #131503

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR adjusts how SafeFileHandle.CanSeek is determined and cached so FileStream construction works reliably on Windows for handles that can’t be safely probed via file-type detection (notably write-only named pipes), and adds a regression test to cover the scenario.

Changes:

• Centralizes SafeFileHandle.CanSeek caching state in the shared SafeFileHandle partial.
• Updates Windows seekability probing to use SetFilePointerEx(..., 0, FILE_CURRENT) and cache the result.
• Adds a Windows test validating FileStream construction over a write-only named-pipe handle (sync and overlapped) reports CanSeek == false and preserves IsAsync.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds a regression theory for FileStream over write-only named-pipe handles (sync/overlapped).
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Implements Windows seekability probing via SetFilePointerEx and caches the result.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs	Removes the per-Unix CanSeek member/field now that caching is centralized.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.cs	Adds shared _canSeek cache and CanSeek property used by platform-specific implementations.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:98

• After switching to Interop.Kernel32.CreateNamedPipeFileHandle, the local CreateNamedPipe P/Invoke is redundant and should be removed to avoid carrying duplicate interop declarations.

        [DllImport("kernel32.dll", EntryPoint = "CreateNamedPipeW", CharSet = CharSet.Unicode, SetLastError = true)]
        private static extern SafeFileHandle CreateNamedPipe(
            string pipeName,
            int openMode,
            int pipeMode,

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback

[Copilot]

@copilot address my feedback

Addressed the requested review feedback in 17a1774.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:76

• SECURITY_ATTRIBUTES is passed by ref with nLength == 0 and the pipe name omits the LOCAL namespace. Initializing SECURITY_ATTRIBUTES via the helper and using the \\.\pipe\LOCAL\... naming pattern (as used by SafeFileHandle.CreateAnonymousPipe) makes this test more robust on AppContainer scenarios and avoids relying on an uninitialized SECURITY_ATTRIBUTES layout.

            Interop.Kernel32.SECURITY_ATTRIBUTES securityAttributes = default;
            using SafeFileHandle handle = Interop.Kernel32.CreateNamedPipeFileHandle(
                $@"\\.\pipe\{Guid.NewGuid():N}",

[adamsitnik]

@copilot address my feedback and fix the test project build

[Copilot]

@copilot address my feedback and fix the test project build

Addressed in 6cbbf8f. The test now uses only PIPE_ACCESS_OUTBOUND, and the Windows test project includes the required shared named-pipe interop sources.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[adamsitnik]

@EgorBot -windows_intel

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Benchmarks).Assembly).Run(args);

[MemoryDiagnoser]
public class Benchmarks
{
    private string? _filePath;

    [GlobalSetup]
    public void Setup()
    {
        _filePath = Path.GetTempFileName();
        File.WriteAllBytes(_filePath, new byte[1_000]);
    }

    [GlobalCleanup]
    public void Cleanup() => File.Delete(_filePath!);

    [Benchmark]
    public byte[] ReadAllBytes() => File.ReadAllBytes(_filePath!);

    [Benchmark]
    public Task ReadAllBytesAsync() => File.ReadAllBytesAsync(_filePath!);

    [Benchmark]
    public bool OpenAndCanSeek()
    {
        using FileStream handle = File.OpenRead(_filePath!);
        return handle.CanSeek;
    }
}

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:75

• The test creates the named pipe using the plain \\.\pipe\... namespace. In AppContainer environments Windows requires the \\.\pipe\LOCAL\ prefix (see SafeFileHandle.Windows.cs where CreateAnonymousPipe uses \\.\pipe\LOCAL\... for this reason), so this test can fail under those runs.

Consider using the LOCAL namespace unconditionally (it works both inside and outside AppContainer) to keep the test robust across CI environments.

                $@"\\.\pipe\{Guid.NewGuid():N}",

[adamsitnik]

@EgorBot -windows_intel

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Benchmarks).Assembly).Run(args);

[MemoryDiagnoser]
public class Benchmarks
{
    private string? _filePath;

    [GlobalSetup]
    public void Setup()
    {
        _filePath = Path.GetTempFileName();
        File.WriteAllBytes(_filePath, new byte[1_000]);
    }

    [GlobalCleanup]
    public void Cleanup() => File.Delete(_filePath!);

    [Benchmark]
    public byte[] ReadAllBytes() => File.ReadAllBytes(_filePath!);

    [Benchmark]
    public Task ReadAllBytesAsync() => File.ReadAllBytesAsync(_filePath!);

    [Benchmark]
    public bool OpenAndCanSeek()
    {
        using FileStream handle = File.OpenRead(_filePath!);
        return handle.CanSeek;
    }
}

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• GetCanSeekCore currently treats any FILE_TYPE_DISK handle as seekable when _cachedFileType is unset, which is broader than the previous Type == RegularFile behavior and can permanently cache an incorrect true result (e.g., for directory/symlink handles) without ever consulting the more specific file-type logic. It also doesn’t match the PR description’s claim of probing seekability with SetFilePointerEx. Consider keeping the cheap pipe-safe GetFileType gate, but use the existing SetFilePointerEx probe for disk handles (and fast-path known-regular files opened via SafeFileHandle.Open).

        // That is why we use the cached file type (if available), and if not available, we use GetFileType to determine whether the file is seekable or not.
        private bool GetCanSeekCore() => _cachedFileType != -1
            ? (FileHandleType)_cachedFileType == FileHandleType.RegularFile
            : Interop.Kernel32.GetFileType(this) == Interop.Kernel32.FileTypes.FILE_TYPE_DISK;

[adamsitnik]

LGTM, the benchmark results show there is no performance regression (actually, even a small improvement):

BenchmarkDotNet v0.16.0-preview.1, Windows 11 (10.0.26100.33158/24H2/2024Update/HudsonValley) (Hyper-V)
INTEL XEON PLATINUM 8573C 2.30GHz, 1 CPU, 8 logical and 4 physical cores
Memory: 31.99 GB Total, 27.1 GB Available
.NET SDK 11.0.100-rc.1.26410.104
  [Host]     : .NET 11.0.0 (11.0.0-rc.1.26410.104, 11.0.26.41104), X64 RyuJIT x86-64-v4

Method	Toolchain	Mean	Error	Ratio	Allocated	Alloc Ratio
ReadAllBytes	main	25.05 μs	0.483 μs	1.02	1104 B	1.00
ReadAllBytes	PR #132077	24.62 μs	0.467 μs	1.00	1104 B	1.00
ReadAllBytesAsync	main	48.17 μs	0.702 μs	1.06	1624 B	1.00
ReadAllBytesAsync	PR #132077	45.58 μs	0.901 μs	1.00	1624 B	1.00
OpenAndCanSeek	main	11.23 μs	0.112 μs	1.01	248 B	1.00
OpenAndCanSeek	PR #132077	11.13 μs	0.103 μs	1.00	248 B	1.00

Full logs

[adamsitnik]

/ba-g networking test failures are unrelated