[API Proposal]: `ComWrappers` API to only check for an existing RCW

[AaronRobinsonMSFT]

Background and motivation

A current API exists for taking in an IntPtr to an IUnknown and checking if it is a CCW (COM Callable Wrapper). Adding a symmetrical API for checking if there is an existing wrapper for an RCW (Runtime Callable Wrapper) is also potentially interesting since it could be the case that users may want to perform a non-exception throwing check prior to calling ComWrappers.GetOrRegisterObjectForComInstance() or ComWrappers.GetOrCreateObjectForComInstance().

API Proposal

The proposal here is to reuse the TryGetObject() pattern, but for a native IUnknown as opposed to check if the IUnknown is an existing CCW (wrapper of a managed object).

namespace System.Runtime.InteropServices;

public abstract class ComWrappers
{
+    public static bool TryGetObject(IntPtr externalComObject, ComWrappers comWrappers, out object? wrapper);   
}

API Usage

public static MyCallbackApi(IntPtr unknown)
{
   if (!ComWrappers.TryGetObject(unknown, g_comWrapper, out object? existingWrapper))
   {
       //
       // Do some inspection of the unknown and/or perform actions that need to be done
       // prior to actually attempting the creation.
       //
       existingWrapper = g_comWrapper.GetOrCreateObjectForComInstance(unknown, CreateObjectFlags.None);
   }
}

Alternative Designs

Convert to instance method

public bool TryGetObject(IntPtr externalComObject, out object? wrapper);

Follow the existing pattern and create a "Try" version

public bool TryGetOrCreateObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags, [NotNullWhen(true)] out object? wrapper);  // Instance

Risks

This is for a niche WinRT scenario. It has no other obvious uses at present. It does however create a symmetrical API for both CCWs and RCWs.

The API has a trivial implementation, but does introduce an inherent race condition around what is in essence a concurrent dictionary (RCW cache). The "try" operation is checking for state that might be addressed by another thread calling the GetOrCreateObjectForComInstance or GetOrRegisterObjectForComInstance. This can be mitigated if the user's ComWrappers implementation ensures that it would always create a wrapper for an IUnknown instance based solely on the state of that IUnknown, but that is a hard requirement to enforce.

[dotnet-policy-service]

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

[jkotas]

Convert to instance method

You can have multiple different ComWrappers instances wrapping the same external IUnknown. I think this needs to be instance method so that you know which one to use.

[AaronRobinsonMSFT]

Convert to instance method

You can have multiple different ComWrappers instances wrapping the same external IUnknown. I think this needs to be instance method so that you know which one to use.

It doesn't need to be. The proposed API passes in a target ComWrappers instance to use. I would prefer the inspection of this kind of state to be static as opposed to instance. The existing TryGetObject is an inspection function as is TryGetComInstance. Having all of them as static makes explaining where to look easier in my opinion. If we were to make it an instance, the TryGetObject wouldn't be the name I would go with.

[Sergio0694]

Chatted offline with Aaron, leaving context here as well as requested 🙂

"This is for a niche WinRT scenario."

To clarify, this is not actually the API we'd like for CsWinRT, and we would not be using it. We'd like to get #113591, as that'd address all our issues/requirements. I see that version mentioned in the alternative design here as well, but I've opened a separate proposal to track that just for clarity, since it's a different API shape than TryGetObject, and technically speaking both could be added (though as mentioned, specifically for CsWinRT we're interested in #113591).

[jkotas]

The proposed API passes in a target ComWrappers instance to use.

Ah ok, I have missed that.

It is a topic for API review discussion. I think it is very atypical to have a method that operates on an instance state and that takes the logical this as an argument. Is there any prior art in the BCL for API shape like that?

Wrt. discoverability: It depends on where you start. If you are at GetOrCreateObjectForComInstance and looking for a method that does just the Get without creating anything, I think you are likely to look for instance method.

[AaronRobinsonMSFT]

It is a topic for API review discussion.

Agree.

I think it is very atypical to have a method that operates on an instance state and that takes the logical this as an argument. Is there any prior art in the BCL for API shape like that?

I don't know the BCL that well. Something to ask them.

Wrt. discoverability: It depends on where you start. If you are at GetOrCreateObjectForComInstance and looking for a method that does just the Get without creating anything, I think you are likely to look for instance method.

I don't disagree on that opinion. I'm doing this more for consistent location of APIs by utility. If someone has a CCW and wants to unwrap it there is a static function. If someone has an RCW and wants to to see if it has a wrapper I would assume it would be close to the similar non-creating function or CCWs. It is the symmetry for the entire ComWrappers API. I'm interested in the API review discussion.

[AaronRobinsonMSFT]

Chatted offline with Aaron, leaving context here as well as requested 🙂

"This is for a niche WinRT scenario."

To clarify, this is not actually the API we'd like for CsWinRT, and we would not be using it. We'd like to get #113591, as that'd address all our issues/requirements. I see that version mentioned in the alternative design here as well, but I've opened a separate proposal to track that just for clarity, since it's a different API shape than TryGetObject, and technically speaking both could be added (though as mentioned, specifically for CsWinRT we're interested in #113591).

I have serious concerns with the TryGetOrCreateObjectForComInstance API as it imposes constraints on future ComWrappers needs. I'm not covinced it is an API we can deliver given the current way ComWrappers works.

[Sergio0694]

I've clarified them in the other proposal. I think those concerns (which are completely fair) were only due to the name I included in the proposal. But I did mention the exact name/signature can be whatever is best and whatever API review prefers. The proposal is simply to have a GetOrCreateObjectForComInterface that doesn't throw if CreateObject returns null. All other exceptions can remain in place, it's just about allowing a CreateObject implementation to say "I don't actually want to produce an RCW for this object, give null to callers instead".

[AaronRobinsonMSFT]

Closing this issue as it is no longer desired.