Convert remaining PREPARE_NONVIRTUAL_CALLSITE call sites to UnmanagedCallersOnly#125697
Open
Convert remaining PREPARE_NONVIRTUAL_CALLSITE call sites to UnmanagedCallersOnly#125697
Conversation
…TE → UnmanagedCallersOnlyCaller conversion Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
…e conversions Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
…ies in feature guards Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
…sing Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
jkoritzinsky
March 17, 2026 23:37
View session
jkoritzinsky
approved these changes
Mar 17, 2026
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the conversion of remaining PREPARE_NONVIRTUAL_CALLSITE + CALL_MANAGED_METHOD* usages (excluding intentionally-skipped EH dispatch paths) to the UnmanagedCallersOnly / UnmanagedCallersOnlyCaller pattern, simplifying call sites and reducing GC-mode ceremony by routing native→managed calls through reverse-P/Invoke-compatible wrappers.
Changes:
- Adds new
[UnmanagedCallersOnly]wrappers in System.Private.CoreLib for native-callable entrypoints with trailingException*out-args. - Updates CoreCLR native call sites to use
UnmanagedCallersOnlyCaller.InvokeThrowing*instead ofPREPARE_NONVIRTUAL_CALLSITE+ arg holders. - Updates CoreLib binder signatures (
corelib.h,metasig.h) to match the new wrapper shapes (pointer params + trailing exception handle).
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs | Adds CORECLR-only UCO wrapper for lock inflation state transfer. |
| src/libraries/System.Private.CoreLib/src/System/Threading/AutoreleasePool.cs | Adds CORECLR-only UCO wrappers for ObjC autorelease pool lifecycle. |
| src/coreclr/vm/typeparse.cpp | Converts TypeNameResolver GetTypeHelper call site to UCO pattern and adjusts GC protection. |
| src/coreclr/vm/threads.cpp | Converts thread-related managed callbacks (autorelease pool + OnThreadExiting) to UCO calls. |
| src/coreclr/vm/syncblk.cpp | Converts Lock.InitializeForMonitor call site to UCO and adds GC protection. |
| src/coreclr/vm/runtimecallablewrapper.cpp | Converts __ComObject.ReleaseAllData call site to UCO. |
| src/coreclr/vm/olevariant.cpp | Converts BSTR marshaling call sites to UCO _UCO-suffixed targets. |
| src/coreclr/vm/nativelibrary.cpp | Converts ALC unmanaged-DLL resolution and NativeLibrary callback stub to UCO. |
| src/coreclr/vm/metasig.h | Adds new metasig definitions needed for UCO-compatible signatures. |
| src/coreclr/vm/interoputil.cpp | Converts EnumeratorToEnumVariantMarshaler invocation to UCO and adds GC protection. |
| src/coreclr/vm/interoplibinterface_comwrappers.cpp | Converts ComWrappers global marshalling entrypoints to UCO. |
| src/coreclr/vm/finalizerthread.cpp | Converts GC.RunFinalizers invocation to UCO return-path helper. |
| src/coreclr/vm/dynamicinterfacecastable.cpp | Converts DynamicInterfaceCastable helper calls to UCO and adjusts rooting strategy. |
| src/coreclr/vm/corhost.cpp | Converts Environment.InitializeCommandLineArgs invocation to UCO and adds GC protection. |
| src/coreclr/vm/corelib.h | Updates CoreLib binder method signatures to UCO-compatible forms (ptr params + Exception*). |
| src/coreclr/vm/coreassemblyspec.cpp | Converts AssemblyName.ParseAsAssemblySpec invocation to UCO. |
| src/coreclr/vm/comsynchronizable.cpp | Converts Thread.StartCallback invocation to UCO and adds GC protection. |
| src/coreclr/vm/assemblyspec.cpp | Switches AssemblyName initialization from ctor to CreateAssemblyName UCO wrapper. |
| src/coreclr/vm/assemblynative.cpp | Removes pre-allocation of AssemblyName; relies on new UCO wrapper to allocate. |
| src/coreclr/vm/appdomain.cpp | Removes pre-allocation of AssemblyName during resolver path; relies on new UCO wrapper. |
| src/coreclr/binder/defaultassemblybinder.cpp | Converts default ALC initialization call site to UCO. |
| src/coreclr/binder/activitytracker.cpp | Converts ActivityTracker start/stop call sites to UCO. |
| src/coreclr/System.Private.CoreLib/src/System/__ComObject.cs | Adds UCO wrapper for ReleaseAllData. |
| src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs | Adds UCO wrappers for StartCallback and OnThreadExiting. |
| src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs | Adds UCO wrappers for BSTRMarshaler ConvertToManaged/ConvertToNative. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs | Adds UCO wrappers for unmanaged DLL resolution and assembly-load activity tracking. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.CoreCLR.cs | Adds UCO wrapper for NativeLibrary.LoadLibraryCallbackStub. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/DynamicInterfaceCastableHelpers.cs | Adds UCO wrappers for DynamicInterfaceCastable helper entrypoints. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/CustomMarshalers/EnumeratorToEnumVariantMarshaler.cs | Adds UCO wrapper for InternalMarshalNativeToManaged. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreCLR.cs | Adds UCO wrappers for ComWrappers global marshalling entrypoints. |
| src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs | Adds UCO wrapper for VM-facing GetTypeHelper entrypoint. |
| src/coreclr/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreCLR.cs | Adds UCO wrappers for ParseAsAssemblySpec and CreateAssemblyName. |
| src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs | Adds UCO wrapper for RunFinalizers. |
| src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs | Adds UCO wrapper for InitializeCommandLineArgs. |
Comment on lines
+142
to
+146
| private static unsafe void GetTypeHelper(char* pTypeName, RuntimeAssembly* pRequestingAssembly, byte throwOnError, byte requireAssemblyQualifiedName, IntPtr unsafeAccessorMethod, RuntimeType* pResult, Exception* pException) | ||
| { | ||
| try | ||
| { | ||
| *pResult = GetTypeHelper(pTypeName, *pRequestingAssembly, throwOnError != 0, requireAssemblyQualifiedName != 0, unsafeAccessorMethod); |
Comment on lines
+25
to
+35
| [UnmanagedCallersOnly] | ||
| private static unsafe void IsInterfaceImplemented(IDynamicInterfaceCastable* pCastable, RuntimeType* pInterfaceType, byte throwIfNotImplemented, byte* pResult, Exception* pException) | ||
| { | ||
| try | ||
| { | ||
| *pResult = IsInterfaceImplemented(*pCastable, *pInterfaceType, throwIfNotImplemented != 0) ? (byte)1 : (byte)0; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| *pException = ex; | ||
| } |
Comment on lines
+28
to
+38
| [UnmanagedCallersOnly] | ||
| private static unsafe void LoadLibraryCallbackStub(string* pLibraryName, Assembly* pAssembly, byte hasDllImportSearchPathFlags, uint dllImportSearchPathFlags, IntPtr* pResult, Exception* pException) | ||
| { | ||
| try | ||
| { | ||
| *pResult = LoadLibraryCallbackStub(*pLibraryName, *pAssembly, hasDllImportSearchPathFlags != 0, dllImportSearchPathFlags); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| *pException = ex; | ||
| } |
Comment on lines
+21
to
+31
| struct { | ||
| OBJECTREF obj; | ||
| OBJECTREF managedType; | ||
| } gc; | ||
| gc.obj = *objPROTECTED; | ||
| gc.managedType = interfaceTypeHandle.GetManagedClassObject(); // GC triggers | ||
|
|
||
| CLR_BOOL isImplemented = FALSE; | ||
| GCPROTECT_BEGIN(gc); | ||
| UnmanagedCallersOnlyCaller isInterfaceImplemented(METHOD__DYNAMICINTERFACECASTABLEHELPERS__IS_INTERFACE_IMPLEMENTED); | ||
| isInterfaceImplemented.InvokeThrowing(&gc.obj, &gc.managedType, CLR_BOOL_ARG(throwIfNotImplemented), &isImplemented); |
Comment on lines
+52
to
+59
| } gc; | ||
| gc.obj = *objPROTECTED; | ||
| gc.managedType = interfaceTypeHandle.GetManagedClassObject(); // GC triggers | ||
| gc.result = NULL; | ||
|
|
||
| GCPROTECT_BEGIN(gc); | ||
| UnmanagedCallersOnlyCaller getInterfaceImplementation(METHOD__DYNAMICINTERFACECASTABLEHELPERS__GET_INTERFACE_IMPLEMENTATION); | ||
| getInterfaceImplementation.InvokeThrowing(&gc.obj, &gc.managedType, &gc.result); |
src/coreclr/vm/olevariant.cpp
Outdated
| CALL_MANAGED_METHOD(bstr, BSTR, args); | ||
| GCPROTECT_BEGIN(*pStringObj); | ||
| UnmanagedCallersOnlyCaller convertToNative(METHOD__BSTRMARSHALER__CONVERT_TO_NATIVE_UCO); | ||
| BSTR bstr = NULL; |
Member
…plicate bstr declaration Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Contributor
Author
|
Fixed in d465333:
|
jkotas
reviewed
Mar 18, 2026
| } | ||
|
|
||
| [UnmanagedCallersOnly] | ||
| private static unsafe uint RunFinalizers(Exception* pException) |
Member
There was a problem hiding this comment.
Does this affect diagnosability of unhandled exceptions thrown by finalizers (e.g. from crash dumps)?
Also, the two RunFinalizers methods can be merged.
Member
There was a problem hiding this comment.
@jkoritzinsky See the "Exception Propagation Helper" section in #123864. I've not written the QCall yet, but I can share the details if you'd like.
jkotas
reviewed
Mar 18, 2026
| { | ||
| try | ||
| { | ||
| pThread->StartCallback(); |
Member
There was a problem hiding this comment.
Same here. Does this affect diagnosability of unhandled exceptions?
jkotas
reviewed
Mar 18, 2026
| } | ||
|
|
||
| [UnmanagedCallersOnly] | ||
| private static unsafe void OnThreadExiting(Thread* pThread, Exception* pException) |
Member
There was a problem hiding this comment.
It is a bit misleading to call this OnThreadExiting since it is only ever called after the thread exited.
jkotas
reviewed
Mar 18, 2026
| { | ||
| try | ||
| { | ||
| *pResult = GetTypeHelper(pTypeName, *pRequestingAssembly, throwOnError, requireAssemblyQualifiedName, unsafeAccessorMethod); |
Member
There was a problem hiding this comment.
Actual work can be inlined here.
jkotas
reviewed
Mar 18, 2026
| { | ||
| try | ||
| { | ||
| ParseAsAssemblySpec(pAssemblyName, pAssemblySpec); |
Member
There was a problem hiding this comment.
Actual work can be inlined here.
jkotas
reviewed
Mar 18, 2026
| { | ||
| try | ||
| { | ||
| *pResult = InternalMarshalNativeToManaged(pNativeData); |
Member
There was a problem hiding this comment.
Actual work can be inlined here.
Member
There was a problem hiding this comment.
(This applies to all cases where the managed method exists for the sole purpose of being called by the VM, and it is not very complicated.)
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.
Converts all remaining
PREPARE_NONVIRTUAL_CALLSITE+CALL_MANAGED_METHOD*call sites (outside of the intentionally-skipped EH dispatch methods) to theUnmanagedCallersOnly/UnmanagedCallersOnlyCallerpattern, reducing GC-mode ceremony and improving call-site clarity.Description
Infrastructure (
vm/corelib.h,vm/metasig.h)DEFINE_METHODsignatures for ~23 methods to UCO-compatible form (pointer params + trailingException*)DEFINE_METASIG_Tentries;FEATURE_COMINTEROP/FEATURE_COMWRAPPERS-gated entries wrapped in matching#ifdefguardsBSTRMarshaler: kept original entries (used by IL-emit inilmarshalers.cpp) and added_UCO-suffixed entries for native call sitesASSEMBLY_NAME CTOR→CREATE_ASSEMBLY_SPECto better reflect what the method doesC# —
[UnmanagedCallersOnly]wrappers added to:AssemblyLoadContext.CoreCLR.cs—InitializeDefaultContext,StartAssemblyLoad,StopAssemblyLoad,ResolveUnmanagedDll,ResolveUnmanagedDllUsingEventGC.CoreCLR.cs—RunFinalizersEnvironment.CoreCLR.cs—InitializeCommandLineArgsThread.CoreCLR.cs—StartCallback,OnThreadExitingAssemblyName.CoreCLR.cs—ParseAsAssemblySpec,CreateAssemblyName__ComObject.cs—ReleaseAllDataComWrappers.CoreCLR.cs—CallICustomQueryInterface,GetOrCreateComInterfaceForObjectWithGlobalMarshallingInstance,GetOrCreateObjectForComInstanceWithGlobalMarshallingInstanceDynamicInterfaceCastableHelpers.cs—IsInterfaceImplemented(UCO wrapper usesbool/bool*to matchCLR_BOOLmetasig),GetInterfaceImplementationTypeNameResolver.CoreCLR.cs—GetTypeHelper(UCO wrapper usesboolparameters to matchCLR_BOOLmetasig)StubHelpers.cs(BSTRMarshaler) —ConvertToManaged,ConvertToNativeEnumeratorToEnumVariantMarshaler.cs—InternalMarshalNativeToManagedNativeLibrary.CoreCLR.cs—LoadLibraryCallbackStub(UCO wrapper usesboolto matchCLR_BOOLmetasig)Lock.cs—InitializeForMonitor(under#if CORECLR)AutoreleasePool.cs—CreateAutoreleasePool,DrainAutoreleasePool(under#if CORECLR)C++ call sites converted in:
activitytracker.cpp,defaultassemblybinder.cpp,finalizerthread.cpp,syncblk.cpp,comsynchronizable.cpp,threads.cpp,corhost.cpp,dynamicinterfacecastable.cpp,typeparse.cpp,coreassemblyspec.cpp,assemblyspec.cpp,assemblynative.cpp,appdomain.cpp,runtimecallablewrapper.cpp,interoplibinterface_comwrappers.cpp,olevariant.cpp,interoputil.cpp,nativelibrary.cppPre-allocations of
AssemblyNameobjects removed fromassemblynative.cppandappdomain.cpp— theCreateAssemblyNameUCO wrapper now allocates the object itself.Bug fixes (addressed in review):
dynamicinterfacecastable.cpp: Fixed GC holes inCallIsInterfaceImplementedandCallGetInterfaceImplementation—gcis now initialized to NULLs andGCPROTECT_BEGINis entered before the GC-triggeringGetManagedClassObject()call, withgc.objassigned inside the protected scope.olevariant.cpp: Removed shadowingBSTR bstr = NULL;declaration inside theGCPROTECT_BEGINscope that causedInvokeThrowing's result to be discarded, always returning NULL.Intentionally skipped:
exceptionhandling.cpp/excep.cpp(RhThrowHwEx,RhThrowEx,RhRethrow— NORETURN EH dispatch) anddllimport.cpp(PREPARE_NONVIRTUAL_CALLSITE_USING_CODE— dynamic code pointer).📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.