[RuntimeAsync] Remove reflection TODOs. Add tests.

[VSadov]

Fixes: #121764
Fixes: #121762

Following up on TODOs. Contributes to: #115094

Added scenarios for other ways to get reflection info on methods.
Checks that we only see the formal definitions, never thunks.

[dotnet-policy-service]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

[VSadov]

TODO: revert the change that enables runtime async before merging

[VSadov]

@MichalStrehovsky - are the new scenarios AOT-compatible?

[Copilot]

Pull request overview

This pull request removes reflection-related TODOs from the RuntimeAsync feature and adds comprehensive tests for async method reflection behavior. The changes enable RuntimeAsync by default and verify that async method variants are properly handled by the reflection APIs.

Key changes:

• Added three new test cases (CurrentMethod, FromStack, EnumerateAll) to validate reflection behavior with async methods
• Enabled RuntimeAsync by default (configuration value changed from 0 to 1)
• Removed completed TODOs and fixed a typo in C++ comments

Reviewed changes

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

Show a summary per file

File	Description
src/tests/async/reflection/reflection.csproj	New test project file with proper structure and TestLibrary reference
src/tests/async/reflection/reflection.cs	Added three new test methods to verify MethodBase.GetCurrentMethod(), StackFrame.GetMethod(), and Type.GetMethods() behavior with async methods
src/tests/async/Directory.Build.targets	Removed property group that was disabling tests for non-nativeaot builds, enabling the tests to run
src/coreclr/vm/runtimehandles.cpp	Fixed typo: "varinat" → "variant" and removed completed TODO comment
src/coreclr/vm/commodule.cpp	Removed completed TODO comment about async variant support
src/coreclr/inc/clrconfigvalues.h	Changed RuntimeAsync default value from 0 to 1, enabling the feature by default

Comments suppressed due to low confidence (2)

src/tests/async/reflection/reflection.cs:346

• The Assert.Equal parameters are in the wrong order. According to xUnit conventions, the expected value should be the first parameter and the actual value should be the second parameter. Currently it's asserting Equal(actual[i], expected[i]) when it should be Equal(expected[i], actual[i]).
  src/tests/async/reflection/reflection.cs:341
• The array initializer has inconsistent indentation. The first element "Boolean Equals(System.Object)" is indented with 12 spaces, but the remaining elements (lines 333-341) are indented with 17 spaces. All elements should have consistent indentation.

[VSadov]

NativeAOT seems to have a bug here

How did reflection get a RuntimeMethodHandle for an async variant?
   at System.Diagnostics.DebugProvider.Fail(String, String) + 0x36
   at System.Diagnostics.Debug.Fail(String, String) + 0x32
   at Internal.Runtime.TypeLoader.TypeLoaderEnvironment.TryGetRuntimeMethodHandleComponents(RuntimeMethodHandle, RuntimeTypeHandle&, QMethodDefinition&, RuntimeTypeHandle[]&) + 0x6c
   at Internal.Reflection.Augments.ReflectionAugments.GetMethodFromHandle(RuntimeMethodHandle) + 0x65
   at async!<BaseAddress>+0x3055aa
   at async!<BaseAddress>+0x2fd22e
   at System.Runtime.CompilerServices.AsyncHelpers.RuntimeAsyncTask`1.DispatchContinuations() + 0x13e
   at System.Threading.ThreadPoolWorkQueue.Dispatch() + 0x196
   at System.Threading.WindowsThreadPool.DispatchCallback(IntPtr, IntPtr, IntPtr) + 0x7d
Expected: 100

[MichalStrehovsky]

NativeAOT seems to have a bug here

Yep, MethodBase.GetCurrentMethod is annotated as RequiresUnreferencedCode, so it can be broken with native AOT (we generate a warning, and therefore the API might not work), but this is fixable. I filed #122546 on it, let's ActiveIssue this new test on that.

@MichalStrehovsky - are the new scenarios AOT-compatible?

StackFrame.GetMethod is also annotated as RequiresUnreferencedCode so it may not work (and will not). However, we have DiagnosticMethodInfo.Create(StackFrame) API that is trim safe. On CoreCLR it simply builds on top of StackFrame.GetMethod. If you update the test to use that API, we can ActiveIssue it on #122547. If it stays as StackFrame.GetMethod, it's unsuspportable and the test should not run on native AOT.

[MichalStrehovsky]

What's the purpose of this roundtripping?

[VSadov]

The test kind of "evolved" from initially using a hardcoded name of the class. There is no real reason. I will just use typeof.

[MichalStrehovsky]

Are the stack frames of thunks observable on CoreCLR? I.e. what would happen if this is called from an unawaited call and we construct new StackFrame(1)?

[VSadov]

Are the stack frames of thunks observable on CoreCLR? I.e. what would happen if this is called from an unawaited call and we construct new StackFrame(1)?

I do not know if thunk frames are present in the stack trace. I will check.
Just the presence of an extra infrastructure frame would not bother me hugely though. async1 can have like 4 or 5 extra frames around await which are meaningless to the user.

The main concern is to not expose variants with async calling convention. For the purpose of reflection identity the runtime async methods have just one form - the Task-returning form that is in the metadata.

The following would seem acceptable or at least not blocking for enabling of the feature:

• thunk frames are not in the trace.
• thunk frames are in the trace but report the MethodInfo for the "real" method.
• thunk frames are in the trace but report null for MethodInfo

It is worth adding a test for StackFrame(1) though, Thanks!

I also wonder what we get for the resume frame.