microsoft / vs-servicehub

The service broker libraries used by Visual Studio to exchange intra- and inter-process services
MIT License
20 stars 8 forks source link

Improve communication of optional interfaces on brokered services #225

Closed AArnott closed 2 weeks ago

AArnott commented 1 month ago

Leverage RpcMarshalableOptionalInterfaceAttribute and RemoteServiceConnectionInfo to communicate optional interfaces on brokered services

With some enhancements to ServiceJsonRpcDescriptor, we could probably teach it to consider the RpcMarshalableOptionalInterfaceAttribute attributes applied to a brokered service interface. If it could communicate these remotely over the RemoteServiceConnectionInfo (which feels a bit hacky, but is the struct that the service broker shares with the client prior to RPC being set up), then the original proxy that the client receives could have the right set of optional interfaces built in. Or if the proxy generator doesn't (yet?) support multiple interfaces on the same proxy, we could at least throw when asked to produce a proxy for a service that we believe isn't available instead of blindly creating it.

The IJsonRpcLocalProxy generated proxies could use this same attribute list to know which interfaces to test for on the original service object in order to make casting of the proxy work too. That would finally allow us to achieve classic interface casting that works equally well for local and remote services.

AArnott commented 1 month ago

Alternative (preferred) design:

No reuse of the RpcMarshalableOptionalInterfaceAttribute from StreamJsonRpc. Instead, StreamJsonRpcDescriptor might have a property added that contains a map of int->Type. Whenever an RPC target object is given, it is tested against the Types in this map and any that are implemented by the object get the int added to an array of integers to be sent to the client. The int array is still sent by way of a new field on the RemoteServiceConnectionInfo struct as before. A similarly initialized descriptor on the client will then reverse the process, by reviewing this int[] and instructing the generated proxy to implement any interface whose int appears in the array. As is the case today, the proxy will always be generated with the original interface requested by the client, regardless of whether its type appears in the int[] array list or the dictionary property on the descriptor itself.

A risk to the above design is that it relies on the RemoteServiceConnectionInfo being created on the server only after the service factory has run. In ServiceHub, I suspect this to not be the case. If we cannot guarantee the ordering required by the above design, an alternative is that the service registration itself declare what the optional interfaces will be. If so, the server-side broker could either remote this information back to the client via the same proposed struct, or if the client already has access to the same registration, the client could leverage that directly with no additional support from the server side. If we need the metadata in the registration, MEF exported parts could participate automatically by updating the MEF-ServiceBroker bridge to reflect over the MEF parts. But that would require loading those types, defeating the lazy load nature of both MEF and the service broker. MEF metadata would need to explicitly list the implemented types. I suspect our custom ExportBrokeredServiceAttribute might even be able to produce this list during MEF catalog creation time so that no extra work on the part of the brokered service is required.

BertanAygun commented 3 weeks ago

Just saw this so wanted to add the Gladstone perspective where registration happens via json file. In those cases, we can add the interface information to the registration itself since we already generate service entries by looking at the type. So client (devenv) would know which interfaces that brokered service were to implement.

AArnott commented 2 weeks ago

The finishing piece of this is in this Visual Studio pull request.