Repository navigation
Fix OverflowException for sign-extended handles in SafeHandle conversions - #1864
Merged
Jevan Saks (jevansaks) merged 2 commits intoOct 22, 2026
Merged
Jevan Saks (jevansaks) merged 2 commits into
Jevan Saks (jevansaks) merged 2 commits into
Conversation
…ions #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
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
Jevan Saks (jevansaks)
marked this pull request as ready for review
October 21, 2026 23:31
Sergio Pedri (Sergio0694)
approved these changes
Oct 21, 2026
Jevan Saks (jevansaks)
enabled auto-merge (squash)
October 21, 2026 23:42
Jevan Saks (jevansaks)
deleted the
jevansaks-review-raiifree-pseudohandle-regression
branch
October 22, 2026 00:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since v0.3.366, generated SafeHandle code compiled with
CheckForOverflowUnderflowthrowsOverflowExceptionfor handles with the high bit set, such asINVALID_HANDLE_VALUE, pseudo-handles likeGetCurrentProcess(), and sign-extended GDI/USER handles.#1844 introduced two checked conversions:
(IntPtr)handle.Valuewhen wrapping a typedef handle in a SafeHandle, and(void*)nintwhen leasing a value back out of one. Previously the conversion went through the typedef's implicitIntPtroperator, which can't throw. This is the same class of bug as #1412 (fixed by #1513).Fix
Emit both conversions in an
uncheckedcontext.Tests
Both test classes call real Win32 APIs rather than synthetic metadata.
SignExtendedHandleTestspasses pseudo-handles,INVALID_HANDLE_VALUE, and sign-extended GDI bitmap handles through the SafeHandle overloads and leases. Without the fix they fail withOverflowExceptionon .NET 8 and later.HandleOwnershipTestschecks whether the SafeHandle returned by an API owns its handle, for APIs whose metadata is already correct, to catch future metadata regressions.