Use registered ComWrappers for object <-> COM interface

[elinor-fung]

Fix #33329

If a global ComWrappers instance is registered, use it for conversion between object and COM interfaces. This includes the Marshal APIs, interface marshaler for p/invokes, and COM activation.

[elinor-fung]

The doc for ComputeVtables says that if it returns null, GetOrCreateComInterfaceForObject will throw ArgumentNullException. However, the existing behaviour was that GetOrCreateComInterfaceForObject would throw ArgumentException if ComputeVtables return a negative count or returned null and a non-zero count. I kept the current behaviour (such that returning no vtables is valid) - should I make it conform to the doc instead?

[elinor-fung]

Chatted with @AaronRobinsonMSFT offline - will update the doc.

[AaronRobinsonMSFT]

    void* wrapperRaw = NULL;

wrapperRawMaybe

---

Refers to: src/coreclr/src/vm/interoplibinterface.cpp:497 in 5d2b7db. [](commit_id = 5d2b7db, deletion_comment = False)

[AaronRobinsonMSFT]

        OBJECTREF objRef;

objRefMaybe?

---

Refers to: src/coreclr/src/vm/interoplibinterface.cpp:613 in 5d2b7db. [](commit_id = 5d2b7db, deletion_comment = False)

[AaronRobinsonMSFT]

@GrabYourPitchforks or @jkotas Can one of you give us a reading on if we should do another API review since we are technically changing the semantics of Marshal.GetIUnknownForObject(), Marshal.GetObjectForIUnknown(), and Marshal.GetUniqueObjectForIUnknown() if a ComWrappers instance has been globally registered? Perhaps at a minimum we may want a new API on ComWrappers that indicates a global instance is registered (e.g. public static bool ComWrappers::IsGlobalIntanceSet()).

[jkotas]

Can one of you give us a reading on if we should do another API review

I do not think another API review is required for this change.

[jkotas]

Should this also kick in for runtime-generated marshaling? E.g. for COM interop used in P/Invoke signatures? Or is there a good reason for not doing that?

[elinor-fung]

Should this also kick in for runtime-generated marshaling? E.g. for COM interop used in P/Invoke signatures?

As in things like MarshalAs with UnmanagedType.IUnknown?
@AaronRobinsonMSFT - thoughts?

[AaronRobinsonMSFT]

Should this also kick in for runtime-generated marshaling? E.g. for COM interop used in P/Invoke signatures?

As in things like MarshalAs with UnmanagedType.IUnknown?
@AaronRobinsonMSFT - thoughts?

This is starting to scare me a bit. I like the idea, but the more I think about it this creates a mechanism that user's can't over come (i.e. if a ComWrappers is globally registered, that will always be called).

@jkotas What are you thoughts on keeping some of the cleanup in this PR, but remove the use of ComWrappers in the existing Marshal APIs for object <-> IUnknown mappings. If we really think we should keep this then I would agree we should update the COM marshaling semantics.

[jkotas]

this creates a mechanism that user's can't over come

This is property of all globally registered things - same concern exists for assembly loading, etc.

The global registrations are powerful. With power comes responsibility: The globally registratered handlers better work well.

[AaronRobinsonMSFT]

With power comes responsibility: The globally registratered handlers better work well.

@jkotas Should we add this to the documentation for that API 😆

@elinor-fung Sounds like we should also do the COM marshaling as well.

[jkotas]

Is it possible to handle all cases? I think this needs to handle all cases in order to be useful.

For example, I can imagine this being useful for statically pre-compiled COM interop carried with the apps.

[jkotas]

I can imagine this being useful for statically pre-compiled COM interop carried with the apps.

This may be a good test case: Take a test or an app that does a bunch of COM interop, and inject the COM interop into it via this so that the build-in COM interop is not used.

[AaronRobinsonMSFT]

If this all encompassing approach is being done now - was hoping for post .NET 5 - then we should also update Marshal.GetIDispatchForObject().

Edit: IDispatch shouldn't be support here.
Edit2: Adding back after offline conversation/convincing this will be okay.

[jkotas]

Doing this after .NET 5 would be a breaking change, so we would need to think twice about the breaking potential, etc.

It would be best to try to make this work the way we want for .NET 5.

[AaronRobinsonMSFT]

I was thinking of the next major version (e.g. .NET 6). I appreciate the breaking change concern, which is why I thought we were limiting this to just the CsWinRT scenarios since learnings would fall out of that and we could fold that into general support. My concern with this now is merely the lack of concrete best practices and we will probably need enhancements (e.g. new enums values) - without some real world learnings I'm not confident the API is ready.

That could be me just being shy of providing such a base building block, but I don't really see the need to get it completely wired up when the only scenario that really needs it is CsWinRT for .NET 5.

Not say that we can't wire all it up, but I don't see the pressing concern to push something right now.

[jkotas]

It is going to be a public API. It is likely that other people are going to show up who will start using it.

If we are not confident in the shape and the behavior of the API, we should ship it as experimental (e.g. similar to Utf8 string) so that it is not present in the officially supported public surface.

[AaronRobinsonMSFT]

Let's just do it. I am not sure how to convey my concerns properly.

[AaronRobinsonMSFT]

Agree with all options except for the IDispatch support. We shouldn't expect the ComWrappers to support that yet since it is so onerous. Perhaps we can address that in the future but for right now I would prefer if we at least block that support.

Edit: I have been convinced this isn't as big a concern as I was making it out to be.

[elinor-fung]

Updated to basically do this for everything now (including COM activation) if there is a ComWrappers registered globally.

[jkotas]

@andrew-boyarshin I see that you are watching this PR. Do you expect this to be useful for something that you are building?

[andrew-boyarshin]

@jkotas yeah, I work with managed graphics APIs and I am dissatisfied with the performance (overhead) of existing DirectX bindings (I am especially interested in DirectWrite and Direct2D). They are usually generated using SharpGen, thus bringing their own COM interop. There is an already existing infrastructure in .NET runtime to do that. I intend to experiment with that to generate an alternative set of bindings, that would use the runtime-provided COM interop. The end goal would be to beat the performance of SharpGen and get as close to unmanaged code perf as possible. This PR feels like something that could enable a .NET 5 way to implement that.
P.S. But I also follow closely the CsWinRT project, even though my past experience with C++/WinRT was rough, to say the least.

[AaronRobinsonMSFT]

@andrew-boyarshin This API isn't very different from the approach taken by SharpGenTools. In fact most of it was modeled off of approaches like SharpGenTools with some optimizations based on the internal CCW/RCW support. I am unclear if this approach is going to be significantly faster than the SharpGenTools.

Also note that the Direct2D APIs are COM in spirit but don't entirely adhere to all the COM rules and thus can cause issues at the ABI level. Tools like SharpGenTools handles these ABI issues properly where as the current built-in CCW/RCW support won't make for a very nice experience.

/cc @jkoritzinsky

[AaronRobinsonMSFT]

@andrew-boyarshin An example of an issue with the built-in system is #10901.

Not trying to push you away from using the built-in system or from using this, but SharpGenTools really does a good job and I would imagine it will eventually use this longer term anyways.

[jkoritzinsky]

As the author of SharpGenTools, I'm planning on moving to use at least some of this infrastructure once I have time again to sit down and really put a lot of time into SharpGenTools.