Skip to content

Do not store result of cast in IDynIntfCastable scenarios#125605

Merged
MichalStrehovsky merged 2 commits intomainfrom
MichalStrehovsky-patch-1
Mar 16, 2026
Merged

Do not store result of cast in IDynIntfCastable scenarios#125605
MichalStrehovsky merged 2 commits intomainfrom
MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky
Copy link
Member

Fixes #125577

Incorrectly copied a piece of logic and comment from CoreCLR but different considerations apply here (we do have an object instance).

Cc @dotnet/ilc-contrib

Refactor cache update logic to avoid unnecessary updates for IDynamicInterfaceCastable types.
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a NativeAOT type-cast caching bug where IDynamicInterfaceCastable.IsInterfaceImplemented results could be incorrectly reused across different instances of the same type, causing is/cast behavior to depend on prior casts.

Changes:

  • Prevent caching of interface cast results when the source type is IDynamicInterfaceCastable (both isinst/is and castclass-style paths).
  • Add a regression test that validates per-instance IsInterfaceImplemented behavior across multiple instances.

Reviewed changes

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

File Description
src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs Adds a regression test to ensure IDynamicInterfaceCastable.IsInterfaceImplemented isn’t effectively cached per type across instances.
src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs Skips updating the cast cache for interface casts involving IDynamicInterfaceCastable, preventing instance-dependent results from being reused.

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

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@MichalStrehovsky
Copy link
Member Author

/ba-g deadletter on mobile

@MichalStrehovsky MichalStrehovsky merged commit b954bd7 into main Mar 16, 2026
109 of 116 checks passed
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch March 16, 2026 21:09
@Sergio0694
Copy link
Contributor

Sergio0694 commented Mar 16, 2026

@MichalStrehovsky could we backport this to .NET 10 since it's breaking CsWinRT in a bunch of scenarios? 😅

Also, thank you for the quick fix!! 😄

@MichalStrehovsky
Copy link
Member Author

@MichalStrehovsky could we backport this to .NET 10 since it's breaking CsWinRT in a bunch of scenarios? 😅

Can you give me more details I can use for shiproom justification? Since this was broken since .NET 9.

@Sergio0694
Copy link
Contributor

Sure! So the TLDR is that CsWinRT 2.x didn't use generic IDIC on AOT, because the supporting code around it relied on a lost of reflection stuff that we couldn't properly make safe, so to keep things deterministic we just disabled that whole support for AOT scenarios. In CsWinRT 3.0 instead since we're rebuilding everything for AOT, we have the new interop code generator that can discover all generic instantiations and generate all necessary IDIC code for constructed generics, such that things will work fine and be fully AOT even when doing stuff like IDIC casts on generic WinRT interfaces. Because CsWinRT 2.x didn't properly support AOT, we also never enabled our unit tests on AOT. We did that work in 3.0 and that's how we noticed this test was actually failing. Also cc. @manodasanW in case he has anything else to add here as justification 🙂

@MichalStrehovsky
Copy link
Member Author

I see, CsWinRT just not using this before was the key detail I was missing.

@MichalStrehovsky
Copy link
Member Author

/backport to release/10.0

@github-actions
Copy link
Contributor

Started backporting to release/10.0 (link to workflow run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.NET AOT caching IDynamicInterfaceCastable.IsInterfaceImplemented results based on type rather than instance

4 participants