Make FileStream.Name work for FileStream instances created out of handle by Copilot · Pull Request #126629 · dotnet/runtime

Copilot · 2026-04-08T06:08:25Z

FileStream.Name returned [Unknown] for streams created from raw OS handles because SafeFileHandle had no mechanism to query the OS for the path. This extends the internal SafeFileHandle.Path property to resolve the path from the kernel when not already known, so FileStream.Name returns the real path in those cases.

Changes

Native (`System.Native`)

New SystemNative_GetFilePathFromHandle(fd, buf, size) in pal_io.h/pal_io.c:
- Linux: readlink("/proc/self/fd/<fd>", buf, size)
- macOS/FreeBSD/Solaris: fcntl(fd, F_GETPATH, buf) (guarded by HAVE_F_GETPATH)
- Other: errno = ENOTSUP; return -1
Precondition uses a single assert(buffer != NULL && bufferSize > 0) (no redundant runtime check)
On F_GETPATH platforms, added a MAXPATHLEN guard: returns errno = ENAMETOOLONG if buffer is too small, preventing native buffer overwrite
Added HAVE_F_GETPATH cmake detection + pal_config.h.in entry
Registered in entrypoints.c

Managed (`System.Private.CoreLib`)

internal string? SafeFileHandle.Path caches lazily via _path ??= GetPath() — all caching is centralized in the Path property; platform-specific GetPath() implementations just fetch the value
SafeFileHandle.Windows.cs: GetPath() via GetFinalPathNameByHandle, correctly handles both \\?\C:\foo and \\?\UNC\server\share prefixes; uses 26-char initial buffer in non-RELEASE builds to exercise buffer-growing code path
SafeFileHandle.Unix.cs: GetPath() simplified to Interop.Sys.GetFilePathFromHandle(this) (no caching; Path handles it)
Unix interop GetFilePathFromHandle returns non-nullable string; returns SR.IO_UnknownFileName instead of null on failure; uses SafeFileHandle parameter (not IntPtr) and a fixed 4096-byte ArrayPool buffer; uses AsSpan for null-terminator search
OSFileStreamStrategy.Name unchanged — still _fileHandle.Path ?? SR.IO_UnknownFileName
No public API changes

Tests

GetFileType.cs: added two FileStream.Name tests — one for path-via-open, one for path-via-raw-handle (asserts exact path equality; CI will identify any unsupported platforms)
Platform-specific raw-handle name tests removed from GetFileType.Unix.cs and GetFileType.Windows.cs (covered by shared test)
ProcessHandlesTests.cs: added NonInheritedFileHandle_IsNotAvailableInChildProcess (decorated with [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] only) — saves the raw handle value, creates a FileStream from the handle and asserts Assert.Equal(path, parentFs.Name) in the parent process, then spawns a child with InheritedHandles = [] and verifies the file handle is not accessible

Example

// Previously returned "[Unknown]" for handles opened externally
using var handle = new SafeFileHandle(nativeHandle, ownsHandle: false);

// FileStream.Name now resolves correctly via OS lookup
using var fs = new FileStream(handle, FileAccess.Read);
Console.WriteLine(fs.Name); // e.g. "/tmp/foo.txt" on Linux, "C:\tmp\foo.txt" on Windows

dotnet-policy-service · 2026-04-08T06:45:48Z

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

- Add SystemNative_GetFilePathFromHandle to pal_io.h and pal_io.c: - Linux: readlink /proc/self/fd/<fd> - BSD/macOS: fcntl(fd, F_GETPATH, buf) - Other: errno=ENOTSUP, return -1 - Add HAVE_F_GETPATH cmake check and pal_config.h.in define - Add DllImportEntry for new native function - Add Interop.GetFilePathFromHandle.cs Unix P/Invoke wrapper - Add public Name property to SafeFileHandle.cs - Add GetName() implementations to SafeFileHandle.Unix.cs and SafeFileHandle.Windows.cs - Update OSFileStreamStrategy.Name to use SafeFileHandle.Name - Update System.Runtime ref assembly with new Name property - Add Name tests to GetFileType.cs, GetFileType.Unix.cs, GetFileType.Windows.cs - Add NonInheritedFileHandle_IsNotAvailableInChildProcess test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/65fff4ad-9081-4ef8-b744-02fef761db67 Co-authored-by: adamsitnik <[email protected]>

