views:

3966

answers:

7

I like instantiating my WCF service clients within a using block as it's pretty much the standard way to use resources that implement IDisposable:

using (var client = new SomeWCFServiceClient()) 
{
    //Do something with the client 
}

But, as noted in this MSDN article, wrapping a WCF client in a using block could mask any errors that result in the client being left in a faulted state (like a timeout or communication problem). Long story short, when Dispose() is called, the client's Close() method fires, but throws and error because it's in a faulted state. The original exception is then masked by the second exception. Not good.

The suggested workaround in the MSDN article is to completely avoid using a using block, and to instead instantiate your clients and use them something like this:

try
{
    ...
    client.Close();
}
catch (CommunicationException e)
{
    ...
    client.Abort();
}
catch (TimeoutException e)
{
    ...
    client.Abort();
}
catch (Exception e)
{
    ...
    client.Abort();
    throw;
}

Compared to the using block, I think that's ugly. And a lot of code to write each time you need a client.

Luckily, I found a few other workarounds, such as this one on IServiceOriented. You start with:

public delegate void UseServiceDelegate<T>(T proxy); 

public static class Service<T> 
{ 
    public static ChannelFactory<T> _channelFactory = new ChannelFactory<T>(""); 

    public static void Use(UseServiceDelegate<T> codeBlock) 
    { 
        IClientChannel proxy = (IClientChannel)_channelFactory.CreateChannel(); 
        bool success = false; 
        try 
        { 
            codeBlock((T)proxy); 
            proxy.Close(); success = true; 
        } 
        finally 
        { 
            if (!success) 
            { 
                proxy.Abort(); 
            } 
        } 
     } 
}

Which then allows:

