-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix diagnostic for calls on marshalled delegates #117073
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jkotas
commented
Jun 27, 2025
- Fix diagnostic for calls on marshalled delegates. It got broken by switch to precode thunks.
- Add test for diagnostic for calls on marshalled delegates.
- Pass UMEntryThunkData* around in the delegate marshaling code instead of UMEntryThunk*. This avoids constant back and forth hops between the two.
This avoids constant back and forth hops between the two
Tagging subscribers to this area: @mangod9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restores and enhances diagnostics for calls on marshalled delegates by switching from UMEntryThunk*
to UMEntryThunkData*
and adding a test to verify the behavior.
- Added an unhandled exception check for collected delegates and a new test project/source (
collecteddelegate
) - Refactored runtime delegate marshaling code to pass and operate on
UMEntryThunkData*
instead ofUMEntryThunk*
- Introduced
CallbackOnCollectedDelegate
to emit a fail-fast message when invoking a collected delegate
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/tests/baseservices/exceptions/unhandled/unhandledTester.cs | Assert the correct diagnostic message for a collected delegate scenario |
src/tests/baseservices/exceptions/unhandled/collecteddelegate.csproj & .cs | New test project and source triggering a callback on a collected delegate |
src/coreclr/vm/dllimportcallback.h / dllimportcallback.cpp | Updated APIs to use UMEntryThunkData* , added CallbackOnCollectedDelegate |
src/coreclr/vm/syncblk.h / syncblk.cpp | Changed member types and getters to UMEntryThunkData* |
src/coreclr/vm/comdelegate.cpp | Adjusted COM delegate marshaling to the new UMEntryThunkData interface |
src/coreclr/vm/stubmgr.cpp | Updated reverse P/Invoke tracing to accept UMEntryThunkData* |
Comments suppressed due to low confidence (2)
src/coreclr/vm/dllimportcallback.cpp:230
- Missing semicolon after
CallbackOnCollectedDelegate(pUMEntryThunkData)
, causing a compile error.
CallbackOnCollectedDelegate(pUMEntryThunkData)
src/coreclr/vm/comdelegate.cpp:1435
pUMEntryThunk
is now aUMEntryThunkData*
and does not have aGetData()
method; this call will not compile. Consider adding aGetObjectHandle()
method toUMEntryThunkData
or calling the correct accessor directly onpUMEntryThunk
.
? pUMEntryThunk->GetData()->GetObjectHandle()