adamsitnik

@copilot address my feedback

src/libraries/System.Runtime/ref/System.Runtime.cs

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

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

src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs

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

adamsitnik · 2026-04-08T07:13:12Z

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

+                // GetFinalPathNameByHandle always returns with extended DOS prefix (\\?\).
+                // Trim the prefix to keep the result consistent with the path stored in _path.


Is the conversion always as simple as removing first four characters?

Not always — for UNC paths, GetFinalPathNameByHandle returns \\?\UNC\server\share, and just removing 4 chars gives UNC\server\share (wrong). Fixed in b600d32 to handle both cases:

\\?\UNC\server\share → \\server\share (strips \\?\UNC\, prepends \\)

\\?\C:\foo → C:\foo (strips \\?\)

src/native/libs/System.Native/pal_io.c

…SafeFileHandle in interop Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d6f0695b-ba5d-44a8-aa39-c07ea398bcd0 Co-authored-by: adamsitnik <[email protected]>

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d6f0695b-ba5d-44a8-aa39-c07ea398bcd0 Co-authored-by: adamsitnik <[email protected]>

adamsitnik

@copilot address my feedback

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

src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs

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

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

...libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Unix.cs

...raries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs

src/native/libs/System.Native/pal_io.c

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/bb17e842-35f7-4bae-8d99-bf98e21541b2 Co-authored-by: adamsitnik <[email protected]>

src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs

adamsitnik

@copilot fix following build failure:

src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs(557,84): error CS1739: (NETCORE_ENGINEERING_TELEMETRY=Build) The best overload for 'FileStream' does not have a parameter named 'leaveOpen'

make sure you build the product and run the tests before pushing an update

…Stream, remove redundant SkipOnPlatform Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/45057e03-9697-4819-a2c0-72b02fddccf6 Co-authored-by: adamsitnik <[email protected]>

Copilot · 2026-04-08T10:32:33Z

@copilot fix following build failure:

Fixed in a33d3f9. FileStream has no leaveOpen parameter. Restructured to save rawHandle = fileHandle.DangerousGetHandle() before creating the FileStream (which takes ownership of the handle), then assert Assert.Equal(path, parentFs.Name) inside the using block. rawHandle is then passed to the child process test as before.

src/native/libs/System.Native/pal_io.c

Copilot

Pull request overview

This PR makes FileStream.Name return the real file path for FileStream instances created from raw OS handles by teaching SafeFileHandle to resolve a path from the OS when no cached path is available.

Changes:

Added a new native PAL export to resolve a file path from a handle/FD (Linux via /proc/self/fd, Apple/BSD/Solaris via fcntl(F_GETPATH)).
Updated SafeFileHandle.Path to delegate to a platform-specific GetPath() implementation that can query the OS on-demand.
Added/updated tests to validate FileStream.Name for both path-opened streams and raw-handle-opened streams, and added a new process handle inheritance test.

Reviewed changes

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

Show a summary per file

File	Description
src/native/libs/System.Native/pal_io.h	Declares `SystemNative_GetFilePathFromHandle` export.
src/native/libs/System.Native/pal_io.c	Implements path resolution from FD via `F_GETPATH`/`/proc/self/fd`.
src/native/libs/System.Native/entrypoints.c	Registers the new native entrypoint for interop.
src/native/libs/configure.cmake	Detects `F_GETPATH` support at configure time.
src/native/libs/Common/pal_config.h.in	Adds `HAVE_F_GETPATH` to generated PAL config.
src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetFilePathFromHandle.cs	Adds Unix interop wrapper to call the new native export and decode the returned UTF-8 path.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Includes the new Unix interop file in CoreLib builds.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.cs	Changes `SafeFileHandle.Path` to resolve via `GetPath()` rather than only returning `_path`.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs	Implements `GetPath()` using the new Unix interop and caches the resolved path.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Implements `GetPath()` using `GetFinalPathNameByHandle` and normalizes extended path prefixes.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.cs	Adds cross-platform tests for `FileStream.Name` for path-opened and raw-handle-opened cases.
src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs	Adds a RemoteExecutor test ensuring a non-inherited file handle isn’t usable in a child process.

src/native/libs/System.Native/pal_io.c

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

github-actions · 2026-04-08T11:34:11Z

🤖 Copilot Code Review — PR #126629

Note

