Ensure FileStream.CanSeek works for PIPE_ACCESS_OUTBOUND pipes

Project: dotnet/runtime | Language: C# | Concepts: error-handling, refactoring, testing-strategy | Difficulty: mid
Link: https://github.com/dotnet/runtime/pull/132077

The problem

On Windows, when a named pipe is created with PIPE_ACCESS_OUTBOUND (write-only), FileStream.CanSeek incorrectly returns true, even though pipes are not seekable. This can break code that relies on correct seekability reporting, especially in scenarios involving inter-process communication with named pipes.

Why it's non-trivial

A naive fix might just check the file type or attempt a seek operation, but:

• Windows APIs can behave inconsistently: SetFilePointerEx may succeed or fail depending on pipe configuration and access mode.
• Some detection methods require permissions that may not be granted (e.g., GetNamedPipeInfo may fail with ERROR_ACCESS_DENIED for certain pipe types).
• The logic needs to be correct and consistent across platforms (Windows/Unix), and avoid introducing race conditions or performance regressions.

The approach

1. Centralizing and Caching the Seekability Logic

Previously, CanSeek was implemented separately in platform-specific files, with some duplication and inconsistent caching. The PR refactors this so that:

+        internal bool CanSeek
+        {
+            get
+            {
+                if (IsClosed)
+                {
+                    return false;
+                }
+
+                NullableBool canSeek = _canSeek;
+                if (canSeek == NullableBool.Undefined)
+                {
+                    _canSeek = canSeek = GetCanSeekCore() ? NullableBool.True : NullableBool.False;
+                }
+
+                return canSeek == NullableBool.True;
+            }
+        }

What this does:

• Moves the caching and thread-safety logic for CanSeek into the main SafeFileHandle class.
• Uses a NullableBool field to cache the result, avoiding repeated system calls.
• Calls a platform-specific GetCanSeekCore() method to actually determine seekability.

2. Platform-specific Seekability Checks

On Unix:

-private bool GetCanSeek()
-{
-    Debug.Assert(!IsClosed);
-    Debug.Assert(!IsInvalid);
-
-    NullableBool canSeek = _canSeek;
-    if (canSeek == NullableBool.Undefined)
-    {
-        _canSeek = canSeek = Interop.Sys.LSeek(this, 0, Interop.Sys.SeekWhence.SEEK_CUR) >= 0 ? NullableBool.True : NullableBool.False;
-    }
-
-    return canSeek == NullableBool.True;
-}
+private bool GetCanSeekCore() => Interop.Sys.LSeek(this, 0, Interop.Sys.SeekWhence.SEEK_CUR) >= 0;

What this does:

• Removes the old caching logic from the Unix-specific file.
• Implements GetCanSeekCore as a single system call: if lseek succeeds, the file is seekable.

On Windows:

+        private bool GetCanSeekCore() => _cachedFileType != -1
+            ? (FileHandleType)_cachedFileType == FileHandleType.RegularFile
+            : Interop.Kernel32.GetFileType(this) == Interop.Kernel32.FileTypes.FILE_TYPE_DISK;

What this does:

• Uses the cached file type if available, otherwise calls GetFileType.
• Only regular files (FILE_TYPE_DISK) are considered seekable.
• Avoids problematic APIs like GetNamedPipeInfo that can fail for some pipes.

3. Testing the Fix

A new test is added to ensure that a FileStream opened on a write-only named pipe correctly reports CanSeek == false:

[Theory]
[InlineData(false)]
[InlineData(true)]
public void FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync(bool isAsync)
{
    int openMode = (int)Interop.Kernel32.PipeOptions.PIPE_ACCESS_OUTBOUND;
    if (isAsync)
    {
        openMode |= Interop.Kernel32.FileOperations.FILE_FLAG_OVERLAPPED;
    }

    Interop.Kernel32.SECURITY_ATTRIBUTES securityAttributes = Interop.Kernel32.SECURITY_ATTRIBUTES.Create();
    using SafeFileHandle handle = Interop.Kernel32.CreateNamedPipeFileHandle(
        $@"\\.\pipe\{Guid.NewGuid():N}",
        openMode,
        pipeMode: (int)Interop.Kernel32.PipeOptions.PIPE_TYPE_BYTE,
        maxInstances: 1,
        outBufferSize: 0,
        inBufferSize: 0,
        defaultTimeout: 0,
        ref securityAttributes);
    Assert.False(handle.IsInvalid);

    using FileStream stream = new(handle, FileAccess.Write, bufferSize: 4096, isAsync);
    Assert.False(stream.CanSeek);
    Assert.Equal(isAsync, stream.IsAsync);
}

What this does:

• Creates a named pipe with PIPE_ACCESS_OUTBOUND (write-only).
• Wraps it in a FileStream and asserts that CanSeek is false, both for sync and async streams.

4. Test Project Maintenance

To support the test, the PR ensures that the necessary interop files are included in the test project:

+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CreateNamedPipe_SafeFileHandle.cs" Link="Common\Interop\Windows\Interop.CreateNamedPipe_SafeFileHandle.cs" />
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.PipeOptions.cs" Link="Common\Interop\Windows\Interop.PipeOptions.cs" />

What this does:

• Ensures the test project can call the correct interop methods for named pipes.

From the review thread

keep the "not using bool? as it's not thread safe" comment here, as there are still some NullableBool fileds left here

There is too much code duplication.

@copilot do the following:

• extend CanSeek with GetCanSeek logic:

        internal bool CanSeek
        {
           get
           {
               if (IsClosed)
               {
                   return false;
               }
               
               
            NullableBool canSeek = _canSeek;
            if (canSeek == NullableBool.Undefined)
            {
                _canSeek = canSeek = CanSeekCore() ? NullableBool.True : NullableBool.False;
            }

            return canSeek == NullableBool.True;
           }
        }

• remove GetCanSeek() from SafeFileHandle.Windows.cs and SafeFileHandle.Unix.cs
• extend SafeFileHandle.Unix.cs with:

private bool GetCanSeekCore() =>Interop.Sys.LSeek(this, 0, Interop.Sys.SeekWhence.SEEK_CUR) >= 0;

• extend SafeFileHandle.Windows.cs with:

private bool GetCanSeekCore() =>Interop.Kernel32.SetFilePointerEx(this, 0, out _, (uint)SeekOrigin.Current);

Prefer using the existing Interop.Kernel32.CreateNamedPipeFileHandle wrapper (already used by SafeFileHandle.CreateAnonymousPipe) instead of introducing a new P/Invoke surface in this test. This reduces duplication and keeps the tests aligned with the repo’s interop definitions.

don't add new magic numbers, just reuse existing ones from src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PipeOptions.cs

the original repro is not using FILE_FLAG_FIRST_PIPE_INSTANCE, I don't see a reason to use it here.

            int openMode = (int)Interop.Kernel32.PipeOptions.PIPE_ACCESS_OUTBOUND;

I suspect that this even won't compile, as the src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateNamedPipe_SafeFileHandle.cs and src/libraries/Common/src/Interop/Windows/Kernel32/Interop.PipeOptions.cs files needs to be added here:

https://github.com/dotnet/runtime/blob/17a177437412cd17e080958fff682a57355c0e9d/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/System.IO.FileSystem.Tests.csproj#L121-L151

Takeaways

• Centralize platform-variant logic behind a single property: Avoid code duplication and inconsistencies by using a single property with platform-specific implementations behind a common interface.
• Cache expensive or repeated system calls: Use thread-safe caching for properties like seekability that require system calls, to improve performance and avoid race conditions.
• Write regression tests for edge-case resource types: When fixing subtle bugs (like seekability on pipes), add targeted tests to prevent regressions, especially for resource types that behave differently than files.
• Prefer existing abstractions and constants over magic numbers: Use repo-wide interop wrappers and enums to reduce duplication and errors in P/Invoke code and tests.