From 3368f4d6f4fcb6f9f1ed0513d4827f41626716f2 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Wed, 21 Oct 2026 14:18:02 -0700 Subject: [PATCH 1/2] Fix OverflowException for sign-extended handles in SafeHandle conversions #1844 converted typedef handles to IntPtr with an explicit `(IntPtr)ptr` cast, and leased SafeHandles back to `(void*)nint`. Under CheckForOverflowUnderflow those casts throw for handles with the high bit set: INVALID_HANDLE_VALUE, pseudo-handles such as GetCurrentProcess(), and sign-extended GDI/USER handles. Before #1844 the conversion went through the typedef's own IntPtr operator and never threw. Emit both conversions in an unchecked context. Add sandbox tests that call real APIs (pseudo-handles, failure handles, and thousands of GDI bitmaps) through the SafeHandle overloads and leases. Add HandleOwnershipTests, which call real APIs to verify whether each returned SafeHandle owns its handle. Tests for the annotations added by microsoft/win32metadata#2380 are skipped until CsWin32 consumes a metadata package that contains them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61981950-206c-4b2c-844e-c87ed4af9432 --- .../Generator.FriendlyOverloads.cs | 7 +- .../Generator.SafeHandleLease.cs | 3 +- .../HandleOwnershipTests.cs | 466 ++++++++++++++++++ .../GenerationSandbox.Tests/NativeMethods.txt | 52 ++ .../SignExtendedHandleTests.cs | 152 ++++++ 5 files changed, 677 insertions(+), 3 deletions(-) create mode 100644 test/GenerationSandbox.Tests/HandleOwnershipTests.cs create mode 100644 test/GenerationSandbox.Tests/SignExtendedHandleTests.cs diff --git a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs index 53b38799..7621afe7 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs @@ -92,14 +92,17 @@ private bool HasAmbiguousCrefParameterTypes(ParameterListSyntax parameterList) private ExpressionSyntax GetIntPtrFromTypeDef(ExpressionSyntax typedefValue, TypeHandleInfo typeDefTypeInfo) { ExpressionSyntax intPtrValue = typedefValue; + + // Pointer-to-nint conversions must be unchecked: handles such as INVALID_HANDLE_VALUE, pseudo-handles, + // and sign-extended GDI/USER handles have the high bit set and would throw under CheckForOverflowUnderflow. if (typeDefTypeInfo is PointerTypeHandleInfo) { - return CastExpression(IntPtrTypeSyntax, typedefValue); + return UncheckedExpression(CastExpression(IntPtrTypeSyntax, typedefValue)); } if (this.TryGetTypeDefFieldType(typeDefTypeInfo, out TypeHandleInfo? backingType) && backingType is PointerTypeHandleInfo) { - return CastExpression(IntPtrTypeSyntax, MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, typedefValue, IdentifierName("Value"))); + return UncheckedExpression(CastExpression(IntPtrTypeSyntax, MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, typedefValue, IdentifierName("Value")))); } if (this.TryGetTypeDefFieldType(typeDefTypeInfo, out TypeHandleInfo? returnTypeField) && returnTypeField is PrimitiveTypeHandleInfo primitiveReturnField) diff --git a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs index 8d2b6a14..7a4f77d6 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs @@ -81,7 +81,8 @@ private ExpressionSyntax GetNativeValueFromHandle(ExpressionSyntax handle, TypeH TypeSyntax fieldType = backingType.ToTypeSyntax(this.fieldTypeSettings, GeneratingElement.HelperClassMember, null).Type; return backingType switch { - PointerTypeHandleInfo => CastExpression(fieldType, handle), + // Handles with the high bit set (e.g. sign-extended GDI handles) must not throw under CheckForOverflowUnderflow. + PointerTypeHandleInfo => UncheckedExpression(CastExpression(fieldType, handle)), PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.IntPtr } => handle, PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.UIntPtr } => UncheckedExpression(CastExpression(fieldType, CastExpression(IdentifierName("nint"), handle))), PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.UInt32 } => CheckedExpression(CastExpression(fieldType, UncheckedExpression(CastExpression(IdentifierName("nuint"), CastExpression(IdentifierName("nint"), handle))))), diff --git a/test/GenerationSandbox.Tests/HandleOwnershipTests.cs b/test/GenerationSandbox.Tests/HandleOwnershipTests.cs new file mode 100644 index 00000000..b1ae7329 --- /dev/null +++ b/test/GenerationSandbox.Tests/HandleOwnershipTests.cs @@ -0,0 +1,466 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Reflection; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.Graphics.Gdi; +using Windows.Win32.Security; +using Windows.Win32.System.Console; +using Windows.Win32.System.Memory; +using Windows.Win32.System.Ole; +using Windows.Win32.UI.WindowsAndMessaging; + +/// +/// Calls real Win32 APIs through their SafeHandle-returning friendly overloads and verifies whether the returned +/// SafeHandle owns (will release) the native handle. +/// +/// +/// +/// Ownership comes from win32metadata: [DoNotRelease], [RAIIFree], or the default release function of +/// the handle type. A wrong annotation makes disposing the SafeHandle release a resource the caller was only lent, +/// such as deleting a bitmap that is still in use, unloading the user's keyboard layout, closing the process's window +/// station, or calling CloseHandle on a GDI handle value (which can close an unrelated kernel handle). +/// These tests exist to catch metadata regressions, so they call the actual APIs rather than synthetic metadata. +/// +/// +/// Where releasing a wrongly owned handle has an observable effect on a resource the test created, the test disposes +/// the SafeHandle and checks that the resource survives. Otherwise, it inspects the SafeHandle's ownership directly, +/// then marks it invalid so that it can never release the handle, even when the assertion fails. +/// +/// +[Trait("WindowsOnly", "true")] +public unsafe class HandleOwnershipTests +{ + /// + /// The skip reason for tests that verify annotations added by a win32metadata version that CsWin32 doesn't consume yet. + /// + private const string RequiresMetadataUpdate = "Requires the annotations from https://github.com/microsoft/win32metadata/pull/2380"; + + /// + /// A string resource name that user32 interprets as the integer resource 32512 (IDC_ARROW and IDI_APPLICATION). + /// + private const string SystemResource32512 = "#32512"; + + /// + /// The private field behind the ownership flag passed to the constructor. + /// There is no public API to read it. The name is the same on .NET Framework and .NET. + /// + private static readonly FieldInfo OwnsHandleField = typeof(SafeHandle).GetField("_ownsHandle", BindingFlags.Instance | BindingFlags.NonPublic) + ?? throw new MissingFieldException(nameof(SafeHandle), "_ownsHandle"); + + [Fact] + public void GetCurrentProcess_IsBorrowed() => AssertBorrowed(PInvoke.GetCurrentProcess_SafeHandle()); + + [Fact] + public void GetStdHandle_IsBorrowed() => AssertBorrowed(PInvoke.GetStdHandle_SafeHandle(STD_HANDLE.STD_OUTPUT_HANDLE)); + + [Fact] + public void GetProcessHeap_IsBorrowed() => AssertBorrowed(PInvoke.GetProcessHeap_SafeHandle()); + + [Fact] + public void GetModuleHandle_IsBorrowed() + { + FreeLibrarySafeHandle kernel32 = PInvoke.GetModuleHandle("kernel32.dll"); + Assert.False(kernel32.IsInvalid); + AssertBorrowed(kernel32); + } + + [Fact] + public void LoadCursor_SharedCursor_IsBorrowed() + { + DestroyCursorSafeHandle arrow = PInvoke.LoadCursor(null, SystemResource32512); + Assert.False(arrow.IsInvalid); + AssertBorrowed(arrow); + } + + // The following APIs return handles the caller must release. They guard against over-applying [DoNotRelease]. + [Fact] + public void CreateRectRgn_IsOwned() + { + using DeleteObjectHRGNSafeHandle region = PInvoke.CreateRectRgn_SafeHandle(0, 0, 1, 1); + Assert.False(region.IsInvalid); + AssertOwned(region); + } + + [Fact] + public void CreatePopupMenu_IsOwned() + { + using DestroyMenuSafeHandle menu = PInvoke.CreatePopupMenu_SafeHandle(); + Assert.False(menu.IsInvalid); + AssertOwned(menu); + } + + [Fact] + public void OpenProcessToken_IsOwned() + { + using SafeFileHandle process = PInvoke.GetCurrentProcess_SafeHandle(); + Assert.True(PInvoke.OpenProcessToken(process, TOKEN_ACCESS_MASK.TOKEN_QUERY, out SafeFileHandle token)); + using (token) + { + AssertOwned(token); + } + } + + [Fact] + public void FindFirstFile_IsOwned() + { + using FindCloseSafeHandle find = PInvoke.FindFirstFile(Path.Combine(Environment.SystemDirectory, "kernel32.dll"), out _); + Assert.False(find.IsInvalid); + AssertOwned(find); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetCurrentThread_IsBorrowed() => AssertBorrowed(PInvoke.GetCurrentThread_SafeHandle()); + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetProcessWindowStation_IsBorrowed() + { + CloseWindowStationSafeHandle windowStation = PInvoke.GetProcessWindowStation_SafeHandle(); + Assert.False(windowStation.IsInvalid); + AssertBorrowed(windowStation); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetThreadDesktop_IsBorrowed() + { + CloseDesktopSafeHandle desktop = PInvoke.GetThreadDesktop_SafeHandle(PInvoke.GetCurrentThreadId()); + Assert.False(desktop.IsInvalid); + AssertBorrowed(desktop); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetKeyboardLayout_IsBorrowed() + { + // Releasing this would unload the user's active keyboard layout. + AssertBorrowed(PInvoke.GetKeyboardLayout_SafeHandle(0)); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetStockObject_IsBorrowed() + { + DeleteObjectSafeHandle brush = PInvoke.GetStockObject_SafeHandle(GET_STOCK_OBJECT_FLAGS.WHITE_BRUSH); + Assert.Equal((uint)OBJ_TYPE.OBJ_BRUSH, PInvoke.GetObjectType(brush)); + AssertBorrowed(brush); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetSysColorBrush_IsBorrowed() + { + DeleteObjectHBRUSHSafeHandle brush = PInvoke.GetSysColorBrush_SafeHandle(SYS_COLOR_INDEX.COLOR_WINDOW); + Assert.False(brush.IsInvalid); + AssertBorrowed(brush); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void SelectObject_PreviousObjectIsBorrowed() + { + WithMemoryDC(dc => + { + using DeleteObjectHBITMAPSafeHandle first = PInvoke.CreateCompatibleBitmap_SafeHandle(dc, 1, 1); + using DeleteObjectHBITMAPSafeHandle second = PInvoke.CreateCompatibleBitmap_SafeHandle(dc, 1, 1); + using DeleteObjectSafeHandle original = PInvoke.SelectObject(dc, first); + using (DeleteObjectSafeHandle previous = PInvoke.SelectObject(dc, second)) + { + Assert.Equal(first.DangerousGetHandle(), previous.DangerousGetHandle()); + } + + // Disposing SelectObject's return value must not delete the bitmap, which this test still owns. + BITMAP info; + Assert.Equal(sizeof(BITMAP), PInvoke.GetObject((HGDIOBJ)first.DangerousGetHandle(), sizeof(BITMAP), &info)); + using DeleteObjectSafeHandle restored = PInvoke.SelectObject(dc, original); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetCurrentObject_IsBorrowed() + { + WithMemoryDC(dc => + { + DeleteObjectSafeHandle bitmap = PInvoke.GetCurrentObject_SafeHandle(dc, OBJ_TYPE.OBJ_BITMAP); + Assert.Equal((uint)OBJ_TYPE.OBJ_BITMAP, PInvoke.GetObjectType(bitmap)); + AssertBorrowed(bitmap); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void SelectPalette_PreviousPaletteIsBorrowed() + { + WithMemoryDC(dc => + { + using DeleteObjectSafeHandle defaultPalette = PInvoke.GetStockObject_SafeHandle(GET_STOCK_OBJECT_FLAGS.DEFAULT_PALETTE); + DeleteObjectHPALETTESafeHandle previous = PInvoke.SelectPalette(dc, defaultPalette, false); + Assert.False(previous.IsInvalid); + AssertBorrowed(previous); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetColorSpace_And_SetColorSpace_AreBorrowed() + { + WithMemoryDC(dc => + { + DeleteColorSpaceSafeHandle current = PInvoke.GetColorSpace_SafeHandle(dc); + Assert.False(current.IsInvalid); + DeleteColorSpaceSafeHandle previous = PInvoke.SetColorSpace(dc, current); + Assert.Equal(current.DangerousGetHandle(), previous.DangerousGetHandle()); + AssertBorrowed(previous); + AssertBorrowed(current); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void CopyImage_ReturnedOriginalIsBorrowed() + { + DestroyIconSafeHandle icon = PInvoke.LoadIcon(null, SystemResource32512); + try + { + Assert.False(icon.IsInvalid); + SafeFileHandle copy = PInvoke.CopyImage(icon, GDI_IMAGE_TYPE.IMAGE_ICON, 0, 0, IMAGE_FLAGS.LR_COPYRETURNORG); + + // LR_COPYRETURNORG returns the original shared icon, which the system owns. + // For any other result the right release function depends on the image type, so CloseHandle is never right. + Assert.Equal(icon.DangerousGetHandle(), copy.DangerousGetHandle()); + AssertBorrowed(copy); + } + finally + { + // LoadIcon's shared icon must never be destroyed, whatever the metadata says about it. + icon.SetHandleAsInvalid(); + } + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void LoadIcon_SharedIcon_IsBorrowed() + { + DestroyIconSafeHandle icon = PInvoke.LoadIcon(null, SystemResource32512); + Assert.False(icon.IsInvalid); + AssertBorrowed(icon); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void LoadImage_SharedIcon_IsBorrowed() + { + using FreeLibrarySafeHandle user32 = PInvoke.LoadLibrary("user32.dll"); + SafeFileHandle icon = PInvoke.LoadImage(user32, "#100", GDI_IMAGE_TYPE.IMAGE_ICON, 0, 0, IMAGE_FLAGS.LR_DEFAULTSIZE | IMAGE_FLAGS.LR_SHARED); + Assert.False(icon.IsInvalid, $"LoadImage failed: {Marshal.GetLastWin32Error()}"); + AssertBorrowed(icon); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetCursor_And_SetCursor_AreBorrowed() + { + using DestroyCursorSafeHandle arrow = PInvoke.LoadCursor(null, SystemResource32512); + DestroyCursorSafeHandle previous = PInvoke.SetCursor(arrow); + DestroyCursorSafeHandle current = PInvoke.GetCursor_SafeHandle(); + Assert.Equal(arrow.DangerousGetHandle(), current.DangerousGetHandle()); + DestroyCursorSafeHandle restored = PInvoke.SetCursor(previous); + Assert.Equal(arrow.DangerousGetHandle(), restored.DangerousGetHandle()); + + AssertBorrowed(previous); + AssertBorrowed(current); + AssertBorrowed(restored); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetSubMenu_IsBorrowed() + { + using DestroyMenuSafeHandle parent = PInvoke.CreatePopupMenu_SafeHandle(); + using DestroyMenuSafeHandle child = PInvoke.CreatePopupMenu_SafeHandle(); + HMENU childValue = (HMENU)child.DangerousGetHandle(); + Assert.True(PInvoke.AppendMenu(parent, MENU_ITEM_FLAGS.MF_POPUP, (nuint)childValue.Value, "Child")); + child.SetHandleAsInvalid(); // The parent now owns the submenu and destroys it along with itself. + + using (DestroyMenuSafeHandle subMenu = PInvoke.GetSubMenu(parent, 0)) + { + Assert.True(subMenu.DangerousValue == childValue); + } + + // Disposing GetSubMenu's return value must not destroy the submenu, which still belongs to its parent. + Assert.True(PInvoke.IsMenu(childValue)); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetMenu_IsBorrowed() + { + WithWindow(hwnd => + { + using DestroyMenuSafeHandle menu = PInvoke.CreateMenu_SafeHandle(); + HMENU menuValue = (HMENU)menu.DangerousGetHandle(); + Assert.True(PInvoke.SetMenu(hwnd, menu)); + menu.SetHandleAsInvalid(); // The window now owns the menu and destroys it along with itself. + + using (DestroyMenuSafeHandle windowMenu = PInvoke.GetMenu_SafeHandle(hwnd)) + { + Assert.True(windowMenu.DangerousValue == menuValue); + } + + // Disposing GetMenu's return value must not destroy the window's menu. + Assert.True(PInvoke.IsMenu(menuValue)); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetSystemMenu_IsBorrowed() + { + WithWindow(hwnd => + { + HMENU systemMenuValue; + using (DestroyMenuSafeHandle systemMenu = PInvoke.GetSystemMenu_SafeHandle(hwnd, false)) + { + Assert.False(systemMenu.IsInvalid); + systemMenuValue = (HMENU)systemMenu.DangerousGetHandle(); + } + + // Disposing GetSystemMenu's return value must not destroy the window's system menu. + Assert.True(PInvoke.IsMenu(systemMenuValue)); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetWindowTheme_IsBorrowed() + { + WithWindow(hwnd => + { + using CloseThemeDataSafeHandle theme = PInvoke.OpenThemeData(hwnd, "BUTTON"); + + // GetWindowTheme returns the handle OpenThemeData most recently returned for the window, which the caller + // of OpenThemeData owns. Both are null when visual styles are off. + CloseThemeDataSafeHandle windowTheme = PInvoke.GetWindowTheme_SafeHandle(hwnd); + Assert.Equal(theme.DangerousGetHandle(), windowTheme.DangerousGetHandle()); + AssertBorrowed(windowTheme); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void GetProp_IsBorrowed_And_RemoveProp_IsNotAutoReleased() + { + WithWindow(hwnd => + { + const string PropertyName = "CsWin32.HandleOwnershipTests"; + using DeleteObjectHRGNSafeHandle data = PInvoke.CreateRectRgn_SafeHandle(0, 0, 1, 1); + Assert.True(PInvoke.SetProp(hwnd, PropertyName, data)); + + SafeFileHandle property = PInvoke.GetProp(hwnd, PropertyName); + Assert.Equal(data.DangerousGetHandle(), property.DangerousGetHandle()); + AssertBorrowed(property); + + // RemoveProp hands the data back to the caller, who must free it with the function that matches its type + // (DeleteObject, through `data`, in this case). CloseHandle is never correct. + SafeFileHandle removed = PInvoke.RemoveProp(hwnd, PropertyName); + Assert.Equal(data.DangerousGetHandle(), removed.DangerousGetHandle()); + AssertNotAutoReleased(removed); + }); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void FindExecutable_IsBorrowed() + { + Span executable = stackalloc char[260]; + FreeLibraryHINSTANCESafeHandle result = PInvoke.FindExecutable(Path.Combine(Environment.SystemDirectory, "cmd.exe"), null, executable); + + // FindExecutable returns an error code (greater than 32 on success), not a module handle. + Assert.True(result.DangerousGetHandle().ToInt64() > 32, $"FindExecutable failed: {result.DangerousGetHandle()}"); + AssertBorrowed(result); + } + + [Fact(Skip = RequiresMetadataUpdate)] + public void OleDuplicateData_IsNotAutoReleased() + { + using GlobalFreeSafeHandle source = PInvoke.GlobalAlloc_SafeHandle(GLOBAL_ALLOC_FLAGS.GMEM_MOVEABLE, 16); + Assert.False(source.IsInvalid); + + SafeFileHandle copy = PInvoke.OleDuplicateData(source, CLIPBOARD_FORMAT.CF_TEXT, GLOBAL_ALLOC_FLAGS.GMEM_MOVEABLE); + IntPtr copyValue = copy.DangerousGetHandle(); + try + { + Assert.NotEqual(IntPtr.Zero, copyValue); + AssertNotAutoReleased(copy); + } + finally + { + // The caller owns the copy, but must free it with the function that matches its format, not CloseHandle. + PInvoke.GlobalFree((HGLOBAL)copyValue); + } + } + + private static bool OwnsHandle(SafeHandle handle) => (bool)OwnsHandleField.GetValue(handle)!; + + /// + /// Asserts that will not release the native handle it wraps, then marks it invalid + /// so that it cannot release the handle even if the assertion fails. + /// + /// The SafeHandle returned by the API under test. + private static void AssertBorrowed(SafeHandle handle) + => AssertDoesNotOwn(handle, $"{handle.GetType().Name} owns a handle that the API only lends to the caller."); + + /// + /// Asserts that will not release the native handle it wraps, then marks it invalid + /// so that it cannot release the handle even if the assertion fails. + /// + /// The SafeHandle returned by the API under test. + /// + /// Use this for handles the caller owns but must release with a function that depends on what the handle refers to, + /// so no single release function in the metadata can be right. + /// + private static void AssertNotAutoReleased(SafeHandle handle) + => AssertDoesNotOwn(handle, $"{handle.GetType().Name} owns a handle whose release function depends on what the handle refers to."); + + private static void AssertDoesNotOwn(SafeHandle handle, string message) + { + try + { + Assert.False(OwnsHandle(handle), message); + } + finally + { + handle.SetHandleAsInvalid(); + } + } + + /// + /// Asserts that will release the native handle it wraps. + /// + /// The SafeHandle returned by the API under test. + private static void AssertOwned(SafeHandle handle) + => Assert.True(OwnsHandle(handle), $"{handle.GetType().Name} does not own a handle that the caller must release."); + + /// + /// Runs with a memory device context, then deletes it. + /// + /// The test to run. + private static void WithMemoryDC(Action test) + { + HDC dc = PInvoke.CreateCompatibleDC(default); + Assert.False(dc.IsNull); + try + { + test(dc); + } + finally + { + PInvoke.DeleteDC(dc); + } + } + + /// + /// Runs with a hidden top-level window, which (unlike a message-only window) can have + /// a menu and a system menu, then destroys it. + /// + /// The test to run. + private static void WithWindow(Action test) + { + HWND hwnd = PInvoke.CreateWindowEx(0, "STATIC", nameof(HandleOwnershipTests), WINDOW_STYLE.WS_OVERLAPPEDWINDOW, 0, 0, 100, 100, default, null, null, null); + Assert.False(hwnd.IsNull, $"CreateWindowEx failed: {Marshal.GetLastWin32Error()}"); + try + { + test(hwnd); + } + finally + { + PInvoke.DestroyWindow(hwnd); + } + } +} diff --git a/test/GenerationSandbox.Tests/NativeMethods.txt b/test/GenerationSandbox.Tests/NativeMethods.txt index 84a6cba0..9c21d08b 100644 --- a/test/GenerationSandbox.Tests/NativeMethods.txt +++ b/test/GenerationSandbox.Tests/NativeMethods.txt @@ -111,3 +111,55 @@ INetFwMgr NetFwMgr IEnumVARIANT INetFwAuthorizedApplication +AppendMenu +CopyImage +CreateCompatibleBitmap +CreateCompatibleDC +CreatePopupMenu +CreateRectRgn +FindExecutable +FindFirstFile +GetCurrentObject +GetCurrentProcess +GetCurrentThread +GetCurrentThreadId +GetCursor +GetKeyboardLayout +GetObjectType +GetProcessHeap +GetProcessId +GetProcessWindowStation +GetStdHandle +GetStockObject +GetSubMenu +GetSysColorBrush +GetThreadDesktop +GetThreadId +IsMenu +LoadCursor +LoadIcon +LoadImage +OpenProcessToken +SelectObject +CloseThemeData +CreateMenu +CreateWindowEx +DeleteDC +DestroyWindow +GetColorSpace +GetMenu +GetProp +GetSystemMenu +GetWindowTheme +OpenThemeData +RemoveProp +SelectPalette +SetColorSpace +SetCursor +SetMenu +SetProp +GlobalAlloc +GlobalFree +OleDuplicateData +GetObject +BITMAP diff --git a/test/GenerationSandbox.Tests/SignExtendedHandleTests.cs b/test/GenerationSandbox.Tests/SignExtendedHandleTests.cs new file mode 100644 index 00000000..6f2fff6d --- /dev/null +++ b/test/GenerationSandbox.Tests/SignExtendedHandleTests.cs @@ -0,0 +1,152 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.Graphics.Gdi; +using Windows.Win32.Security; +using Windows.Win32.Storage.FileSystem; + +/// +/// Calls real Win32 APIs whose handle values have the high bit set, through the SafeHandle-returning +/// friendly overloads, in a project that has CheckForOverflowUnderflow enabled. +/// +/// +/// +/// Native handles are pointer-sized values, not addresses. Several common ones have the high bit set when viewed +/// as a native integer: INVALID_HANDLE_VALUE (-1), the GetCurrentProcess (-1) and +/// GetCurrentThread (-2) pseudo-handles, and 32-bit GDI/USER handles that are sign-extended to 64 bits. +/// Converting such a value between void* and with a checked C# cast throws +/// , so all generated conversions must be unchecked. +/// See https://github.com/microsoft/CsWin32/issues/1412. +/// +/// +[Trait("WindowsOnly", "true")] +public class SignExtendedHandleTests +{ + /// + /// GDI handle values are recycled with an incrementing uniqueness counter in their upper bits, so creating and + /// deleting objects in a loop yields a sign-extended handle within a few hundred iterations. + /// This cap only bounds the loop if that assumption ever stops holding. + /// + private const int MaxGdiAttempts = 10_000; + + [Fact] + public void GetCurrentProcess_SafeHandle_PseudoHandleIsUsable() + { + using SafeFileHandle process = PInvoke.GetCurrentProcess_SafeHandle(); + + Assert.Equal(new IntPtr(-1), process.DangerousGetHandle()); + Assert.Equal((uint)Process.GetCurrentProcess().Id, PInvoke.GetProcessId(process)); + + Assert.True(PInvoke.OpenProcessToken(process, TOKEN_ACCESS_MASK.TOKEN_QUERY, out SafeFileHandle token), $"OpenProcessToken failed: {Marshal.GetLastWin32Error()}"); + using (token) + { + Assert.False(token.IsInvalid); + } + } + + [Fact] + public void GetCurrentThread_SafeHandle_PseudoHandleIsUsable() + { + SafeFileHandle thread = PInvoke.GetCurrentThread_SafeHandle(); + try + { + Assert.Equal(new IntPtr(-2), thread.DangerousGetHandle()); + Assert.Equal(PInvoke.GetCurrentThreadId(), PInvoke.GetThreadId(thread)); + } + finally + { + // The pseudo-handle must never be closed. HandleOwnershipTests verifies the ownership annotation. + thread.SetHandleAsInvalid(); + } + } + + [Fact] + public void CreateFile_Failure_ReturnsInvalidHandle() + { + string missingFile = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + using SafeFileHandle file = PInvoke.CreateFile( + missingFile, + (uint)FILE_ACCESS_RIGHTS.FILE_GENERIC_READ, + FILE_SHARE_MODE.FILE_SHARE_READ, + lpSecurityAttributes: null, + FILE_CREATION_DISPOSITION.OPEN_EXISTING, + FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL, + hTemplateFile: null); + int lastError = Marshal.GetLastWin32Error(); + + Assert.True(file.IsInvalid); + Assert.Equal(new IntPtr(-1), file.DangerousGetHandle()); + Assert.Equal((int)WIN32_ERROR.ERROR_FILE_NOT_FOUND, lastError); + } + + [Fact] + public void FindFirstFile_NoMatch_ReturnsInvalidHandle() + { + string missingDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + using FindCloseSafeHandle find = PInvoke.FindFirstFile(Path.Combine(missingDirectory, "*"), out _); + + Assert.True(find.IsInvalid); + Assert.Equal(new IntPtr(-1), find.DangerousGetHandle()); + } + + [Fact] + public void GdiBitmaps_SignExtendedHandles_RoundTripThroughSafeHandles() + { + HDC memoryDC = PInvoke.CreateCompatibleDC(default); + Assert.False(memoryDC.IsNull); + try + { + // GDI handles are 32-bit values that are sign-extended on 64-bit Windows. The upper bits hold a reuse + // counter, so creating and deleting objects soon produces one with the high bit set. + // Bitmaps are used rather than regions because gdi32 caches deleted regions and hands back the same handle. + bool sawSignExtendedBitmap = false; + for (int i = 0; i < MaxGdiAttempts && !sawSignExtendedBitmap; i++) + { + sawSignExtendedBitmap = CreateUseAndDeleteBitmap(memoryDC); + } + + // If this fails, the test never exercised a sign-extended handle, so it proves nothing. + Assert.True(sawSignExtendedBitmap, "No sign-extended HBITMAP was observed."); + } + finally + { + PInvoke.DeleteDC(memoryDC); + } + } + + /// + /// Exercises the generated Marshal.InitHandle path, DangerousValue, Lease, a + /// parameter, and ReleaseHandle for a bitmap. + /// + /// The device context the bitmap is compatible with. + /// if the bitmap's handle was sign-extended. + private static unsafe bool CreateUseAndDeleteBitmap(HDC memoryDC) + { + HBITMAP value; + BITMAP info; + using (DeleteObjectHBITMAPSafeHandle bitmap = PInvoke.CreateCompatibleBitmap_SafeHandle(memoryDC, 1, 1)) + { + Assert.False(bitmap.IsInvalid); + value = (HBITMAP)bitmap.DangerousGetHandle(); + Assert.True(bitmap.DangerousValue == value); + + using (DeleteObjectHBITMAPSafeHandle.LeaseScope lease = bitmap.Lease()) + { + Assert.True(lease.Value == value); + } + + Assert.Equal((uint)OBJ_TYPE.OBJ_BITMAP, PInvoke.GetObjectType(bitmap)); + Assert.Equal(sizeof(BITMAP), PInvoke.GetObject((HGDIOBJ)value, sizeof(BITMAP), &info)); + } + + // Disposal must have deleted the bitmap, not just returned without throwing. + // GetObjectType can still report a type for a deleted handle, but GetObject fails. + Assert.Equal(0, PInvoke.GetObject((HGDIOBJ)value, sizeof(BITMAP), &info)); + return ((IntPtr)value).ToInt64() < 0; + } +} From fd3289a65057e1c48f7ba4d206db14451e96d75f Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Wed, 21 Oct 2026 16:27:03 -0700 Subject: [PATCH 2/2] Keep only ownership tests that pass with current and corrected metadata Remove the 21 HandleOwnershipTests that were skipped until CsWin32 consumes the annotations from microsoft/win32metadata#2380. Each would have needed a CsWin32 change once the metadata is fixed. The remaining tests cover APIs whose ownership is already correct, and pass against both the current metadata package and a winmd built with #2380. Drop the NativeMethods.txt entries and helpers that only those tests used. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61981950-206c-4b2c-844e-c87ed4af9432 --- .../HandleOwnershipTests.cs | 348 +----------------- .../GenerationSandbox.Tests/NativeMethods.txt | 35 +- 2 files changed, 6 insertions(+), 377 deletions(-) diff --git a/test/GenerationSandbox.Tests/HandleOwnershipTests.cs b/test/GenerationSandbox.Tests/HandleOwnershipTests.cs index b1ae7329..b7196801 100644 --- a/test/GenerationSandbox.Tests/HandleOwnershipTests.cs +++ b/test/GenerationSandbox.Tests/HandleOwnershipTests.cs @@ -5,13 +5,8 @@ using System.Runtime.InteropServices; using Microsoft.Win32.SafeHandles; using Windows.Win32; -using Windows.Win32.Foundation; -using Windows.Win32.Graphics.Gdi; using Windows.Win32.Security; using Windows.Win32.System.Console; -using Windows.Win32.System.Memory; -using Windows.Win32.System.Ole; -using Windows.Win32.UI.WindowsAndMessaging; /// /// Calls real Win32 APIs through their SafeHandle-returning friendly overloads and verifies whether the returned @@ -21,24 +16,17 @@ /// /// Ownership comes from win32metadata: [DoNotRelease], [RAIIFree], or the default release function of /// the handle type. A wrong annotation makes disposing the SafeHandle release a resource the caller was only lent, -/// such as deleting a bitmap that is still in use, unloading the user's keyboard layout, closing the process's window -/// station, or calling CloseHandle on a GDI handle value (which can close an unrelated kernel handle). -/// These tests exist to catch metadata regressions, so they call the actual APIs rather than synthetic metadata. +/// or leak one the caller must release. These tests exist to catch metadata regressions, so they call the actual APIs +/// rather than synthetic metadata. /// /// -/// Where releasing a wrongly owned handle has an observable effect on a resource the test created, the test disposes -/// the SafeHandle and checks that the resource survives. Otherwise, it inspects the SafeHandle's ownership directly, -/// then marks it invalid so that it can never release the handle, even when the assertion fails. +/// Only APIs whose ownership is already correct in the metadata that CsWin32 consumes belong here, so that fixing +/// the metadata never requires changing these tests. /// /// [Trait("WindowsOnly", "true")] public unsafe class HandleOwnershipTests { - /// - /// The skip reason for tests that verify annotations added by a win32metadata version that CsWin32 doesn't consume yet. - /// - private const string RequiresMetadataUpdate = "Requires the annotations from https://github.com/microsoft/win32metadata/pull/2380"; - /// /// A string resource name that user32 interprets as the integer resource 32512 (IDC_ARROW and IDI_APPLICATION). /// @@ -112,280 +100,6 @@ public void FindFirstFile_IsOwned() AssertOwned(find); } - [Fact(Skip = RequiresMetadataUpdate)] - public void GetCurrentThread_IsBorrowed() => AssertBorrowed(PInvoke.GetCurrentThread_SafeHandle()); - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetProcessWindowStation_IsBorrowed() - { - CloseWindowStationSafeHandle windowStation = PInvoke.GetProcessWindowStation_SafeHandle(); - Assert.False(windowStation.IsInvalid); - AssertBorrowed(windowStation); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetThreadDesktop_IsBorrowed() - { - CloseDesktopSafeHandle desktop = PInvoke.GetThreadDesktop_SafeHandle(PInvoke.GetCurrentThreadId()); - Assert.False(desktop.IsInvalid); - AssertBorrowed(desktop); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetKeyboardLayout_IsBorrowed() - { - // Releasing this would unload the user's active keyboard layout. - AssertBorrowed(PInvoke.GetKeyboardLayout_SafeHandle(0)); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetStockObject_IsBorrowed() - { - DeleteObjectSafeHandle brush = PInvoke.GetStockObject_SafeHandle(GET_STOCK_OBJECT_FLAGS.WHITE_BRUSH); - Assert.Equal((uint)OBJ_TYPE.OBJ_BRUSH, PInvoke.GetObjectType(brush)); - AssertBorrowed(brush); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetSysColorBrush_IsBorrowed() - { - DeleteObjectHBRUSHSafeHandle brush = PInvoke.GetSysColorBrush_SafeHandle(SYS_COLOR_INDEX.COLOR_WINDOW); - Assert.False(brush.IsInvalid); - AssertBorrowed(brush); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void SelectObject_PreviousObjectIsBorrowed() - { - WithMemoryDC(dc => - { - using DeleteObjectHBITMAPSafeHandle first = PInvoke.CreateCompatibleBitmap_SafeHandle(dc, 1, 1); - using DeleteObjectHBITMAPSafeHandle second = PInvoke.CreateCompatibleBitmap_SafeHandle(dc, 1, 1); - using DeleteObjectSafeHandle original = PInvoke.SelectObject(dc, first); - using (DeleteObjectSafeHandle previous = PInvoke.SelectObject(dc, second)) - { - Assert.Equal(first.DangerousGetHandle(), previous.DangerousGetHandle()); - } - - // Disposing SelectObject's return value must not delete the bitmap, which this test still owns. - BITMAP info; - Assert.Equal(sizeof(BITMAP), PInvoke.GetObject((HGDIOBJ)first.DangerousGetHandle(), sizeof(BITMAP), &info)); - using DeleteObjectSafeHandle restored = PInvoke.SelectObject(dc, original); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetCurrentObject_IsBorrowed() - { - WithMemoryDC(dc => - { - DeleteObjectSafeHandle bitmap = PInvoke.GetCurrentObject_SafeHandle(dc, OBJ_TYPE.OBJ_BITMAP); - Assert.Equal((uint)OBJ_TYPE.OBJ_BITMAP, PInvoke.GetObjectType(bitmap)); - AssertBorrowed(bitmap); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void SelectPalette_PreviousPaletteIsBorrowed() - { - WithMemoryDC(dc => - { - using DeleteObjectSafeHandle defaultPalette = PInvoke.GetStockObject_SafeHandle(GET_STOCK_OBJECT_FLAGS.DEFAULT_PALETTE); - DeleteObjectHPALETTESafeHandle previous = PInvoke.SelectPalette(dc, defaultPalette, false); - Assert.False(previous.IsInvalid); - AssertBorrowed(previous); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetColorSpace_And_SetColorSpace_AreBorrowed() - { - WithMemoryDC(dc => - { - DeleteColorSpaceSafeHandle current = PInvoke.GetColorSpace_SafeHandle(dc); - Assert.False(current.IsInvalid); - DeleteColorSpaceSafeHandle previous = PInvoke.SetColorSpace(dc, current); - Assert.Equal(current.DangerousGetHandle(), previous.DangerousGetHandle()); - AssertBorrowed(previous); - AssertBorrowed(current); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void CopyImage_ReturnedOriginalIsBorrowed() - { - DestroyIconSafeHandle icon = PInvoke.LoadIcon(null, SystemResource32512); - try - { - Assert.False(icon.IsInvalid); - SafeFileHandle copy = PInvoke.CopyImage(icon, GDI_IMAGE_TYPE.IMAGE_ICON, 0, 0, IMAGE_FLAGS.LR_COPYRETURNORG); - - // LR_COPYRETURNORG returns the original shared icon, which the system owns. - // For any other result the right release function depends on the image type, so CloseHandle is never right. - Assert.Equal(icon.DangerousGetHandle(), copy.DangerousGetHandle()); - AssertBorrowed(copy); - } - finally - { - // LoadIcon's shared icon must never be destroyed, whatever the metadata says about it. - icon.SetHandleAsInvalid(); - } - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void LoadIcon_SharedIcon_IsBorrowed() - { - DestroyIconSafeHandle icon = PInvoke.LoadIcon(null, SystemResource32512); - Assert.False(icon.IsInvalid); - AssertBorrowed(icon); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void LoadImage_SharedIcon_IsBorrowed() - { - using FreeLibrarySafeHandle user32 = PInvoke.LoadLibrary("user32.dll"); - SafeFileHandle icon = PInvoke.LoadImage(user32, "#100", GDI_IMAGE_TYPE.IMAGE_ICON, 0, 0, IMAGE_FLAGS.LR_DEFAULTSIZE | IMAGE_FLAGS.LR_SHARED); - Assert.False(icon.IsInvalid, $"LoadImage failed: {Marshal.GetLastWin32Error()}"); - AssertBorrowed(icon); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetCursor_And_SetCursor_AreBorrowed() - { - using DestroyCursorSafeHandle arrow = PInvoke.LoadCursor(null, SystemResource32512); - DestroyCursorSafeHandle previous = PInvoke.SetCursor(arrow); - DestroyCursorSafeHandle current = PInvoke.GetCursor_SafeHandle(); - Assert.Equal(arrow.DangerousGetHandle(), current.DangerousGetHandle()); - DestroyCursorSafeHandle restored = PInvoke.SetCursor(previous); - Assert.Equal(arrow.DangerousGetHandle(), restored.DangerousGetHandle()); - - AssertBorrowed(previous); - AssertBorrowed(current); - AssertBorrowed(restored); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetSubMenu_IsBorrowed() - { - using DestroyMenuSafeHandle parent = PInvoke.CreatePopupMenu_SafeHandle(); - using DestroyMenuSafeHandle child = PInvoke.CreatePopupMenu_SafeHandle(); - HMENU childValue = (HMENU)child.DangerousGetHandle(); - Assert.True(PInvoke.AppendMenu(parent, MENU_ITEM_FLAGS.MF_POPUP, (nuint)childValue.Value, "Child")); - child.SetHandleAsInvalid(); // The parent now owns the submenu and destroys it along with itself. - - using (DestroyMenuSafeHandle subMenu = PInvoke.GetSubMenu(parent, 0)) - { - Assert.True(subMenu.DangerousValue == childValue); - } - - // Disposing GetSubMenu's return value must not destroy the submenu, which still belongs to its parent. - Assert.True(PInvoke.IsMenu(childValue)); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetMenu_IsBorrowed() - { - WithWindow(hwnd => - { - using DestroyMenuSafeHandle menu = PInvoke.CreateMenu_SafeHandle(); - HMENU menuValue = (HMENU)menu.DangerousGetHandle(); - Assert.True(PInvoke.SetMenu(hwnd, menu)); - menu.SetHandleAsInvalid(); // The window now owns the menu and destroys it along with itself. - - using (DestroyMenuSafeHandle windowMenu = PInvoke.GetMenu_SafeHandle(hwnd)) - { - Assert.True(windowMenu.DangerousValue == menuValue); - } - - // Disposing GetMenu's return value must not destroy the window's menu. - Assert.True(PInvoke.IsMenu(menuValue)); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetSystemMenu_IsBorrowed() - { - WithWindow(hwnd => - { - HMENU systemMenuValue; - using (DestroyMenuSafeHandle systemMenu = PInvoke.GetSystemMenu_SafeHandle(hwnd, false)) - { - Assert.False(systemMenu.IsInvalid); - systemMenuValue = (HMENU)systemMenu.DangerousGetHandle(); - } - - // Disposing GetSystemMenu's return value must not destroy the window's system menu. - Assert.True(PInvoke.IsMenu(systemMenuValue)); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetWindowTheme_IsBorrowed() - { - WithWindow(hwnd => - { - using CloseThemeDataSafeHandle theme = PInvoke.OpenThemeData(hwnd, "BUTTON"); - - // GetWindowTheme returns the handle OpenThemeData most recently returned for the window, which the caller - // of OpenThemeData owns. Both are null when visual styles are off. - CloseThemeDataSafeHandle windowTheme = PInvoke.GetWindowTheme_SafeHandle(hwnd); - Assert.Equal(theme.DangerousGetHandle(), windowTheme.DangerousGetHandle()); - AssertBorrowed(windowTheme); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void GetProp_IsBorrowed_And_RemoveProp_IsNotAutoReleased() - { - WithWindow(hwnd => - { - const string PropertyName = "CsWin32.HandleOwnershipTests"; - using DeleteObjectHRGNSafeHandle data = PInvoke.CreateRectRgn_SafeHandle(0, 0, 1, 1); - Assert.True(PInvoke.SetProp(hwnd, PropertyName, data)); - - SafeFileHandle property = PInvoke.GetProp(hwnd, PropertyName); - Assert.Equal(data.DangerousGetHandle(), property.DangerousGetHandle()); - AssertBorrowed(property); - - // RemoveProp hands the data back to the caller, who must free it with the function that matches its type - // (DeleteObject, through `data`, in this case). CloseHandle is never correct. - SafeFileHandle removed = PInvoke.RemoveProp(hwnd, PropertyName); - Assert.Equal(data.DangerousGetHandle(), removed.DangerousGetHandle()); - AssertNotAutoReleased(removed); - }); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void FindExecutable_IsBorrowed() - { - Span executable = stackalloc char[260]; - FreeLibraryHINSTANCESafeHandle result = PInvoke.FindExecutable(Path.Combine(Environment.SystemDirectory, "cmd.exe"), null, executable); - - // FindExecutable returns an error code (greater than 32 on success), not a module handle. - Assert.True(result.DangerousGetHandle().ToInt64() > 32, $"FindExecutable failed: {result.DangerousGetHandle()}"); - AssertBorrowed(result); - } - - [Fact(Skip = RequiresMetadataUpdate)] - public void OleDuplicateData_IsNotAutoReleased() - { - using GlobalFreeSafeHandle source = PInvoke.GlobalAlloc_SafeHandle(GLOBAL_ALLOC_FLAGS.GMEM_MOVEABLE, 16); - Assert.False(source.IsInvalid); - - SafeFileHandle copy = PInvoke.OleDuplicateData(source, CLIPBOARD_FORMAT.CF_TEXT, GLOBAL_ALLOC_FLAGS.GMEM_MOVEABLE); - IntPtr copyValue = copy.DangerousGetHandle(); - try - { - Assert.NotEqual(IntPtr.Zero, copyValue); - AssertNotAutoReleased(copy); - } - finally - { - // The caller owns the copy, but must free it with the function that matches its format, not CloseHandle. - PInvoke.GlobalFree((HGLOBAL)copyValue); - } - } - private static bool OwnsHandle(SafeHandle handle) => (bool)OwnsHandleField.GetValue(handle)!; /// @@ -394,25 +108,10 @@ public void OleDuplicateData_IsNotAutoReleased() /// /// The SafeHandle returned by the API under test. private static void AssertBorrowed(SafeHandle handle) - => AssertDoesNotOwn(handle, $"{handle.GetType().Name} owns a handle that the API only lends to the caller."); - - /// - /// Asserts that will not release the native handle it wraps, then marks it invalid - /// so that it cannot release the handle even if the assertion fails. - /// - /// The SafeHandle returned by the API under test. - /// - /// Use this for handles the caller owns but must release with a function that depends on what the handle refers to, - /// so no single release function in the metadata can be right. - /// - private static void AssertNotAutoReleased(SafeHandle handle) - => AssertDoesNotOwn(handle, $"{handle.GetType().Name} owns a handle whose release function depends on what the handle refers to."); - - private static void AssertDoesNotOwn(SafeHandle handle, string message) { try { - Assert.False(OwnsHandle(handle), message); + Assert.False(OwnsHandle(handle), $"{handle.GetType().Name} owns a handle that the API only lends to the caller."); } finally { @@ -426,41 +125,4 @@ private static void AssertDoesNotOwn(SafeHandle handle, string message) /// The SafeHandle returned by the API under test. private static void AssertOwned(SafeHandle handle) => Assert.True(OwnsHandle(handle), $"{handle.GetType().Name} does not own a handle that the caller must release."); - - /// - /// Runs with a memory device context, then deletes it. - /// - /// The test to run. - private static void WithMemoryDC(Action test) - { - HDC dc = PInvoke.CreateCompatibleDC(default); - Assert.False(dc.IsNull); - try - { - test(dc); - } - finally - { - PInvoke.DeleteDC(dc); - } - } - - /// - /// Runs with a hidden top-level window, which (unlike a message-only window) can have - /// a menu and a system menu, then destroys it. - /// - /// The test to run. - private static void WithWindow(Action test) - { - HWND hwnd = PInvoke.CreateWindowEx(0, "STATIC", nameof(HandleOwnershipTests), WINDOW_STYLE.WS_OVERLAPPEDWINDOW, 0, 0, 100, 100, default, null, null, null); - Assert.False(hwnd.IsNull, $"CreateWindowEx failed: {Marshal.GetLastWin32Error()}"); - try - { - test(hwnd); - } - finally - { - PInvoke.DestroyWindow(hwnd); - } - } } diff --git a/test/GenerationSandbox.Tests/NativeMethods.txt b/test/GenerationSandbox.Tests/NativeMethods.txt index 9c21d08b..c49c625f 100644 --- a/test/GenerationSandbox.Tests/NativeMethods.txt +++ b/test/GenerationSandbox.Tests/NativeMethods.txt @@ -111,55 +111,22 @@ INetFwMgr NetFwMgr IEnumVARIANT INetFwAuthorizedApplication -AppendMenu -CopyImage CreateCompatibleBitmap CreateCompatibleDC CreatePopupMenu CreateRectRgn -FindExecutable FindFirstFile -GetCurrentObject GetCurrentProcess GetCurrentThread GetCurrentThreadId -GetCursor -GetKeyboardLayout GetObjectType +OBJ_TYPE GetProcessHeap GetProcessId -GetProcessWindowStation GetStdHandle -GetStockObject -GetSubMenu -GetSysColorBrush -GetThreadDesktop GetThreadId -IsMenu LoadCursor -LoadIcon -LoadImage OpenProcessToken -SelectObject -CloseThemeData -CreateMenu -CreateWindowEx DeleteDC -DestroyWindow -GetColorSpace -GetMenu -GetProp -GetSystemMenu -GetWindowTheme -OpenThemeData -RemoveProp -SelectPalette -SetColorSpace -SetCursor -SetMenu -SetProp -GlobalAlloc -GlobalFree -OleDuplicateData GetObject BITMAP