This review was generated by GitHub Copilot using multiple AI models (Claude Opus 4.6 primary, Claude Sonnet 4.5 secondary).

Holistic Assessment

Motivation: Resolving file paths from OS-level handles is a real gap — when a SafeFileHandle is constructed from a raw IntPtr, FileStream.Name previously returned "[Unknown]". This enables richer diagnostics and correct behavior for interop-created handles.

Approach: The approach is sound — using /proc/self/fd/<fd> on Linux, fcntl(F_GETPATH) on macOS/BSD, and GetFinalPathNameByHandle on Windows are the standard OS mechanisms. The native code is well-structured with proper platform guards. The UNC prefix stripping on Windows is more thorough than the existing code in FileSystem.Windows.cs.

Summary: ⚠️ Needs Changes. The Windows GetPath() implementation has a caching omission that causes repeated OS calls on every handle.Path access, which is a performance bug affecting ~40 call sites. Additionally, a test appears to be vacuous due to a handle lifetime issue. The Unix side and native code look good.

Detailed Findings

⚠️ Windows `GetPath()` does not cache the resolved path — performance bug

Location: SafeFileHandle.Windows.cs — GetPath() method

The Unix implementation correctly caches the resolved path:

// Unix — caches via ??=
internal string? GetPath() => _path ??= Interop.Sys.GetFilePathFromHandle(this);

The Windows implementation checks _path at the top but never writes back on success:

internal unsafe string? GetPath()
{
    if (_path is not null)
    {
        return _path;  // ✅ cache check
    }
    // ... expensive GetFinalPathNameByHandle call ...
    return new string(buffer, 0, (int)result);  // ❌ never sets _path
}

This means every access to handle.Path on Windows triggers a GetFinalPathNameByHandle system call when the handle was created from a raw IntPtr. This property is accessed from:

OSFileStreamStrategy.Name (line 91) — user-facing FileStream.Name, could be accessed repeatedly
~35+ error paths in FileStreamHelpers.Windows.cs, RandomAccess.Windows.cs, etc. — multiple accesses per error

Suggested fix: Cache the result before returning, similar to Unix:

// After the prefix stripping, before returning:
return _path = new string(buffer, PathInternal.DevicePrefixLength, ...);

Each return path in the method should assign to _path. The _path ??= pattern used on Unix (benign race for reference types) is the established caching convention in this codebase.

(Flagged by both models)

⚠️ Test `NonInheritedFileHandle_IsNotAvailableInChildProcess` — handle closed before child spawn

Location: ProcessHandlesTests.cs:544-610

The test creates a FileStream from fileHandle on line 559:

using (FileStream parentFs = new(fileHandle, FileAccess.ReadWrite))
{
    Assert.Equal(path, parentFs.Name);
}

FileStream(SafeFileHandle, FileAccess) does not have a leaveOpen parameter — when disposed, OSFileStreamStrategy.Dispose calls _fileHandle.Dispose(), which closes the OS handle. By the time RemoteExecutor.Invoke spawns the child process (line 569), the handle identified by rawHandle is already closed in the parent.

This makes the test pass trivially — the handle isn't available in the child because it was closed before the child was spawned, not because InheritedHandles = [] prevented inheritance. The test would pass identically without options.StartInfo.InheritedHandles = [].

Suggested fix: Either move the FileStream assertion to not dispose the handle before the child spawn, or restructure so the handle remains open when the child is forked. For example, verify parentFs.Name using a separate handle, or test Name separately from the inheritance test.

💡 Code duplication with `FileSystem.Windows.cs`

Location: SafeFileHandle.Windows.cs:466-530 vs FileSystem.Windows.cs:707-750

Both methods implement nearly identical logic: rent buffer from ArrayPool<char>, call GetFinalPathNameByHandle, grow buffer if needed, strip extended DOS prefix, return to pool. The new code is actually an improvement over the existing version (proper UNC handling), but the duplication could be consolidated into a shared helper in a follow-up PR.

✅ Native implementation — solid

The native C code in pal_io.c is well-structured:

Linux: correctly uses readlink on /proc/self/fd/<fd> with proper bounds checking (count >= bufferSize → ENAMETOOLONG) and null termination
macOS/BSD: uses standard fcntl(F_GETPATH)
Fallback: returns -1 with errno = ENOTSUP
CMake configuration correctly detects F_GETPATH availability

