wcf icon indicating copy to clipboard operation
wcf copied to clipboard

CallbackBehavior ignored from a base class

Open dspear opened this issue 2 years ago • 3 comments

Describe the bug If you define CallbackBehavior for a callback for a DuplexClient, and then derive another class from that, that CallbackBehavior is not captured by the ServiceEndpoint for the DuplexClientBase. In the system ServiceModel for .Net Framework, this is not a problem. CallbackBehavior is captured in each base class.

To Reproduce Steps to reproduce the behavior:

  1. Define a Callback class, derived from a callback interface.
  2. Specify a CallbackBehavior attribute, such as [CallbackBehavior(ConcurrencyMode=ConcurrencyMode.Multiple)]
  3. Define a Callback class derived from the first, with no CallbackBehavior attribute.
  4. Create a DuplexClient using the derived Callback class as the callback.
  5. The service endpoint that is generated will not have the CallbackBehavior from the base callback class. It will have a default one with ConcurrencyMode.Single set.

Expected behavior CallbackBehavior attributes should be settable anywhere in the class tree.

Additional context I have found the cause of this problem. It is in the TypeLoader.AddBehaviorsFromImplementationType function. The first foreach loop calls ServiceReflector.GetCustomAttributes, with "false" as the 3rd parameter. That parameter indicates whether it should inherit attributes. It should. Changing that parameter to "true" fixes the problem.

I am not submitting a pull request with this fix, in part, because there are many places where GetCustomAttributes is passed false, and I expect that some other places also should be passing true. I have not looked at all of those paths, and I do not have the experience in WCF to know which ones should and which ones shouldn't inherit. I just know that in this case, I had code that worked in .Net Framework and that broke in .Net Core, and this was the problem.

dspear avatar May 13 '22 18:05 dspear

We will provide a fix to alleviate the problem in the near term, and fix this properly in .NET 7.0.

HongGit avatar May 19 '22 20:05 HongGit

I presumed this was a regression from .NET Framework, but after comparing the code with .NET Framework, I checked what the behavior is on .NET Framework with a test app and this is the same behavior as for .NET Framework. Changing this behavior could break existing apps. If someone is accidentally depending on the default behavior of ConcurrencyMode.Single but have a base class specifying Multiple, this change could trigger a latent bug.
I need to put some thought into if there's a way to do this without risking breaking anyone. You have a few options as a workaround. You could derive the class again applying an attribute at the top level specifying the intended behavior. Or you reflect the type yourself to get the instance and then replace the CallbackBehaviorAttribute endpoint behavior:

var callbackBehaviorAttribute = typeof(DerivedCallbackType).GetCustomAttributes(typeof(CallbackBehaviorAttribute), true).Single() as CallbackBehaviorAttribute;
factory.Endpoint.EndpointBehaviors[0] = callbackBehaviorAttribute;

I'm leaving this open for now in case I can find a way to make this change without breaking anyone (as it's a reasonable expectation). One option might be to bring back some of the other mechanisms for providing a callback instance such as just specifying the type. We could make it so if you specify the type and not an instance or InstanceContext, that we search derived types too. That still doesn't feel right though as it's inconsistent behavior depending on which mechanism you use, and is still different from .NET Framework.

mconnew avatar Aug 17 '22 17:08 mconnew

Well, I was definitely getting an error in code that worked fine in .Net Framework 4.8, which was related to this. I did fix it by putting the behavior in the most derived class. I don't know for sure if the actual CallbackBehavior attribute in .Net Framework was different, or if the threading issues are different, where a sequence that caused an error with one CallbackBehavior in .Net 6 did not cause the same error in .Net Framework.

I'm using ConcurrencyMode.Multiple. I actually wanted to use ConcurrencyMode.Reentrant (in order to guarantee the correct order of events), but that is not available in .Net Standard. In some of my callbacks, I spawn another Task and issue calls back into the service. Until I put ConcurrencyMode.Multiple in the top level class, I got an exception saying I needed to use ConcurrencyMode.Multiple, when running under .Net 6. The exact same code, when built under .Net Framework 4.8 using the .Net Framework ServiceModel, does not get an exception and works fine.

dspear avatar Aug 19 '22 19:08 dspear