dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.63k stars 4.57k forks source link

How to tell if a Type is "awaitable" in native AOT / trimmed apps #103258

Open eerhardt opened 1 month ago

eerhardt commented 1 month ago

In ASP.NET Core, we have some code that determines if a Type is "awaitable". See

https://github.com/dotnet/aspnetcore/blob/206b0aeca39d5eb12e55ce4e35ef4c8b9bc63c86/src/Shared/ObjectMethodExecutor/AwaitableInfo.cs#L46-L109

(Note that this code has invalid UnconditionalSuppressMessage attributes. These suppressions will be removed in a forth-coming PR.)

There are places in ASP.NET Core that need to do special operations for "awaitable" Types. For example, wrapping the custom awaitable in an ObjectMethodExecutorAwaitable.

However, in doing this, the code needs to do reflection on the Type in such a way that will break in trimmed apps. There doesn't appear to be a way to support custom awaitable Types at runtime. Instead, the only viable solution appears to be to use a source generator.

cc @MichalStrehovsky @agocke @davidfowl @BrennanConroy

MichalStrehovsky commented 1 month ago

The "obvious" fix for this would be to build a feature that looks at GetMethod("SomeConstant") calls and keeps methods named SomeConstant on all the types. The obvious problem with that is both leaving too much size on the table and also:

// This is fully trim safe, in a NuGet package A
class Foo
{
    [RequiresUnreferencedCode]
    void SomeConstant()
    {
        Type.GetType(Console.ReadLine());
    }
}

// This would be fully trim safe on its own, in NuGet package B
class Bar
{
    static void Do() => someUnknownType.GetMethod("SomeConstant").Invoke();
}

// App now gets trimming warnings by using trim safe NuGet A and B because someType in B could be Foo.
// Whose fault is it and how to fix? Both NuGet A and B are trim safe, but the app is not.
new Foo();
Bar.Do();

I can't think of a way to fix this using trimming/reflection facilities alone. Looks like we need an interface.

ericstj commented 1 day ago

I moved this API suggestion out to 10.0 under the assumption that this is not blocking 9.0 scenarios. Let me know if that's incorrect @eerhardt.

eerhardt commented 1 day ago

I moved this API suggestion out to 10.0 under the assumption that this is not blocking 9.0 scenarios. Let me know if that's incorrect @eerhardt.

No, this isn't blocking 9.0 scenarios. SignalR "worked around" this by adding a feature switch to only support Task related types, and not custom awaitables. This feature switch is used by default in trimmed and native AOT applications. See https://github.com/dotnet/aspnetcore/blob/7e99eea5f03a90686267f8cb5b6143d9b44e5ac4/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs#L35-L39 and https://github.com/dotnet/sdk/pull/41832.