Fixing COM Aggregation and XAML reference tracking on .NET AOT

[manodasanW]

There were blocks put in place in AOT for COM aggregation and XAML reference tracking wasn't implemented. This PR adds support for both of them by porting the CoreCLR native code that existed to C# for AOT support. With these changes, the ComWrappers API tests and weak reference tests all pass.

Fixes #89713
Fixes #85137

[ghost]

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

Issue Details

---

There were blocks put in place in AOT for COM aggregation and XAML reference tracking wasn't implemented. This PR adds support for both of them by porting the CoreCLR native code that existed to C# for AOT support. With these changes, the ComWrappers API tests all pass. The weak reference tests currently do fail due to that logic isn't ported yet which I am looking in now but starting the PR with the core changes in the meantime.

Fixes #89713
Fixes #85137

Author:	manodasanW
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[jkoritzinsky]

LGTM, but I'll defer to @AaronRobinsonMSFT for final sign-off.

[manodasanW]

Yes we do, the logic as ported checks if there was a previously allocated dependency handle at the slot by checking if it is not default / zero and if so overrides the target and dependent for it rather than allocating a new one.

[AaronRobinsonMSFT]

Thanks @manodasanW. So far so good.

/cc @AustinWise

[Sergio0694]

Just leaving a few thoughts here. This is all looking amazing!! 🎉

[manodasanW]

@jkotas it looks like the build is failing on osx-x64 Release NativeAOT due to the binary size increased. What options do we have here?

  ****************************************************
  * Size test                                        *
  * Size of the executable is   1,780 kB             *
  ****************************************************
  BUG: File size is not in the expected range (1331200 to 1792000 bytes). Did a libraries change regress size of Hello World?

[jkotas]

It means that a lot of the ComWrappers code ends up in the final binary even when none of it is actually used.

The problem is most likely caused by the calls from WeakReference/ComAwareWeakReference. We either need to make all these calls from WeakReference to go through indirections that are initialized only after the Com wrappers tracking is initialized (ie replace the bool that you have added with a bunch of delegates or function pointers, and make sure to do all calls from WeakReference/ComAwareWeakReference via these).

If that does not work for some reason, we may need end up introducing a feature switch to enable this feature in WinUI apps only. https://learn.microsoft.com/en-us/dotnet/core/runtime-config/threading#autoreleasepool-for-managed-threads is a good example of prior art.

[Sergio0694]

"If that does not work for some reason, we may need end up introducing a feature switch to enable this feature in WinUI apps only"

Pretty sure that's what you meant, but just to double check: did you just use WinUI as a reference here but the feature switch would just be a general-purpose feature switch like others in .NET, that anyone could use? Because WinUI 3 apps are not the only applications that would use the new stuff added in this PR. For instance, COM aggregation can be needed by any WASDK app (without WinUI 3) that also happens to have some types deriving from WinRT projection types.

[jkotas]

the feature switch would just be a general-purpose feature switch like others in .NET, that anyone could use?

Correct.

We should investigate first whether this feature switch is actually needed.

[manodasanW]

It means that a lot of the ComWrappers code ends up in the final binary even when none of it is actually used.

The problem is most likely caused by the calls from WeakReference/ComAwareWeakReference. We either need to make all these calls from WeakReference to go through indirections that are initialized only after the Com wrappers tracking is initialized (ie replace the bool that you have added with a bunch of delegates or function pointers, and make sure to do all calls from WeakReference/ComAwareWeakReference via these).

If that does not work for some reason, we may need end up introducing a feature switch to enable this feature in WinUI apps only. https://learn.microsoft.com/en-us/dotnet/core/runtime-config/threading#autoreleasepool-for-managed-threads is a good example of prior art.

Looks like all the tests passed after the callback change. Thanks.

[jkotas]

Some remaining nits

[jkotas]

I do not see this message in CoreCLR anywhere. Is the CoreCLR error handling different? Do we want to make it the same?

[manodasanW]

CoreCLR doesn't have a message, but following the code path, it does throw an ArgumentException in one case and in the other it seems it would hit a NullPointerException from a null access. Given in CoreCLR they are mixing between C# and the C++ layer for where this logic is and the error happens in a more generic function like CallReleaseObjects, I guess they didn't have a specific error message there. But here we are able to.

[jkotas]

@AaronRobinsonMSFT Should we have a test for these paths?

[AaronRobinsonMSFT]

Sure. This code path in CoreCLR is a bit more contrived given the set up, but we can construct a test. Due to how the system is built up it is going to be some effort though.