dotnet / wcf

This repo contains the client-oriented WCF libraries that enable applications built on .NET Core to communicate with WCF services.
MIT License
1.71k stars 558 forks source link

Please make IAsyncCommunicationObject actually public #5269

Open 0xced opened 1 year ago

0xced commented 1 year ago

The System.ServiceModel.IAsyncCommunicationObject interface should be public.

Although IAsyncCommunicationObject is public, it's nowhere in the System.ServiceModel.Primitives.Ref project, making it effectively unavailable to consumers of the NuGet package.

This even confuses JetBrains Rider. 😕

Rider confused by IAsyncCommunicationObject

The files in the System.ServiceModel.Primitives.Ref project all have this line at the top.

Changes to this file must follow the http://aka.ms/api-review process.

Does this still hold true? It seems that recent additions such as the introduction of ITransportCompressionSupport in 2c92580316c5152949f2d4fb2c50e13af56ae8b7 didn't go through the API Review Process.

Should I file an issue in the dotnet/runtime repository or is this very issue enough to discuss making the IAsyncCommunicationObject interface public?

mconnew commented 1 year ago

This is a lot harder than it seems. The difficulty is when an external binding element is used. A binding element creates a ChannelFactory instance which wraps an inner ChannelFactory. Just taking that for example, when you call ChannelFactory.OpenAsync, if one of the chained ChannelFactory instances hasn't implemented OpenAsync, but has implemented BeginOpen, it will call BeginOpen on its own inner ChannelFactory. If that is an internal implementation, then we need to hop over to OpenAsync again. But what if it was an external implementation derived from an internal implementation and we called OpenAsync, we could miss calling an externally overridden BeginOpen method. It took a lot of work to get our internal implementations using the Async variants, but ensure we call the APM Begin/End variants externally. The only way Stream manages to handle this transition is with special CLR internal native helpers to inform it whether methods have been overridden.
The only way to make IAsyncCommunicationObject public would be to immediately deprecate/remove the APM methods, which would be a breaking change. I do want to do this, but it has to happen on a major version number bump. The next major version bump would be 8.0, but there's not time to do this by then. Sadly this means we can't do this until .NET 10 (WCF client from 6.0 is tracking the LTS major version releases)

0xced commented 11 months ago

Well, crossing fingers for .NET 10 then. 🤞