0
\$\begingroup\$

I have an ASP.NET web-client and many WCF services. I used it this way:

public class UserController
{
 private Service1Client _service1Client = new Service1Client(); 
 private Service2Client _service2Client = new Service2Client();
 public JsonResult HasDebts()
 {
 var user = _service1Client.GetUserByUsername(User.Identity.Name);
 return Json(_service2Client.HasDebts(user.Id));
 }
}

Recently, I have learned that all proxy clients should be closed or aborted after usage.
Now, I am trying to implement a universal generic service caller. At first, I have implemented a generic caller:

public static class ServiceCaller
{
 private static void CloseProxyProperly<T>(T serviceProxy)
 where T : ICommunicationObject, new()
 {
 if (serviceProxy.State == CommunicationState.Faulted)
 serviceProxy.Abort();
 else
 serviceProxy.Close();
 }
 public static void Use<T>(Action<T> action)
 where T : ICommunicationObject, new()
 {
 T serviceProxy = new T();
 try
 {
 action(serviceProxy);
 }
 finally
 {
 CloseProxyProperly(serviceProxy);
 }
 }
 public static void Use<T1, T2>(Action<T1, T2> action)
 where T1 : ICommunicationObject, new()
 where T2 : ICommunicationObject, new()
 {
 T1 serviceProxy1 = new T1();
 T2 serviceProxy2 = new T2();
 try
 {
 action(serviceProxy1, serviceProxy2);
 }
 finally
 {
 CloseProxyProperly(serviceProxy1);
 CloseProxyProperly(serviceProxy2);
 }
 }
 public static void Use<T1, T2, T3>(Action<T1, T2, T3> action)
 where T1 : ICommunicationObject, new()
 where T2 : ICommunicationObject, new()
 where T3 : ICommunicationObject, new()
 {
 T1 serviceProxy1 = new T1();
 T2 serviceProxy2 = new T2();
 T3 serviceProxy3 = new T3();
 try
 {
 action(serviceProxy1, serviceProxy2, serviceProxy3);
 }
 finally
 {
 CloseProxyProperly(serviceProxy1);
 CloseProxyProperly(serviceProxy2);
 CloseProxyProperly(serviceProxy3);
 }
 }
 public static TResult Use<T, TResult>(Func<T, TResult> func)
 where T : ICommunicationObject, new()
 {
 T serviceProxy = new T();
 TResult result;
 try
 {
 result = func(serviceProxy);
 }
 finally
 {
 CloseProxyProperly(serviceProxy);
 }
 return result;
 }
 public static TResult Use<T1, T2, TResult>(Func<T1, T2, TResult> func)
 where T1 : ICommunicationObject, new()
 where T2 : ICommunicationObject, new()
 {
 T1 serviceProxy1 = new T1();
 T2 serviceProxy2 = new T2();
 TResult result;
 try
 {
 result = func(serviceProxy1, serviceProxy2);
 }
 finally
 {
 CloseProxyProperly(serviceProxy1);
 CloseProxyProperly(serviceProxy2);
 }
 return result;
 }
 public static TResult Use<T1, T2, T3, TResult>(Func<T1, T2, T3, TResult> func)
 where T1 : ICommunicationObject, new()
 where T2 : ICommunicationObject, new()
 where T3 : ICommunicationObject, new()
 {
 T1 serviceProxy1 = new T1();
 T2 serviceProxy2 = new T2();
 T3 serviceProxy3 = new T3();
 TResult result;
 try
 {
 result = func(serviceProxy1, serviceProxy2, serviceProxy3);
 }
 finally
 {
 CloseProxyProperly(serviceProxy1);
 CloseProxyProperly(serviceProxy2);
 CloseProxyProperly(serviceProxy3);
 }
 return result;
 }
}

Usage:

var hasDebts = ServiceCaller.Use<Service1Client, Service2Client, bool>((svc1, svc2) => {
 var user = svc1.GetUserByUsername(User.Identity.Name);
 return svc2.HasDebts(user.Id);
});

I think that you will not be surprised if I tell you that I don't like it:

  • code repetition
  • inconvenient, three-storey, non-inferrable generic types
  • makes it more difficult to read, debug and support code

I also tried to implement IDisposable class with singleton clients:

public class ServiceCaller : IDisposable 
{
 private Service1Client _service1Client;
 public Service1Client Service1Client { 
 get {
 return _service1Client ?? (_service1Client = new Service1Client()); }
 }
 }
 // ... other clients
 public void Dispose()
 {
 CloseProxyProperly(_service1Client); // with check for null inside
 // ... other clients
 } 
}
using (var sc = new ServiceCaller())
{
 var user = sc.Service1Client.FindUserByUsername(User.Identity.Name);
 return sc.Service2Client.HasDebts(user.Id);
}

