Allow caching IDynamicInterfaceCastable results

[MichalStrehovsky]

Resolves #107999.

The caching on CoreCLR VM side is wild. The native AOT cache is per-callsite. So we will have observable differences in how the cache works. I assume anyone who comes to us with "My Gizmo doesn't work for more than one type" where Gizmo is defined as:

class Gizmo : IDynamicInterfaceCastable
{
    public Type InterfaceType { get; init; }
    public Type ImplType { get; init; }

    public RuntimeTypeHandle GetInterfaceImplementation(RuntimeTypeHandle interfaceType)
        => interfaceType.Equals(InterfaceType.TypeHandle) ? ImplType.TypeHandle : throw new Exception();
    public bool IsInterfaceImplemented(RuntimeTypeHandle interfaceType, bool throwIfNotImplemented)
        => interfaceType.Equals(InterfaceType.TypeHandle) ? true : (throwIfNotImplemented ? throw new InvalidCastException() : false);
}

will be sent away, although I can't find docs that we could point them to saying this is illegal. The CoreCLR VM implementation that sends things through a cache doesn't seem to allow that, and neither will native AOT.

Cc @dotnet/ilc-contrib @AaronRobinsonMSFT @jkoritzinsky

[Copilot]

Pull Request Overview

This PR modifies the dispatch cache logic to remove the special-case for IDynamicInterfaceCastable, aiming to allow caching of its results in Native AOT.

• Removed the branch that bypassed cache updates for IDynamicInterfaceCastable
• Simplified the if (pTargetCode != IntPtr.Zero) block

Comments suppressed due to low confidence (1)

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/CachedInterfaceDispatch.cs:30

• By removing the else return pTargetCode branch, calls for IDynamicInterfaceCastable objects now fall through to the fail-fast path instead of returning the resolved target. Restore a direct return pTargetCode; for the dynamic-castable case or refactor so both paths correctly return.

            if (pTargetCode != IntPtr.Zero)

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[jkotas]

Do we need to add some tests?

[MichalStrehovsky]

Do we need to add some tests?

Wouldn't it require building a lot of infrastructure so that we can control when/how the cache gets flushed to get anything reliable? The CoreCLR caching behavior is baffling and making a native AOT specific test for this really doesn't feel like worth the effort.

[MichalStrehovsky]

So the ValidateOverriddenInterface test is failing because the test itself has a Gizmo equivalent from the top-post but just lucks out because the CoreCLR VM's version of the cache is super weird in how it caches things. This took a lot longer to root cause than I would be willing to admit.

I honestly think that the caching here is user-hostile, whoever disagrees, just try to root cause the failing test in a debugger.