[API Proposal]: 'ComWrappers.GetOrCreateObjectForComInstance' variant returning 'null' without throwing

[Sergio0694]

Background and motivation

Currently, CsWinRT uses ComWrappers.GetOrCreateObjectForComInstance to produce RCWs for native objects. However, this is inefficient when it comes to values which are "boxed" (ie. IReference<T> and IReferenceArray<T> objects), as in that case what we want to do is to immediately "unbox" them, and return the inner value to the user. To make some examples:

• IReference<Int32> -> we return a boxed int
• IReference<SomeDelegate> -> we return a SomeDelegate instance

However, the only way for us to know which type of object we're dealing with, is to perform some additional work on it (eg. calling GetRuntimeClassName on it, after doing a QueryInterface for IInspectable). This is fine, since we'd need to do this anyway to figure out what managed type to create. The problem is that once we're inside ComWrappers.CreateObject, we have to return some valid instance, or ComWrappers will throw an exception. To avoid the problem, what we're currently doing is to return some dummy "wrapper" object as RCW, and then we immediately unwrap it on the other side.

This works, but it's far from ideal, for the following reasons:

• It adds overhead due to creating this dummy object and having ComWrappers register it, and track it
  • We genuinely don't care about this object at all, so this is purely wasted work
• It forces us to have a ConditionalWeakTable<,> to extend the lifetime of each native object to that of our returned dummy object. This is to avoid "pointer reuse" bugs (where native code could reuse the same pointer address for another object, if we didn't keep it alive). That is, we get:
  • Extra complexity to handle this caching
  • Unnecessary overhead for managing this table, plus creating some managed, finalizable object to release the object
  • We're keeping the native objects alive for literally no reason (we don't need them)
• It also makes our code in CsWinRT more complicated and clunky

All of these problems would go away if only we had a way to return null from our ComWrappers.CreateObject without throwing.

API Proposal

 namespace System.Runtime.InteropServices;

 public abstract class ComWrappers
 {
     public object GetOrCreateObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags);
+    public bool TryGetOrCreateObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags, [NotNullWhen(true)] out object? obj);
 }

This API would allow us to completely fix all issues mentioned above, in the most efficient way possible. Our ComWrappers would be able to simply create the right "unboxed" object where appropriate, stash it in some [ThreadStatic] field, and return null. Then our caller would check for null, and if so, grab the "standalone" object from that field, and return that to callers. Everything else would remain the same. We just need to be able to not throw that exception here.

Suggesting the Try pattern here as it's well established. Other names/signatures would be fine though, eg.:

• Returning object? directly
• Some other name, eg. GetOrCreateObjectForComInstanceOrDefault
• etc.

Basically as long as the semantics is as described, we don't have a strong preference here.

API Usage

// In our CsWinRT marshaller
if (!WindowsRuntimeComWrappers.Instance.TryGetOrCreateObjectForComInstance(unk, flags, out object? obj))
{
    obj = UnboxedValue; // Grab the unboxed value from our '[ThreadStatic]' instead
}

// In WindowsRuntimeComWrappers
protected override object CreateObject(IntPtr externalComObject, CreateObjectFlags flags)
{
    if (CheckIfShouldCreateAnRcw(...))
    {
        return rcw;
    }

    // Otherwise, unbox directly
    UnboxedValue = UnboxTheValue(...);
    return null;
}

Alternative Designs

Something like suggested in #113581. This however would not be ideal in this scenario:

• It'd mean we'd be performing multiple lookups in ComWrappers every single time, which we'd like to avoid
• It'd also mean ComWrappers would need to do a handful of QIs multiple times as well (to get the object identity, twice)
• It'd force us to split our logic to handle marshalling in multiple places across CsWinRT, rather than keeping it in CreateObject

Risks

None. It's just the Try version of an existing API, following a well established pattern (don't throw, and return false/null).

[dotnet-policy-service]

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

[AaronRobinsonMSFT]

None. It's just the Try version of an existing API, following a well established pattern (don't throw, and return false/null).

This is not correct. The current implementation is currently defined as such, but that isn't what would be needed. This is asking for a restriction on any future flag implementation because now we must never throw an exception from this API, but instead return false.

The GetOrCreateObjectForComInstance method is incredibly complex and driven by various flags and how they interact. Forcing a method to reduce that all down to a simple true/false isn't something the interop team is going to accept at present.

[AaronRobinsonMSFT]

This is not correct. The current implementation is currently defined as such, but that isn't what would be needed. This is asking for a restriction on any future flag implementation because now we must never throw an exception from this API, but instead return false.

Additionally, the user would then also need to try and determine why the current combination of inputs and flags resulted in a false. Try semantics for this API are not tenable in my mind given the interaction matrix.

[Sergio0694]

"This is asking for a restriction on any future flag implementation because now we must never throw an exception from this API, but instead return false."

That is not what the proposal is asking. The "try" refers to CreateObject returning null, not to other checks. Exceptions there are fine, the ask is not to prevent those too. This is also why I said the exact signature/name can be whatever is deemed best here, the ask here is just to be able to have CreateObject returning null without throwing. If another name is more appropriate, or whatever else, that's be completely fine for CsWinRT 🙂

[Sergio0694]

To further add to my point being "we don't care about the exact API as long as the semantics are these", eg. this would also work:

 namespace System.Runtime.InteropServices;

 public abstract class ComWrappers
 {
+    protected bool IsRegisterObjectForComInstanceEnabled { get; set; }
 }

That is always set to true before CreateObject is called, and CreateObject can set it to false if needed. If that's done, then ComWrappers would completely skip RegisterObjectForComInstance on the returned object, and just directly pass it back to callers. This would also avoid the concern of the nullable returned object, as the API would keep always returning not-null.

Now, I imagine this property might be weird to add, and that's fair. The point I'm making is just that from CsWinRT's standpoint, we don't have any strict requirements on the exact API, and we can roll which whatever works best for ComWrappers as a whole, as long as we get "some way" of either returning null, or to signal "don't register this RCW, just return it directly".

[AaronRobinsonMSFT]

I've spoken with a representative of the API review team. The general offline feedback has been the following:

• This is COM so level of concern is minimal.
• Throwing from a Try function is acceptable. Many Try APIs exist as prior art.
• The expectation of a Try function is only that when false is returned the behavior is non-ambiguous. In this case CreateObject being null would work.
• Changing the signature to be object? is a breaking change, but there is prior art for that in some circumstances as well. It is an option.
• Creating a new CreateObjectFlags value that indicates the returned value object may be null even though it isn't annocated as such is something to consider. The user is explicilty opting into the behavior and it is clear they are okay ignoring the annotations.

Given the above details I will submit an offline API review for adding a new flag CreateObjectFlags.AllowNullReturn to enable this scenario. I prefer this to creating another function.

[AaronRobinsonMSFT]

Closing this in deference to #113622