Service<IOrderService>.Use(orderService => 
{ 
    orderService.PlaceOrder(request); 
}

That's not bad, but I don't think it's as expressive and easily understandable as the using block.

The workaround I'm currently trying to use I first read about on blog.davidbarret.net. Basically you override the client's Dispose() method wherever you use it. Something like:

public partial class SomeWCFServiceClient : IDisposable
{
    void IDisposable.Dispose() 
    {
        if (this.State == CommunicationState.Faulted) 
        {
            this.Abort();
        } 
        else 
        {
            this.Close();
        }
    }
}

This appears to be able to allow the using block again without the danger of masking a faulted state exception.

So, are there any other gotchas I have to look out for using these workarounds? Has anybody come up with anything better?

+7  A: 

I wrote a higher order function to make it work right. We've used this in several projects and it seems to work great. This is how things should have been done from the start, without the "using" paradigm or so on.

You can make calls like this:

int a = 1; 
int b = 2; 
int sum = UseService((ICalculator calc) => calc.Add(a, b)); 
Console.WriteLine(sum);

This is pretty much just like you have in your example. In some projects, we write strongly typed helper methods, so we end up writing things like "Wcf.UseFooService(f=>f...)".

I find it quite elegant, all things considered. Is there a particular problem you encountered?

Edit: I should add, this allows other nifty features to be plugged in. For instance, on one site, the site authenticates to the service on behalf of the logged in user. (The site has no credentials by itself.) By writing our own "UseService" method helper, we can configure the channel factory the way we want, etc. We're also not bound to using the generated proxies -- any interface will do.

MichaelGG
Thanks, that's very interesting.
Eric King
+1 for a very smart aproach, that is not lacking the possibility to return values from the service operation
Marc Wittke
+2  A: 

Marc Gravell wrote an interesting blog post dealing with this issue.

LukeH
+15  A: 

Actually, although I blogged (see Luke's answer), I think this is better than my IDisposable wrapper. Typical code:

Service<IOrderService>.Use(orderService=>
{
  orderService.PlaceOrder(request);
}

(edit per comments)

Since Use returns void, the easiest way to handle return values is via a captured variable:

int newOrderId = 0; // need a value for definite assignment
Service<IOrderService>.Use(orderService=>
{
  newOrderId = orderService.PlaceOrder(request);
}
Console.WriteLine(newOrderId); // should be updated
Marc Gravell
Thanks Marc. I'll have to give this one a second look.
Eric King
This is a good solution to the problem.
RichardOD
@Kamarey - done
Marc Gravell
@Marc Gravell: I must really need coffee or something, but what's with the `new ChannelFactory<T>("")` bit in that code? That doesn't give me a valid channel factory?
Thorarin
@Thorarin - where? on "iserviceoriented"? Not my code, so I can't be sure, but I *believe* that should work **if** you have a matching block (for the given `T`) in your app.config.
Marc Gravell
hey, what if orderService had a return value? how would I modify the use function to return something?
KevinDeus
@Kevin : Does it return `void`? If so I'd use a capture:
Marc Gravell
(updated to show)
Marc Gravell
Missing a closing ); on the 4th/5th lines respectively? I can't edit yet...
Rawling
A: 

What do you guys think about this solution; Our system architecture often uses the Unity IoC framework to create instances of ClientBase so theres no sure way to enforce the other developers even use using{} blocks. In order to make it as fool-proof as possible, I had made this custom class that extends ClientBase, and handles closing down the channel on dispose, or on finalize in case someone doesn't explicitly dispose of the Unity created instance.

There is also stuff that needed to be done in the constructor to set up the channel for custom credentials and stuff, so thats in here too...

Thoughts / concerns / opinions?

public abstract class PFServer2ServerClientBase<TChannel> : ClientBase<TChannel>, IDisposable where TChannel : class
{
 private bool disposed = false;

 public PFServer2ServerClientBase()
 {
  // copy info from custom identity into credentials, and other channel setup...
 }

 ~PFServer2ServerClientBase()
 {
  this.Dispose(false);
 }

 void IDisposable.Dispose()
 {
  this.Dispose(true);
  GC.SuppressFinalize(this);
 }

 public void Dispose(bool disposing)
 {
  if (!this.disposed)
  {
   try
   {
     if (this.State == CommunicationState.Opened)
      this.Close();
   }
   finally
   {
    if (this.State == CommunicationState.Faulted)
     this.Abort();
   }
   this.disposed = true;
  }
 }
}

Then a client can simply:

internal class TestClient : PFServer2ServerClientBase<ITest>, ITest
{
    public string TestMethod(int value)
    {
        return base.Channel.TestMethod(value);
    }
}

And the caller can do any of these:

public SomeClass
{
    [Dependency]
    public ITest test { get; set; }

    // not the best, but should still work due to finalizer.
    public string Method1(int value)
    {
        return this.test.TestMethod(value);
    }

    // the good way to do it
    public string Method2(int value)
    {
        using(ITest t = unityContainer.Resolve<ITest>())
        {
            return t.TestMethod(value);
        }
    }
}
rally25rs
You never make use of the parameter disposing in your Dispose method
Chad
@Chad - I was following Microsoft's common Finalize/Dispose design pattern: http://msdn.microsoft.com/en-us/library/b1yfkh5e%28VS.71%29.aspx It is true that I am not using the variable though, because I don't need to do any different cleanup between a normal dispose and a finalize. It could be rewritten to just have Finalize call Dispose() and move the code from Dispose(bool) to Dispose().
rally25rs
A: 

I have addressed this problem in an open source effort called SoftwareIsHardwork.ServiceModelClient library. SoftwareIsHardwork.ServiceModelClient is a thread-safe, scalable, and functional replacement for the WCF clients ClientBase1 and/or ChannelFactory1. It beats the socks off any implementation I have seen on the internet; even those by so called WCF experts. I will let you be the judge.

Please see the following links:

link text

link text

link text

D. P. Bullington
+7  A: 

Given a choice between the solution advocated by IServiceOriented.com and the solution advocated by David Barret's blog, I prefer the simplicity offered by overriding the client's Dispose() method. This allows me to continue to use the using() statement as one would expect with a disposable object. However, as @Brian pointed out, this solution contains a race condition in that the State might not be faulted when it is checked but could be by the time Close() is called, in which case the CommunicationException still occurs.

So, to get around this, I've employed a solution that mixes the best of both worlds.

void IDisposable.Dispose()
{
    bool success = false;
    try {
        if (State != CommunicationState.Faulted) {
            Close();
            success = true;
        }
    } finally {
        if (!success) {
            Abort();
        }
    }
}
Matt Davis
+1  A: 

i've finally found some solid steps towards a clean solution to this problem.

This custom tool extends WCFProxyGenerator to provide an exception handling proxy. It generates an additional proxy called ExceptionHandlingProxy<T> which inherits ExceptionHandlingProxyBase<T> - the latter of which implements the meat of the proxy's functionality. The result is that you can choose to use the default proxy that inherits ClientBase<T> or ExceptionHandlingProxy<T> which encapsulates managing the lifetime of the channel factory and channel. ExceptionHandlingProxy respects your selections in the Add Service Reference dialog with respect to asynchronous methods and collection types.

Codeplex has a project called Exception Handling WCF Proxy Generator, it basically installs a new custom tool to visual studio 2008, then use this tool to generate the new service proxy (Add service reference), it has some nice functionality to deal with faulted channels, timeouts and safe disposal. There's an excellent video here called ExceptionHandlingProxyWrapper explaining exactly how this works.

You can safely use the Using statement again, and if the channel is faulted on any request (TimeoutException or CommunicationException), the Wrapper will re-initialize the faulted channel and retry the query, if that fails then it will call the Abort() command and dispose of the proxy and rethrow the Exception, if the service throws a FaultException code will stop executing and the proxy will be aborted safely throwing the correct exception as expected.

Neil
Very nice, thanks. I'll have to take a look at it.
Eric King