-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Exclude runtime-async methods from built-in COM vtables and GUID calculations #122195
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
…vtable map concept to handle excluded managed slots in addition to excluded COM slots.
…narios (and fix RCW scenarios)
|
Tagging subscribers to this area: @dotnet/interop-contrib |
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 ensures that built-in COM interop correctly handles runtime-async methods by excluding them from vtable and GUID calculations. For CCWs, async-call-conv methods are omitted from the vtable and CLSID/IID computation to preserve backward compatibility with existing IClassX IIDs/CLSIDs. For RCWs, async-call-conv methods are excluded from vtable slot calculations, and async thunks are adjusted to call Task-returning interface methods using CALLVIRT.
Key changes:
- Excludes runtime-async methods from COM vtable and interface ID generation
- Adds
SparseVTableMap::RecordExcludedMethodto track excluded async methods - Updates async thunk generation to use CALLVIRT for abstract ComImport methods
- Adds comprehensive tests validating vtable layout for both compiler-async and runtime-async scenarios
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/methodtablebuilder.cpp | Records async variant methods as excluded in COM vtables when building method tables |
| src/coreclr/vm/method.cpp | Adds assertion that async methods don't request COM slots |
| src/coreclr/vm/interoputil.cpp | Converts ThrowHR to assertion for async methods in stringified interface definitions |
| src/coreclr/vm/dispatchinfo.cpp | Returns NULL instead of throwing for runtime-async methods in IDispatch scenarios |
| src/coreclr/vm/comtoclrcall.cpp | Removes redundant async method check and adds precondition assertions |
| src/coreclr/vm/commtmemberinfomap.cpp | Skips async methods when building COM member info maps and handles null entries |
| src/coreclr/vm/comcallablewrapper.cpp | Filters out async methods during CCW vtable layout for both classes and interfaces |
| src/coreclr/vm/clrtocomcall.cpp | Fixes switch statement formatting and confirms async methods not supported on IDispatch |
| src/coreclr/vm/class.h | Declares RecordExcludedMethod for SparseVTableMap |
| src/coreclr/vm/class.cpp | Implements RecordExcludedMethod to track excluded vtable slots |
| src/coreclr/vm/asyncthunks.cpp | Uses CALLVIRT for abstract task-returning variants (ComImport scenarios) |
| src/tests/Interop/COM/ServerContracts/Server.Contracts.h | Adds COM interface definitions with runtime-generated IIDs for test validation |
| src/tests/Interop/COM/RuntimeAsync/RuntimeAsync.cs | Tests that async methods don't modify CCW/RCW vtables |
| src/tests/Interop/COM/RuntimeAsync/TaskComServer.cs | Implements COM server with Task-returning methods for testing |
| src/tests/Interop/COM/RuntimeAsync/RuntimeAsyncNative.cpp | Native functions to validate vtable slot layout from C++ |
| src/tests/Interop/COM/RuntimeAsync/RuntimeAsync.csproj | Test project with runtime-async feature enabled |
| src/tests/Interop/COM/RuntimeAsync/CompilerAsync.csproj | Test project using compiler async (baseline comparison) |
| src/tests/Interop/COM/RuntimeAsync/CMakeLists.txt | Build configuration for native test library |
| src/tests/Interop/CMakeLists.txt | Adds RuntimeAsync subdirectory to build |
AaronRobinsonMSFT
left a comment
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.
Should we also update the COM source generator to avoid generating these signatures? Basically any interface marked with GeneratedComInterface can't have an async method?
I don't think we need to block anything for Unlike built-in COM, we don't need to worry about the runtime introducing additional synthetic members as we burn in vtable offsets at compile time (and even if we set them up at runtime, the async variants are hidden from reflection so we wouldn't accidentally see them). |
Co-authored-by: Aaron R Robinson <[email protected]>
jkotas
left a comment
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.
LGTM. Thank you for adding the tests
Adjust built-in COM to interact correctly with runtime-async.
For CCWs, async-call-conv methods are excluded from the vtable and CLSID/IID computation. This ensures that auto-generated IIDs/CLSIDs for the IClassX scenario are unchanged. Additionally, this ensures that the Task-returning (compiler-async call conv) variants are the variants present in the vtable, preserving existing behavior.
For RCWs, the async-call-conv methods are excluded from the vtable slot calculation and the async thunk variants are adjusted to not be treated as CLR->COM methods and instead call the Task-returning thunks on the ComImport interface (similar to how they behave regularly).
Also adds tests that run with both compiler async and runtime async to ensure equivalent behavior.
Fixes #121765