Resolve dllimport.cpp runtimeasync todos

[jkoritzinsky]

Roslyn should never mark a P/Invoke or a delegate's Invoke method as an async method, so there's not a good way to add tests without writing them in IL and it doesn't seem particularly useful to do so.

Fixes#121758

[Copilot]

Pull request overview

This PR resolves TODOs related to async method handling in the P/Invoke implementation. The changes replace placeholder TODO comments with explanatory comments about why async methods are not supported for P/Invoke scenarios, convert some assertions to preconditions, and adjust error handling in one case to defer exception throwing to the IL stub generation path.

Key changes:

• Adds PRECONDITION checks for async methods in constructor contracts and function preconditions
• Replaces TODO comments with clarifying comments explaining that async P/Invoke methods should use async variant stubs that call task-returning interop stubs
• Changes MarshalingRequired to return TRUE for async methods (to trigger proper exception later) instead of throwing immediately

[VSadov]

Roslyn should never mark a P/Invoke or a delegate's Invoke method as an async method

Interestingly it allows Synchronized.
Like:

[MethodImpl(MethodImplOptions.Synchronized)][DllImport("kernel32.dll")]privatestaticexternboolQueryPerformanceFrequency(outlongfrequency);

It results in

Unhandled exception. System.TypeLoadException: Synchronized attribute cannot be used with this method type.
at Program.Main()

Perhaps MethodImplOptions.Async should result in a similar error.

In theory we could just ignore Async on DllImport methods, since we spec'd it as

The [MethodImpl(MethodImplOptions.Async)] only has effect when applied to method definitions with CIL implementation.

The runtime could be more strict than the spec and making it an error as it is the case with Synchronized might be more consistent.

[VSadov]

I can think of only two ways to get async involved with DllImport.

• Non-task Async method.

Roslyn does not like adding Async outside of corlib/AsyncHelpers, but should be possible in IL.
Perhaps should produce the same error as with Synchronized

The runtime also does not allow non-task Async methods outside of corlib, so maybe that will error out first.

[MethodImpl(MethodImplOptions.Async)][DllImport("kernel32.dll")]privatestaticexternboolQueryPerformanceFrequency(outlongfrequency);

• Task-returning pinvokes:

[DllImport("kernel32.dll")]privatestaticexternTaskQueryPerformanceFrequency(outlongfrequency);[DllImport("kernel32.dll")]privatestaticexternValueTaskQueryPerformanceCounter(outlongcounter);

I am not sure if either of above can work right now.
Ideally the behavior with async enabled should be the same.

[jkoritzinsky]

@VSadov good point on MethodImpl attribute. I'll add some tests to make sure we don't crash.

[jkoritzinsky]

@VSadov I've rewritten this PR to have the following implementation:

Loading a P/Invoke with the async bit set fails to load.

The async variant for a P/Invoke method (if even usable) will not be considered a P/Invoke (similar to how we handled this for COM interop).

Jitting a method with UnmanagedCallersOnly with the async bit set fails with InvalidProgramException.

dllimport.cpp asserts that it never sees a method with the async bit set.

[VSadov]

The [MethodImpl(MethodImplOptions.Async)] + [DllImport] scenario could be hard to test since Roslyn would not allow MethodImplOptions.Async on random methods.

The scenario like the following is testable though. It is also a preexisting scenario, so regression is a bigger deal. Perhaps we should have a test.

[DllImport("kernel32.dll")]privatestaticexternTaskQueryPerformanceFrequency(outlongfrequency);[DllImport("kernel32.dll")]privatestaticexternValueTaskQueryPerformanceCounter(outlongcounter);

Otherwise looks good to me.

[jkotas]

Thanks