The bufferSize is technically ignored on the F_GETPATH path (since F_GETPATH writes up to MAXPATHLEN = 1024 bytes), but the sole caller always passes 4096, which is well above MAXPATHLEN. This is safe in practice.

✅ Unix managed implementation — correct caching pattern

The _path ??= pattern on Unix is the standard benign-race caching convention for reference types in dotnet/runtime. Reference type writes are atomic on all .NET platforms, so the worst case is redundant work (multiple threads resolve the path simultaneously), not corruption. This is consistent with how the codebase handles similar lazy initialization (e.g., see the comment on line 40 of SafeFileHandle.Unix.cs explaining why bool? isn't used for thread safety, but NullableBool is — reference types don't have that issue).

✅ Windows prefix stripping — thorough

The UNC path handling (\\?\UNC\server\share → \\server\share) is an improvement over the existing pattern in FileSystem.Windows.cs which only handles the simpler \\?\ prefix case. The use of PathInternal.IsDeviceUNC, UncPathPrefix, and UncExtendedPrefixLength constants is correct.

✅ Tests for path resolution — good coverage

The Name_WhenOpenedWithPath_ReturnsPath and Name_WhenOpenedFromHandle_ReturnsPath tests in GetFileType.cs directly exercise the new functionality. The second test creates a SafeFileHandle from a raw IntPtr (no cached _path), which triggers the OS-level resolution path. Platform skip attributes for Browser/WASI are appropriate.

Generated by Code Review for issue #126629 · ◷

adamsitnik

@copilot address my feedback

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

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

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

src/native/libs/System.Native/pal_io.c

… guard, return SR.IO_UnknownFileName on Unix failure Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c0bb5bd4-b358-456c-a074-116c8a17d415 Co-authored-by: adamsitnik <[email protected]>

Copilot · 2026-04-08T12:03:30Z

@copilot address my feedback

Addressed in b97f003:

Path => _path ??= GetPath() — caching centralized in Path property
Unix/Windows GetPath() — just fetch the value, no caching
GetFilePathFromHandle returns non-nullable string; returns SR.IO_UnknownFileName on failure instead of null
pal_io.c F_GETPATH block: (void)bufferSize replaced with a MAXPATHLEN guard

adamsitnik · 2026-04-08T12:21:25Z

@jkotas I've marked it as "NO REVIEW" as I expect it's going to take me a while to get to the point where I am happy about path normalization myself

jkotas · 2026-04-08T15:03:34Z

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

+                    buffer = ArrayPool<char>.Shared.Rent((int)result);
+                    ArrayPool<char>.Shared.Return(toReturn);
+
+                    result = GetFinalPathNameByHandleHelper(buffer);


Is it guaranteed that the path cannot change between the two calls? This pattern is often wrapped in a loop (see e.g.

runtime/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs

Lines 18 to 21 in 2eb5f4c

while ((length = Interop.Kernel32.GetEnvironmentVariable(variable, ref builder.GetPinnableReference(), (uint)builder.Capacity)) > builder.Capacity)

{

builder.EnsureCapacity((int)length);

}

)

Initial plan

da0bff0

Copilot AI assigned Copilot and adamsitnik Apr 8, 2026

Copilot AI review requested due to automatic review settings April 8, 2026 06:08

Copilot started work on behalf of adamsitnik April 8, 2026 06:08 View session

Copilot AI linked an issue Apr 8, 2026 that may be closed by this pull request

Make FileStream.Name work for FileStream instances created out of handle #126628

Open

Copilot AI review requested due to automatic review settings April 8, 2026 06:08

jkotas added the area-System.IO label Apr 8, 2026

Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 06:50

Copilot AI changed the title ~~[WIP] Fix FileStream.Name for instances created out of handle~~ Implement SafeFileHandle.Name to resolve file paths from OS handles Apr 8, 2026

Copilot AI requested a review from adamsitnik April 8, 2026 07:04

Copilot finished work on behalf of adamsitnik April 8, 2026 07:04

adamsitnik requested changes Apr 8, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik April 8, 2026 07:15 View session

Address PR review feedback: make API internal, fix UNC handling, use …

b600d32

…SafeFileHandle in interop Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d6f0695b-ba5d-44a8-aa39-c07ea398bcd0 Co-authored-by: adamsitnik <[email protected]>

Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 07:44