However, it doesn't reduce code complexity and repetition. Moreover, if I add a new service reference to a project, I will have to modify this class - it sounds wrong.

The third guess it to make an extension and call it manually in ASP.NET:

public static class WcfServiceExtensions
{
 public static void SafeClose(this ICommunicationObject obj)
 {
 if (serviceProxy.State == CommunicationState.Faulted)
 serviceProxy.Abort();
 else
 serviceProxy.Close();
 }
}
public JsonResult HasDebts()
{
 var service1Client = new Service1Client();
 var service2Client = new Service2Client();
 var user = service1Client.GetUserByUsername(User.Identity.Name);
 var result = service2Client.HasDebts(user.Id);
 service1Client.SafeClose();
 service2Client.SafeClose();
 return result;
}

There is a code repetition. Again.

How do people solve this problem? What is the best way to wrap a WCF proxy call and close it properly?

asked Oct 26, 2015 at 8:15
\$\endgroup\$
1
  • \$\begingroup\$ If you control the WCF code and contract, try moving to a model as described here. It makes it really easy to create a single proxy class (with just a few lines of code) that correctly closes the client in the background without your application code to know about it, and gives many other advantages. \$\endgroup\$ Commented Oct 26, 2015 at 10:21

1 Answer 1

2
\$\begingroup\$

You should let the Service1Client and Service2Client (and the remaining clients) implement the IDisposable interface and let the Dispose() method take care about the closing of the proxy.

This would lead the UserController looking like

public class UserController
{
 public JsonResult HasDebts()
 {
 using (var _service1Client = new Service1Client())
 using (var _service2Client = new Service2Client())
 {
 var user = _service1Client.GetUserByUsername(User.Identity.Name);
 return Json(_service2Client.HasDebts(user.Id));
 }
 }
} 

If those clients all share the same base object you could consider to let the base class implement IDisposable.

Based on this comment

Unfortunatelly, these service clients are generated by Visual Studio. I can't change it and any changes will be overriden when I update service references

Because the generated service clients are System.ServiceModel.ClientBase<TChannel> objects which implements IDisposable by default and the underlaying Dispose() method just calls Close(), you can use the using statement like shown above.

Edit based on latest comment stating

Dispose() calls the proxy close() method, that sends a message from the client to the service indicating that the connection session is no longer needed. A problem can arise with this approach if there is an exception when calling the close() method. This is why the using approach is not recommended when calling WCF methods.

From do-not-use-using-in-wcf-client

There are various reasons for which the underlying connection can be at broken state before the using block is completed and the .Dispose() is called. Common problems like network connection dropping, IIS doing an app pool recycle at that moment, some proxy sitting between you and the service dropping the connection for various reasons and so on.

So adding a extension method could be the way to go but a wrapper arround the client would be better (taken from the comments of the above link) like so

public class ServiceClientSafeDisposingWrapper<T> : IDisposable where T : ICommunicationObject
{
 private readonly T _serviceClient;
 public T Client
 {
 get { return _serviceClient; }
 }
 public ServiceClientSafeDisposingWrapper(T serviceClient)
 {
 if (serviceClient == null) { throw new ArgumentNullException("serviceClient"); }
 _serviceClient = serviceClient;
 }
 public void Dispose()
 {
 if (_serviceClient.State == CommunicationState.Faulted)
 {
 _serviceClient.Abort();
 }
 else if (_serviceClient.State != CommunicationState.Closed)
 {
 _serviceClient.Close();
 }
 }
}

which then can be used like so

public class UserController
{
 public JsonResult HasDebts()
 {
 using (var _service1Client = new ServiceClientSafeDisposingWrapper<Service1Client>(new Service1Client()))
 using (var _service1Client = new ServiceClientSafeDisposingWrapper<Service2Client>(new Service2Client()))
 {
 var user = _service1Client.Client.GetUserByUsername(User.Identity.Name);
 return Json(_service2Client.Client.HasDebts(user.Id));
 }
 }
}

Maybe enclosing the action in Dispose inside a try..catch should be done too.

answered Oct 26, 2015 at 8:30
\$\endgroup\$
2
  • \$\begingroup\$ Unfortunatelly, these service clients are generated by Visual Studio. I can't change it and any changes will be overriden when I update service references. \$\endgroup\$ Commented Oct 26, 2015 at 8:34
  • \$\begingroup\$ using statement of client proxies is not recommented be either MSDN and "Programming WCF Services" author. "Dispose() calls the proxy close() method, that sends a message from the client to the service indicating that the connection session is no longer needed. A problem can arise with this approach if there is an exception when calling the close() method. This is why the using approach is not recommended when calling WCF methods." codeproject.com/Articles/622989/… \$\endgroup\$ Commented Oct 26, 2015 at 8:59

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.