runtime: Marshal.ReleaseComObject does not account for ComWrappers

The challenge with changes like this is testing - how do we know that it actually works?

I think it would preferable to work towards the 100% ComWrappers solution: Pick one of these libraries, figure out how to make wean it off of the built-in COM support by default, and make it happen. Rinse & repeat.

Does System.Speech not appear in search for IL2050 because it uses ComImport attribute for object creation and no built-in COM marshalling is used, just built-in object creation?

No, we choose not to analyze System.Speech for trimmability yet, because it isn’t a very used assembly (it only got supported on .NET Core in the 5.0 time frame). Here is the list of assemblies we ignore for trimmability (for now):

https://github.com/dotnet/runtime/blob/bde0322a9e11fba40a7a65c6d38200d6514efc72/src/libraries/illink-oob.targets#L15-L25

Some of that reasoning is listed here: https://github.com/dotnet/runtime/pull/52272#discussion_r627140855

FYI - the first round of converting System.Drawing.Common to ComWrappers: https://github.com/dotnet/runtime/pull/54636.

For sure! I’ll tag you when I have a PR ready. Probably sometime next week.

@eerhardt For now I would like to shadow you, to see how ComWrappers should be implemented. Can you tag me if you have PR or draft? Is there general consensus how it should be done? I can start poking into System.Data.Odbc or System.Management based on what should be done. Want to limit scope to easiest target first.

@kant2002 I believe that @eerhardt is also looking into System.Drawing.Common. You two might want to sync.

Supporting ComWrappers in APIs like Marshal.ReleaseComObject costs very little and it is in the same spirit as ComWrappers.RegisterForMarshalling API that was also introduce to ease the migration.

I don’t think it costs very little. It is a complicated scenario with a fair bit of nuance.

1. This could only be made to work for a globally registered ComWrappers instance for the marshalling scenario. Which means we would need to detect that one was registered and the object passed in was created with it.
2. We would then call the ReleaseObjects() on the ComWrappers instance and pass in a single object – which is far more expensive than the current API implementation.

The above would mean (1) when and how this works is complicated and (2) the performance relative to the existing behavior (i.e., built-in) is much more and that defeats the purpose of ComWrappers.

I would argue we should separate them at this point and rely on either an IDisposable contract or perhaps something else. I do not see the value in overloading this API when it has a fair bit of complexity that users are bound to get wrong and shouldn’t really be using anyways. The number of times we’ve had to diagnose early release RCWs is too many to count for me and introducing that outside of the ComWrappers API seems going down a road that has already be tread with many lessons learned.

I agree that the ultimate goal should be to get to 100% ComWrappers, but I think it would make sense to update APIs like Marshal.ReleaseComObject to handle ComWrappers to make migration to ComWrappers easier.

Supporting ComWrappers in APIs like Marshal.ReleaseComObject costs very little and it is in the same spirit as ComWrappers.RegisterForMarshalling API that was also introduce to ease the migration.