Fix null terminator search to be bounded by PathMaxSize

635eed0

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/d6f0695b-ba5d-44a8-aa39-c07ea398bcd0 Co-authored-by: adamsitnik <[email protected]>

Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 07:47

Copilot AI changed the title ~~Implement SafeFileHandle.Name to resolve file paths from OS handles~~ Make FileStream.Name work for FileStream instances created out of handle Apr 8, 2026

Copilot finished work on behalf of adamsitnik April 8, 2026 07:48

Copilot AI requested a review from adamsitnik April 8, 2026 07:48

adamsitnik reviewed Apr 8, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik April 8, 2026 08:38 View session

Address second round of PR review feedback

9a7fd3e

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/bb17e842-35f7-4bae-8d99-bf98e21541b2 Co-authored-by: adamsitnik <[email protected]>

Copilot AI review requested due to automatic review settings April 8, 2026 08:54

adamsitnik reviewed Apr 8, 2026

View reviewed changes

src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs Outdated Show resolved Hide resolved

adamsitnik reviewed Apr 8, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik April 8, 2026 10:21 View session

Fix build failure: remove leaveOpen param, save rawHandle before File…

a33d3f9

…Stream, remove redundant SkipOnPlatform Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/45057e03-9697-4819-a2c0-72b02fddccf6 Co-authored-by: adamsitnik <[email protected]>

Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 10:32

Copilot finished work on behalf of adamsitnik April 8, 2026 10:34

Copilot AI requested a review from adamsitnik April 8, 2026 10:34

adamsitnik reviewed Apr 8, 2026

View reviewed changes

src/native/libs/System.Native/pal_io.c Show resolved Hide resolved

fix OSX build

f0d33c3

Copilot AI review requested due to automatic review settings April 8, 2026 11:13

adamsitnik added the NO-REVIEW Experimental/testing PR, do NOT review it label Apr 8, 2026

Copilot started reviewing on behalf of adamsitnik April 8, 2026 11:14 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

src/native/libs/System.Native/pal_io.c Outdated Show resolved Hide resolved

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs Outdated Show resolved Hide resolved

adamsitnik reviewed Apr 8, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik April 8, 2026 11:41 View session

Address reviewer feedback: centralize caching in Path, add MAXPATHLEN…

b97f003

… guard, return SR.IO_UnknownFileName on Unix failure Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c0bb5bd4-b358-456c-a074-116c8a17d415 Co-authored-by: adamsitnik <[email protected]>

Copilot finished work on behalf of adamsitnik April 8, 2026 12:07

Copilot AI requested a review from adamsitnik April 8, 2026 12:08

jkotas reviewed Apr 8, 2026

View reviewed changes

		// GetFinalPathNameByHandle always returns with extended DOS prefix (\\?\).
		// Trim the prefix to keep the result consistent with the path stored in _path.

	while ((length = Interop.Kernel32.GetEnvironmentVariable(variable, ref builder.GetPinnableReference(), (uint)builder.Capacity)) > builder.Capacity)
	{
	builder.EnsureCapacity((int)length);
	}

Conversation

Copilot AI commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Native (System.Native)

Managed (System.Private.CoreLib)

Tests

Example

Uh oh!

dotnet-policy-service bot commented Apr 8, 2026

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamsitnik Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Apr 8, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 8, 2026

🤖 Copilot Code Review — PR #126629

Holistic Assessment

Detailed Findings

⚠️ Windows GetPath() does not cache the resolved path — performance bug

⚠️ Test NonInheritedFileHandle_IsNotAvailableInChildProcess — handle closed before child spawn

💡 Code duplication with FileSystem.Windows.cs

✅ Native implementation — solid

✅ Unix managed implementation — correct caching pattern

✅ Windows prefix stripping — thorough

✅ Tests for path resolution — good coverage

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Apr 8, 2026

Uh oh!

adamsitnik commented Apr 8, 2026

Uh oh!

jkotas Apr 8, 2026

Copilot AI commented Apr 8, 2026 •

edited

Loading

Native (`System.Native`)

Managed (`System.Private.CoreLib`)

⚠️ Windows `GetPath()` does not cache the resolved path — performance bug

⚠️ Test `NonInheritedFileHandle_IsNotAvailableInChildProcess` — handle closed before child spawn

💡 Code duplication with `FileSystem.Windows.cs`