[Default interfaces] Interaction with COM interop

[MichalStrehovsky]

Things like what if a COM interface is implemented by default interface methods. Or what if ICustomMarshaler is implemented by that. Or who knows what else.

[luqunl]

@MichalStrehovsky , is this for X-plat or Windows only?

[MichalStrehovsky]

@luqunl Wherever COM is supported on CoreCLR and a user could pass an interface with a default interface method in it.

[MichalStrehovsky]

The POR is going to be to disallow default interface methods with anything COM related. We won't allow loading such types. This should be disallowed in similar places where we disallow generics in COM.

[luqunl]

@MichalStrehovsky , what does POR stand for?

[MichalStrehovsky]

Plan of record

[AaronRobinsonMSFT]

This should be disallowed in similar places where we disallow generics in COM.

Please don't do this until we have some tests in place. We need to see the scenario crash/succeed prior to doing this or else we will have a huge deficit in scenario comprehension.

[jkotas]

Does the milestone on this need to be changed to 3.0?

[AaronRobinsonMSFT]

@jkotas That is a good question. I don't know if default interfaces is slated for 3.0 or not. We should be able to add COM testing for this in that timeframe, but if we decide to add support that is unknowable until we see how badly this fails in a COM scenario.

[MichalStrehovsky]

@AaronRobinsonMSFT in .NET Core 3 per the email thread.

[AaronRobinsonMSFT]

@MichalStrehovsky I have added a simple test for Default Interfaces at dotnet/coreclr#23195. I didn't get very far before hitting an issue. There are other scenarios that need to be tested, but we have a failure as soon as an interfaces tries to provide an implementation of another interface's method. The basic scenario - an interface with a default implementation just works though and is now being tested through COM.

cc @davidwrighton

[AaronRobinsonMSFT]

Another scenario that will probably also fail is interspersed non-virtual functions in an interface definition.

[AaronRobinsonMSFT]

I had to leave so I didn't get a chance to see what was crashing only that it did. I commented out the offending call in the test https://github.com/dotnet/coreclr/blob/5b8b57574f3bdba8d2fd7f0d48b6aaa17e0fc23c/tests/src/Interop/COM/NativeClients/DefaultInterfaces/DefaultInterfacesTests.cpp#L77-L82

Implementation for the offending call is in
https://github.com/dotnet/coreclr/blob/5b8b57574f3bdba8d2fd7f0d48b6aaa17e0fc23c/tests/src/Interop/COM/NETServer/NETServer.DefaultInterfaces.il#L44-L65

[AaronRobinsonMSFT]

Another scenario that will probably also fail is interspersed non-virtual functions in an interface definition.

@MichalStrehovsky I added some of these functions to the COM test and it worked, so this is also good. The only real issue blocking managed COM server consumption from native (CCW) is the redefinition of an existing default interface function. I have not written an RCW scenario so that is something that needs to happen at some point. We should keep this issues open to track RCW testing and the CCW issue mentioned above.

[MichalStrehovsky]

I spent some time looking at the failure in the commented out test referenced above.

I tracked the bad HRESULT coming from the default interface method call to incorrectly constructed interop vtable (likely around BuildInteropVTable_PlaceMembers).

It seems like there’s a parallel method resolution algorithm here that tries to build a static vtable for interop purposes upfront. This is different from how interface calls are done in the VM (they go through VSD and there’s no actual slots in play).

The one default interface method call in the test that succeeds (where the implementation and declaration are in the same method) seems to only work by accident (the result would be wrong if there’s an override in a different interface). The failing test (that calls a default method where the implementation is provided by a different interface) fails for good reasons.

We probably could replicate the default interface resolution algorithm here (however disgusting it is to duplicate a very sensitive piece of logic), but it would immensely increase the test cost for the feature (we now need to make sure the two algorithms really match). I also expect this to be problematic when the resolution needs to result in an exception (like in the diamond case).

With this in mind, I suggest scoping default interface methods in COM out, and turning them into an early throw (same as e.g. generic interfaces get rejected), like originally planned.