Skip to content

Convert exception-related MethodDescCallSite calls to UnmanagedCallersOnly#124834

Merged
AaronRobinsonMSFT merged 19 commits intodotnet:mainfrom
AaronRobinsonMSFT:uco-excep-priority2
Mar 2, 2026
Merged

Convert exception-related MethodDescCallSite calls to UnmanagedCallersOnly#124834
AaronRobinsonMSFT merged 19 commits intodotnet:mainfrom
AaronRobinsonMSFT:uco-excep-priority2

Conversation

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Feb 24, 2026

Summary

Replace MethodDescCallSite/CallDescrWorker calls with more efficient UnmanagedCallersOnly reverse P/Invoke calls in exception handling code paths (priority 2 from #123864).

Changes

Convert the following call sites in excep.cpp to use UnmanagedCallersOnlyCaller:

  • GetExceptionMessage: Object.ToString via UCO (METHOD__RUNTIMEHELPERS__CALL_TO_STRING)
  • ExceptionPreserveStackTrace: InternalPreserveStackTrace via UCO
  • WrapNonCompliantException: RuntimeWrappedException construction via UCO
  • CreateTypeInitializationExceptionObject: TypeInitializationException construction via UCO
  • GetResourceStringFromManaged: Environment.GetResourceStringLocal via UCO
  • GetEventArgsForNotification: FirstChanceExceptionEventArgs construction via UCO

Pattern

Each conversion follows the established pattern:

  1. Add [UnmanagedCallersOnly] static wrapper in managed code with Exception* out-parameter
  2. Add corresponding metasig in metasig.h and DEFINE_METHOD in corelib.h
  3. Replace MethodDescCallSite + ARG_SLOT with UnmanagedCallersOnlyCaller::InvokeThrowing
  4. Remove now-unused DEFINE_METHOD entries for old constructors

All GC references passed to InvokeThrowing are in GCPROTECT'd locations.

…sOnly

Replace MethodDescCallSite/CallDescrWorker calls with more efficient
UnmanagedCallersOnly reverse P/Invoke calls in exception handling code
paths (priority 2 from dotnet#123864):

- GetExceptionMessage: Object.ToString via UCO
- ExceptionPreserveStackTrace: InternalPreserveStackTrace via UCO
- WrapNonCompliantException: RuntimeWrappedException ctor via UCO
- CreateTypeInitializationExceptionObject: TypeInitializationException via UCO
- GetResourceStringFromManaged: Environment.GetResourceStringLocal via UCO
- GetEventArgsForNotification: FirstChanceExceptionEventArgs ctor via UCO

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a 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 pull request converts six exception-related MethodDescCallSite call sites in excep.cpp to use the more efficient UnmanagedCallersOnly reverse P/Invoke pattern, following the established approach from priority 1 and 2 tasks in issue #123864.

Changes:

  • Replaces MethodDescCallSite/CallDescrWorker with UnmanagedCallersOnlyCaller::InvokeThrowing for six exception-handling methods
  • Adds six new UnmanagedCallersOnly wrapper methods in managed code (Object.CoreCLR.cs, Exception.CoreCLR.cs, Environment.CoreCLR.cs)
  • Updates metasig.h with six new method signatures and corelib.h with updated DEFINE_METHOD entries, removing obsolete constructor definitions

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/coreclr/vm/metasig.h Adds 6 new metasig definitions for UnmanagedCallersOnly wrappers (PtrObj_PtrStr, PtrObj_PtrObj, PtrStr_PtrStr, PtrException_PtrException, PtrChar_PtrException_PtrObj, PtrException_PtrObj, all with PtrException_RetVoid suffix)
src/coreclr/vm/corelib.h Updates DEFINE_METHOD entries for the 6 converted methods and removes 3 obsolete constructor entries (TYPE_INIT_EXCEPTION.STR_EX_CTOR, RUNTIME_WRAPPED_EXCEPTION.OBJ_CTOR, FIRSTCHANCE_EVENTARGS.CTOR)
src/coreclr/vm/excep.cpp Converts 6 call sites to use UnmanagedCallersOnlyCaller: GetExceptionMessage, ExceptionPreserveStackTrace, WrapNonCompliantException, CreateTypeInitializationExceptionObject, GetResourceStringFromManaged, GetEventArgsForNotification
src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs Adds GetToString UnmanagedCallersOnly wrapper with Exception* out-parameter
src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs Adds 4 UnmanagedCallersOnly wrappers: InternalPreserveStackTrace, CreateRuntimeWrappedException, CreateTypeInitializationException, CreateFirstChanceExceptionEventArgs
src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs Converts GetResourceStringLocal to UnmanagedCallersOnly pattern with string* in/out parameters

AaronRobinsonMSFT and others added 3 commits February 25, 2026 10:43
- Move GetToString from System.Object to System.Exception to avoid
  breaking reflection in user code
- Clean up stale comment about SystemException fallback in
  CreateTypeInitializationExceptionObject
- Rename GetResourceStringLocal to GetResourceString, drop Local suffix
- Delete unused DEFINE_CLASS(FIRSTCHANCE_EVENTARGS) from corelib.h
- Fold GetResourceStringFromManaged into its single caller ResMgrGetString,
  pass key as char* and move string creation to managed side
- Add GCPROTECT around ResMgrGetString caller in clrex.cpp

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…o single UCO call

Combine event args construction and delegate invocation into a single
managed DeliverFirstChanceNotification UCO method. Multicast delegate
iteration is now handled by managed Delegate.Invoke. Remove the native
DeliverExceptionNotification and GetEventArgsForNotification methods.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@AaronRobinsonMSFT
Copy link
Member Author

@EgorBot -linux_amd -osx_arm64

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

public class Bench
{
    private Exception _cachedException = new InvalidOperationException("test");

    [Benchmark]
    public void ThrowCatch_NoHandler()
    {
        try
        {
            throw _cachedException;
        }
        catch
        {
        }
    }

    [Benchmark]
    public void ThrowCatch_WithHandler()
    {
        AppDomain.CurrentDomain.FirstChanceException += Handler;
        try
        {
            throw _cachedException;
        }
        catch
        {
        }
        finally
        {
            AppDomain.CurrentDomain.FirstChanceException -= Handler;
        }
    }

    private static void Handler(object? sender, System.Runtime.ExceptionServices.FirstChanceExceptionEventArgs e)
    {
    }
}

AaronRobinsonMSFT and others added 2 commits February 25, 2026 16:47
Replace the CanDeliverNotificationToCurrentAppDomain check, which read the
managed FirstChanceException delegate field via CoreLibBinder in cooperative
GC mode, with a native Volatile<BOOL> flag set via a SuppressGCTransition
QCall when a handler is first registered.

This avoids the GCX_COOP transition in DeliverFirstChanceNotification on
every exception throw when no handler is registered. The flag is sticky:
once set to TRUE it is never cleared, avoiding race conditions between
concurrent add/remove operations.

- Remove CanDeliverNotificationToCurrentAppDomain and FIELD__APPCONTEXT__FIRST_CHANCE_EXCEPTION
- Add AppDomain_SetFirstChanceExceptionHandler QCall (SuppressGCTransition)
- Move GCX_COOP inside the handler check in DeliverFirstChanceNotification

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 26, 2026 02:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

- Remove unused using in Exception.CoreCLR.cs
- Move SetFirstChanceExceptionHandler and OnFirstChanceException UCO from
  AppDomain.CoreCLR.cs to new AppContext.CoreCLR.cs
- Move OnFirstChanceException shared logic to AppContext.cs following the
  OnUnhandledException pattern (RuntimeExport for NativeAOT, UCO for CoreCLR)
- Update NativeAOT to use per-handler try/catch
- Pass AppDomain.CurrentDomain as sender on CoreCLR, null on NativeAOT
- Rename QCall entrypoint to AppContext_SetFirstChanceExceptionHandler

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 27, 2026 01:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copilot AI review requested due to automatic review settings February 27, 2026 18:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings March 2, 2026 04:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 77f0e35 into dotnet:main Mar 2, 2026
163 of 168 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the uco-excep-priority2 branch March 2, 2026 16:30
@github-project-automation github-project-automation bot moved this to Done in AppModel Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants