Fix missing error handling in SafeFileHandle.Type on Windows

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated

[Copilot]

Description

SafeFileHandle.Type silently returns FileHandleType.Unknown on Windows when called with an invalid handle, instead of throwing an exception. This is because GetFileType returns FILE_TYPE_UNKNOWN (0) for both "type is genuinely unknown" and "an error occurred" — the distinction requires checking GetLastError.

Fix: In GetFileTypeCore(), handle FILE_TYPE_UNKNOWN before the switch expression. Check Marshal.GetLastPInvokeError() and throw a Win32 exception if the error code is non-zero; only return FileHandleType.Unknown when GetLastError is ERROR_SUCCESS.

// Before: always returned Unknown for invalid handles
_ =>FileHandleType.Unknown// After: throws for invalid handles, returns Unknown only when genuinely unknownif(kernelFileType==FILE_TYPE_UNKNOWN){interror=Marshal.GetLastPInvokeError();if(error!=Interop.Errors.ERROR_SUCCESS)
throw Win32Marshal.GetExceptionForWin32Error(error,Path);returnFileHandleType.Unknown;}

Changes

• SafeFileHandle.Windows.cs — extract FILE_TYPE_UNKNOWN check before switch in GetFileTypeCore(), add GetLastError check to throw on error

[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 add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file with File.OpenHandle, then create a copy of the handle then close the original handle and then call the Type property for the copy

[Copilot]

@copilot add a test that creates an invalid SafeFileHandle with a positive handle value and ensures that Type throws the same exception on every platform. One idea would be to open a regular file ...

Added GetFileType_InvalidHandle_Throws in GetFileType.cs (commit 275074d): opens a file, wraps the native handle value in a non-owning SafeFileHandle, disposes the original (invalidating the OS handle), and then asserts .Type throws via AssertExtensions.ThrowsAny<IOException, UnauthorizedAccessException> — covering both Windows (IOException from ERROR_INVALID_HANDLE) and Unix (UnauthorizedAccessException from EBADF).

[Copilot]

Pull request overview

This PR fixes Windows-specific behavior in SafeFileHandle.Type so that invalid handles no longer silently report FileHandleType.Unknown; instead, the implementation checks GetLastError when GetFileType returns FILE_TYPE_UNKNOWN and throws a Win32-based exception when an actual error occurred.

Changes:

• Add a pre-switch FILE_TYPE_UNKNOWN branch in GetFileTypeCore() to distinguish “unknown type” from “error” via Marshal.GetLastPInvokeError().
• Throw Win32Marshal.GetExceptionForWin32Error(...) when GetLastError != ERROR_SUCCESS; otherwise return FileHandleType.Unknown.

[adamsitnik]

This is not reliable. The handle could have been reused.

It is complicated to test this reliably for the same reasons discussed in #126318. I do not think we need a test for this.

@jkotas what if we put this test in a dedicated class and annotate it with following attribute that ensures other tests are not running in parallel?

runtime/src/libraries/Common/tests/TestUtilities/System/DisableParallelization.cs

Lines 8 to 11 in 68b538c

	// The collection definitions must be in the same assembly as the test that uses them.
	// So please use "Compile Include" in the project file to include this class.
	[CollectionDefinition(nameof(DisableParallelization),DisableParallelization=true)]
	publicclassDisableParallelization{}

Can runtime still open some new handles/descriptors in the meantime?

[jkotas]

Can runtime still open some new handles/descriptors in the meantime?

Yes, you have GC threads, finalizer thread, tiered compilation thread, debugger thread, diagnostic server thread, and several PAL threads all running in the background doing stuff that you do not have a control over.

[adamsitnik]

LGTM, thank you @jkotas !

[adamsitnik]

/ba-g the failures are unrelated