[Async v2] Implement async method variant handling in AddMethod and UpdateMethod by tommcdon · Pull Request #125397 · dotnet/runtime

tommcdon · 2026-03-10T18:33:59Z

This change implements a TODO item in EEClass::AddMethodDesc to support Runtime Async

dotnet-policy-service · 2026-03-10T18:35:00Z

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR implements runtime-async method variant support in the Edit-and-Continue (EnC) path by extending EEClass::AddMethodDesc/EEClass::AddMethod to carry async metadata (flags + optional alternate signature) and by updating EnC method update logic to consider async method variants.

Changes:

Extend EEClass::AddMethodDesc to accept async flags and an optional async-variant signature, and plumb that into MethodTableBuilder::InitMethodDesc.
Add async return-type classification and async-variant creation logic to EEClass::AddMethod.
Update EditAndContinueModule::UpdateMethod to also reset the entrypoint for the method’s async counterpart.

Reviewed changes

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

File	Description
src/coreclr/vm/encee.cpp	Resets entrypoints for async counterparts during EnC method updates.
src/coreclr/vm/class.h	Extends `AddMethodDesc` signature to accept async flags and optional async signature.
src/coreclr/vm/class.cpp	Implements async return classification and passes async flags/signature into EnC-added `MethodDesc` creation.

You can also share your feedback on Copilot code review. Take the survey.

src/coreclr/vm/class.cpp

src/coreclr/vm/encee.cpp

src/coreclr/vm/class.cpp

Use GetAsyncOtherVariantNoCreate in UpdateMethod

Extract BuildAsyncVariantSignature to share async signature construction The async variant signature construction logic (stripping Task/ValueTask wrapper from the return type) was duplicated between MethodTableBuilder (initial type load) and EEClass::AddMethod (EnC add method). Extract it into a shared BuildAsyncVariantSignature helper in method.cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

src/coreclr/vm/class.cpp

src/coreclr/vm/encee.cpp

src/coreclr/vm/class.cpp

src/coreclr/vm/encee.cpp

When adding a new async method to a generic type via EnC, the async variant MethodDesc was only created for the type definition but not for existing canonical instantiations. This caused crashes when the newly added async method was called on value type instantiations (e.g. G<int>) because the thunk could not find its async variant. Hoist the async variant signature and flags computation above the generic instantiation loop, then create the async variant for each canonical MethodTable alongside the primary thunk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ejection, generic UpdateMethod comment - class.cpp:579: Update GC violation comment to acknowledge the extern-alias edge case as Won't Fix per reviewer feedback - class.cpp:597: Reject infrastructure async methods (IsMiAsync but not task-returning) on non-system modules with COR_E_BADIMAGEFORMAT, mirroring MethodTableBuilder validation - encee.cpp:388: Add comment explaining that ResetCodeEntryPointForEnC cascades from thunk to async variant automatically, so generic UpdateMethod path does not need explicit async variant handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

src/coreclr/vm/method.cpp

…VariantSignature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/coreclr/vm/class.cpp

noahfalk

Looks good 👍 Couple suggestions inline

…ure async in EnC - Clarify GC violation comment to explain the specific unlikely scenario (naming collision + extern alias + hot reload while debugging) - Reject infrastructure async methods unconditionally in EnC rather than only on non-system modules, since no scenario benefits from allowing them Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

src/coreclr/vm/class.cpp

…T_NOT_SUPPORTED - Rewrite GC violation comment per jkotas: the issue is unresolved TypeRef/AssemblyRef for Task/ValueTask, not type misidentification. System.Runtime is typically already resolved so this is unlikely. - Use CORDBG_E_ENC_EDIT_NOT_SUPPORTED instead of COR_E_BADIMAGEFORMAT for infrastructure async rejection, since this is an unsupported edit rather than malformed metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Allocate the async variant signature from each instantiation's own LoaderAllocator instead of reusing the type definition's allocation. AddMethodDesc stores the signature as a raw pointer in AsyncMethodData, so it must be owned by the same allocator as the MethodDesc to avoid dangling pointers on collectible ALC unload. Also fix GC violation comment per jkotas feedback and use CORDBG_E_ENC_EDIT_NOT_SUPPORTED for infrastructure async rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

src/coreclr/vm/class.cpp

src/coreclr/vm/method.hpp

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

src/coreclr/vm/class.cpp

jkotas · 2026-04-05T04:05:11Z

src/coreclr/vm/class.cpp

+
+                // For async task-returning methods, also create the async variant for
+                // this instantiation. Without this, the thunk on the instantiation's
+                // MethodTable cannot find its async variant, causing crashes when the


I believe we can create the async variants lazily for non-EnC methods. What's special about EnC method that it does not work? This comment says that it does not work, but does not really explain why.

The comment was misleading, we didn't support lazily creating the generic async variant today, and felt that it might be a less complex code change to eagerly create it. I've prototyped the change for lazily creating the generic async variant and will be pushing it shortly. If we feel it adds too much complexity we can revert it and update the comment.

@jkotas @noahfalk please see the latest 2 changes - 1) it supports lazily creating the generic async variants, and 2) an opportunistic fix for a ARM64 weak memory model issue caused by SetNextChunk(pNewChunk) adding to the linked list without a VolatileStore.

Generic instantiations are guaranteed to have same or shorter lifetime than the method definition, so reusing the type definition's signature allocation is safe. Remove the unnecessary per-allocator copy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Previously, AddMethod eagerly created async variant MethodDescs for all runtime-async methods added via EnC (on the type-def and every existing generic instantiation). This commit defers variant creation to the point of first use: - FindOrCreateAssociatedMethodDesc (genmeth.cpp): lazily creates the variant using double-check locking with m_InstMethodHashTableCrst, matching the existing NewInstantiatedMethodDesc pattern. - LoadTypicalMethodDefinition (method.cpp): triggers lazy creation when navigating from an instantiation variant back to the type-def variant. - AddAsyncVariant (class.cpp): new helper that computes the variant signature on-demand from metadata (ClassifyMethodReturnKind + BuildAsyncVariantSignature) rather than storing it on primary thunks. - ResetCodeEntryPointForEnC (method.cpp): gracefully handles the case where the variant has not been created yet (no-op). - InitMethodDesc (methodtablebuilder.cpp): restored original contract where only IsAsyncVariant methods store a signature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AddChunk publishes a new MethodDescChunk to a lock-free linked list that concurrent readers iterate without holding a lock. On weakly ordered architectures (ARM64), plain stores can be reordered, allowing a reader to see the new chunk pointer before the chunk's internal data (MethodDescs, flags, etc.) is fully visible. Use VolatileStore at both publish points (head and tail) to act as a release barrier, matching the pattern used elsewhere in the VM (e.g. dacenumerablehash.inl, codeversion.cpp, eehash.inl). This is a no-op on x86/x64 (strong memory model). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Copilot · 2026-04-12T03:22:30Z

src/coreclr/vm/class.cpp

+        // resolved at this point. However, if the TypeRef/AssemblyRef for Task or ValueTask
+        // has not been resolved yet, the resolution path could trigger GC. Accepted as
+        // Won't Fix given this is unlikely in practice.
+        CONTRACT_VIOLATION(GCViolation);
+        returnKind = ClassifyMethodReturnKind(


EEClass::AddMethod is under GC_NOTRIGGER but calls ClassifyMethodReturnKind(), which can trigger GC via type resolution; the added CONTRACT_VIOLATION only suppresses contract checks and doesn't make the code safe if GC actually happens. Consider avoiding the GC-triggering path (e.g., only classify when IsMiAsync is set, and use a metadata-only Task/ValueTask check or otherwise reject the edit when the required type refs aren’t already resolved) rather than relying on a GC_NOTRIGGER violation here.

Copilot · 2026-04-12T03:22:30Z

src/coreclr/vm/class.cpp

+        // Use VolatileStore to ensure the chunk's internal data (MethodDescs, flags, etc.)
+        // is fully visible to concurrent readers before the chunk becomes reachable.
+        VolatileStore(&m_pChunks, pNewChunk);


VolatileStore(&m_pChunks, ...) and SetNextChunkVolatile() are intended to safely publish newly added chunks to concurrent readers, but the corresponding reads (EEClass::GetChunks() and MethodDescChunk::GetNextChunk()) are plain loads. Without an acquire/volatile load, the release-store alone may not provide the intended visibility guarantees on weak memory models; either update the reader side to use VolatileLoad (or otherwise synchronize) or drop the concurrency claim here.

Copilot · 2026-04-12T03:22:30Z

src/coreclr/vm/method.hpp

+    void SetNextChunkVolatile(MethodDescChunk *chunk)
+    {
+        LIMITED_METHOD_CONTRACT;
+        VolatileStore(&m_next, dac_cast<PTR_MethodDescChunk>(chunk));
+    }


SetNextChunkVolatile() uses VolatileStore, but GetNextChunk() returns m_next via a plain load. If this is meant to provide safe publication to concurrent traversals, consider pairing with a VolatileLoad (or otherwise documenting/synchronizing reads); otherwise the volatile store may not achieve the intended memory-ordering guarantee on weak architectures.

Copilot · 2026-04-12T03:22:30Z

src/coreclr/vm/encee.cpp

+        // For runtime-async methods, also reset the async variant's entry point.
+        // The task-returning variant is stored in the module's method lookup, but
+        // its async counterpart also needs to be re-JITted with the new IL.
+        if (pMethod->HasAsyncOtherVariant())


EditAndContinueModule::UpdateMethod now resets the async other variant explicitly for non-generic methods, but MethodDesc::ResetCodeEntryPointForEnC already cascades from async thunks to the IL-owning variant (and now safely no-ops if the variant isn’t created). This can lead to redundant double resets in the common thunk case; consider relying on ResetCodeEntryPointForEnC’s existing async handling, or only doing the extra reset when the passed-in MethodDesc is the async variant.

Suggested change

// For runtime-async methods, also reset the async variant's entry point.

// The task-returning variant is stored in the module's method lookup, but

// its async counterpart also needs to be re-JITted with the new IL.

if (pMethod->HasAsyncOtherVariant())

// For runtime-async methods, the module lookup stores the task-returning

// variant. If this MethodDesc is that IL-owning variant, explicitly reset

// the paired async thunk as well. When pMethod is already the async thunk,

// ResetCodeEntryPointForEnC() cascades to the IL-owning variant, so

// resetting the paired MethodDesc here would be redundant.

if (!pMethod->IsAsyncThunkMethod() && pMethod->HasAsyncOtherVariant())

tommcdon added this to the 11.0.0 milestone Mar 10, 2026

tommcdon self-assigned this Mar 10, 2026

tommcdon added the area-Diagnostics-coreclr label Mar 10, 2026

Copilot AI review requested due to automatic review settings March 10, 2026 18:34

Copilot started reviewing on behalf of tommcdon March 10, 2026 18:35 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

src/coreclr/vm/class.cpp Show resolved Hide resolved

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/encee.cpp Show resolved Hide resolved

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

jkotas reviewed Mar 11, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

jkotas added the runtime-async label Mar 11, 2026

tommcdon and others added 3 commits March 20, 2026 13:45

Implement async method variant handling in AddMethod and UpdateMethod

973c492

Use async variant in EditAndContinueModule::UpdateMethod

957c307

Use GetAsyncOtherVariantNoCreate in UpdateMethod

tommcdon force-pushed the dev/tommcdon/implement_addmethoddesc branch from 76d4eb1 to 437f9ed Compare March 20, 2026 20:08

tommcdon marked this pull request as ready for review March 20, 2026 20:09

Copilot AI review requested due to automatic review settings March 20, 2026 20:09

Copilot started reviewing on behalf of tommcdon March 20, 2026 20:10 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

src/coreclr/vm/class.cpp Show resolved Hide resolved

src/coreclr/vm/class.cpp Show resolved Hide resolved

src/coreclr/vm/encee.cpp Show resolved Hide resolved

noahfalk reviewed Apr 2, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/encee.cpp Show resolved Hide resolved

tommcdon and others added 2 commits April 2, 2026 19:49

Copilot AI review requested due to automatic review settings April 3, 2026 00:50

Copilot started reviewing on behalf of tommcdon April 3, 2026 00:51 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

src/coreclr/vm/method.cpp Outdated Show resolved Hide resolved

Remove dead store taskTokenOffsetFromAsyncDetailsOffset in BuildAsync…

270e1d3

…VariantSignature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

noahfalk reviewed Apr 3, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

noahfalk reviewed Apr 3, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

noahfalk approved these changes Apr 3, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 4, 2026 23:20

Copilot started reviewing on behalf of tommcdon April 4, 2026 23:21 View session

Copilot AI reviewed Apr 4, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

jkotas reviewed Apr 5, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

tommcdon and others added 2 commits April 4, 2026 22:03

Copilot AI review requested due to automatic review settings April 5, 2026 02:41

Copilot started reviewing on behalf of tommcdon April 5, 2026 02:42 View session

Copilot AI reviewed Apr 5, 2026

View reviewed changes

src/coreclr/vm/class.cpp Show resolved Hide resolved

src/coreclr/vm/method.hpp Outdated Show resolved Hide resolved

Replace Unicode arrows with ASCII in C++ comments

79073ff

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

jkotas reviewed Apr 5, 2026

View reviewed changes

src/coreclr/vm/class.cpp Outdated Show resolved Hide resolved

jkotas reviewed Apr 5, 2026

View reviewed changes

tommcdon mentioned this pull request Apr 10, 2026

Fix DAC GetNativeCodeInfo API to use async variant for thunk MethodDescs #126728

Merged

tommcdon and others added 2 commits April 11, 2026 21:48

Copilot AI review requested due to automatic review settings April 12, 2026 03:16

Copilot started reviewing on behalf of tommcdon April 12, 2026 03:17 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

-        // For runtime-async methods, also reset the async variant's entry point.
-        // The task-returning variant is stored in the module's method lookup, but
-        // its async counterpart also needs to be re-JITted with the new IL.
-        if (pMethod->HasAsyncOtherVariant())
+        // For runtime-async methods, the module lookup stores the task-returning
+        // variant. If this MethodDesc is that IL-owning variant, explicitly reset
+        // the paired async thunk as well. When pMethod is already the async thunk,
+        // ResetCodeEntryPointForEnC() cascades to the IL-owning variant, so
+        // resetting the paired MethodDesc here would be redundant.
+        if (!pMethod->IsAsyncThunkMethod() && pMethod->HasAsyncOtherVariant())

Conversation

tommcdon commented Mar 10, 2026

Uh oh!

dotnet-policy-service bot commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

noahfalk left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

tommcdon Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

tommcdon Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development