none
Why does ClientBase Dispose need to throw on faulted state? (Or, what's the difference between close and abort?)

    Question

  • Could someone enlighten me as to why calling Dispose on a faulted client proxy needs to throw an CommunicationObjectFaultedException instead of just aborting the client? 

    Essentially we have to do this:

    Client c = new Client();
    try
    {
     c.Foo();
     c.Close();
    }
    catch (Exception)
    {
     c.Abort();
     throw;
    }

    Instead of:
    using(Client c = new Client())
    {
     c.Foo();
    }

    Yes, I've read the justifications in the SDK and here: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=627970&SiteID=1 but the "summary of internal discussion" seems to be "well, that's what we're doing, so deal with it."

    I still don't understand why Close must throw instead of just, well, closing? I know I'm going to have to live with it, but I just really want to fully understand what Close does that's so magic. What's the difference between close and abort? Couldn't the WCF team just change the implementation of Dispose on ClientBase<T> from "this.Close()" to "this.Abort()"?

    (Finally, grasping at straws :), the Design Guidelines for Developing Class Libraries seems to indicate Dispose really should not throw, depending on how you read it...)

    Tuesday, October 24, 2006 7:08 PM

Answers

  • Here is more detail from the referenced "interal discussion", which provides some historical context as well.  I think a key thing to notice is that Close() often implies doing "real work" that may fail, including network communication handshakes to shutdown sessions, committing transactions, etc.

     

    See also

    http://windowssdk.msdn.microsoft.com/en-us/library/aa355056.aspx

     

     

    ICommunicationObject (from which ServiceHost, ClientBase, IChannel, IChannelFactory, and IChannelListener ultimately derive) has always had two methods for shutting down the object: (a) Close, and (b) Abort.  The semantics are that if you want to shutdown gracefully, call Close otherwise to shutdown ungracefully you call Abort. 

     

    As a consequence, Close() takes a Timeout and has an async version (since it can block), and also Close() can throw Exceptions. Documented Exceptions out of Close are CommunicationException (of which CommunicationObjectFaultedException is a subclass), and TimeoutException.

     

    Abort() conversely is not supposed to block (or throw any expected exceptions), and therefore doesn’t have a timeout or an async version.

     

    These two concepts have held from the inception of Indigo through today. So far, so good.

     

    In its original incarnation, ICommunicationObject : IDisposable.  As a marker interface, we thought it would be useful to notify users that the should eagerly release this object if possible. This is where the problems begin. 

     

    Until Beta 1, we had Dispose() == Abort().  Part of the reasoning was that Dispose() should do the minimum necessary to clean up.  This was possibly our #1 complaint in Beta 1. Users would put their channel in a using() block, and any cached messages waiting to be flushed would get dropped on the floor. Transactions wouldn’t get committed, sessions would get ACKed, etc.

     

    Because of this feedback, in Beta 2 we changed our behavior to have Dispose() ~= Close(). We knew that throwing causes issues (some of which are noted on this thread), so we made Dispose try to be “smart”. That is, if we were not in the Opened state, we would under the covers call Abort(). This has its own set of issues, the topmost being that you can’t reason about the system from a reliability perspective. Dispose can still throw, but it won’t _always_ notify you that something went wrong.  Ultimately we made the decision that we needed to remove IDisposable from ICommunicationObject.  After much debate, IDisposable was left on ServiceHost and ClientBase, the theory being that for many users, it’s ok if Dispose throws, they still prefer the convenience of using(), and the marker that it should be eagerly cleaned up.  You can argue (and some of us did) that we should have removed it from those two classes as well, but for good or for ill we have landed where we have. It’s an area where you will never get full agreement, so we need to espouse best practices in our SDK samples, which is the try{Close}/catch{Abort} paradigm.

     

    Tuesday, October 24, 2006 8:11 PM

All replies

  • Here is more detail from the referenced "interal discussion", which provides some historical context as well.  I think a key thing to notice is that Close() often implies doing "real work" that may fail, including network communication handshakes to shutdown sessions, committing transactions, etc.

     

    See also

    http://windowssdk.msdn.microsoft.com/en-us/library/aa355056.aspx

     

     

    ICommunicationObject (from which ServiceHost, ClientBase, IChannel, IChannelFactory, and IChannelListener ultimately derive) has always had two methods for shutting down the object: (a) Close, and (b) Abort.  The semantics are that if you want to shutdown gracefully, call Close otherwise to shutdown ungracefully you call Abort. 

     

    As a consequence, Close() takes a Timeout and has an async version (since it can block), and also Close() can throw Exceptions. Documented Exceptions out of Close are CommunicationException (of which CommunicationObjectFaultedException is a subclass), and TimeoutException.

     

    Abort() conversely is not supposed to block (or throw any expected exceptions), and therefore doesn’t have a timeout or an async version.

     

    These two concepts have held from the inception of Indigo through today. So far, so good.

     

    In its original incarnation, ICommunicationObject : IDisposable.  As a marker interface, we thought it would be useful to notify users that the should eagerly release this object if possible. This is where the problems begin. 

     

    Until Beta 1, we had Dispose() == Abort().  Part of the reasoning was that Dispose() should do the minimum necessary to clean up.  This was possibly our #1 complaint in Beta 1. Users would put their channel in a using() block, and any cached messages waiting to be flushed would get dropped on the floor. Transactions wouldn’t get committed, sessions would get ACKed, etc.

     

    Because of this feedback, in Beta 2 we changed our behavior to have Dispose() ~= Close(). We knew that throwing causes issues (some of which are noted on this thread), so we made Dispose try to be “smart”. That is, if we were not in the Opened state, we would under the covers call Abort(). This has its own set of issues, the topmost being that you can’t reason about the system from a reliability perspective. Dispose can still throw, but it won’t _always_ notify you that something went wrong.  Ultimately we made the decision that we needed to remove IDisposable from ICommunicationObject.  After much debate, IDisposable was left on ServiceHost and ClientBase, the theory being that for many users, it’s ok if Dispose throws, they still prefer the convenience of using(), and the marker that it should be eagerly cleaned up.  You can argue (and some of us did) that we should have removed it from those two classes as well, but for good or for ill we have landed where we have. It’s an area where you will never get full agreement, so we need to espouse best practices in our SDK samples, which is the try{Close}/catch{Abort} paradigm.

     

    Tuesday, October 24, 2006 8:11 PM
  • Awesome, thanks for this information Brian. Now it's a lot clearer and I understand things much better. I knew MSFT always has good design reasons, so I was hoping to find out how close differs from abort, and now I know (and I definately understand how leaving IDisposable on ClientBase would be such a hot issue). Hopefully the documentation will get updated with a bit of this insight.

    Thanks,
    Michael

    Tuesday, October 24, 2006 8:36 PM
  • What about using finally?

    For example:

             MyServiceClient target = new MyServiceClient();
             try
             {
                MyResultType expected = target.SomeServiceMethod(1);
             }
             catch (FaultException<MyCustomFault> fe)
             {
                 // Handle this exception type
             }
             catch (Exception ex)
             {
                 // Handle this exception type
             }
             finally
             {
               try
               {
                    if (target.State != CommunicationState.Faulted) target.Close();
                }
                catch
                {
                    target.Abort();
                }
            }
    Tuesday, October 31, 2006 11:06 PM
  • Yea, that might be a more elegant way. I'll have to try the different ways out a few times.
    Thanks!
    -Michael

     

    Tuesday, October 31, 2006 11:49 PM
  • Thanks, Michael. I'm hoping that Brian or one of the other WCF experts on this forum can weigh in on this issue. My concern with the simple try/catch approach is that it risk inadvertently generating CommunicationException errors.
    Wednesday, November 01, 2006 5:48 PM
  • I'm unclear what you're asking regarding the risk of 'inadvertantly generating CommunicationException errors'.

     

    See also these samples, which are intended to spell out some of our guidance here.

    http://windowssdk.msdn.microsoft.com/en-us/library/aa355056.aspx

    http://windowssdk.msdn.microsoft.com/en-us/library/aa354510.aspx

     

     

    Wednesday, November 01, 2006 7:00 PM
  • Sorry for the confusion. Maybe you can help me understand the risk of calling Abort on a channel that either may be closed or is in the process of closing. For example, I have applied default error handling to my services using an implementation of IErrorHandler interface. And I've also set up error logging. Will this generate, for example, any unnecessary activity on the server side that can be avoided?

    Maybe I just have a pyschological aversion to using Abort given how other programming framework consider it an action of absolute last resort. For example, Thread.Abort, to name the most obvious example. Better to attempt to close gracefully before using brute force, no?

    Regards.
    Thursday, November 02, 2006 5:27 PM
  • In this client code:

    try { client.Close(); }
    catch (CommunicationException ce) { client.Abort(); }
    catch (TimeoutException te) { client.Abort(); }

    everything is fine.  You tried to Close() gracefully.  But Close() threw.  This means the channel is not 'trying to close' or 'in the process of closing', it is just broken (typically in the faulted state).  If it's faulted, I think the only legal thing you can do on the channel is call Abort().  And Abort() should not generate any more network activity; the channel is no longer working, it is time to just clean up any local resources and cut the channel loose.

    Abort is "rude" to the server, whereas Close is "kind", so you should try to Close first.  But when Close fails, you must Abort, as there is no other option.

    (When Close() on the client throws, there are indeed likely to be errors logged on the server, but it's neither 'unnecessary' nor 'avoidable' server error activity.)

    Thursday, November 02, 2006 6:43 PM
  • i still would like to use the using code so i wrote a simple class that can be used like the following

    try

    {

    NetTcpBinding binding = new NetTcpBinding();

    //binding.MaxReceivedMessageSize = 70 * 1024;

    //binding.ReaderQuotas.MaxArrayLength = 65 * 1024;

    using (SafeChannelFactory<ICalculator> factory = new SafeChannelFactory<ICalculator>(binding, new EndpointAddress("net.tcp://localhost:9000/servicemodelsamples/service")))

    {

    ICalculator calculator = factory.Create();

    calculator.GetBytes();

    }

    }

    catch (Exception exception)

    {

    Console.WriteLine(exception.ToString());

    }

    the formatting does not look right but there is the implementation for the class

     

    using System;

    using System.Collections.Generic;

    using System.Text;

    using System.ServiceModel;

    using System.ServiceModel.Channels;

    namespace Microsoft.ServiceModel.Samples

    {

    /// <summary>

    ///

    /// </summary>

    public class SafeChannelFactory : IDisposable

    {

    private ChannelFactory internalFactory;

    /// <summary>

    ///

    /// </summary>

    protected SafeChannelFactory()

    {

    }

    /// <summary>

    ///

    /// </summary>

    ~SafeChannelFactory()

    {

    Dispose(false);

    }

    /// <summary>

    ///

    /// </summary>

    /// <param name="factory"></param>

    protected void AttachFactory(ChannelFactory factory)

    {

    this.internalFactory = factory;

    }

    /// <summary>

    ///

    /// </summary>

    /// <param name="channelFactory"></param>

    public void Close()

    {

    Close(TimeSpan.FromSeconds(10));

    }

    /// <summary>

    ///

    /// </summary>

    /// <param name="timeout"></param>

    public void Close(TimeSpan timeout)

    {

    if (this.internalFactory != null)

    {

    try

    {

    if (this.internalFactory.State != CommunicationState.Faulted)

    {

    this.internalFactory.Close(timeout);

    }

    else

    {

    this.internalFactory.Abort();

    }

    }

    catch (CommunicationException)

    {

    this.internalFactory.Abort();

    }

    catch (TimeoutException)

    {

    this.internalFactory.Abort();

    }

    }

    }

    /// <summary>

    ///

    /// </esummary>

    /// <param name="disposing"></param>

    private void Dispose(Boolean disposing)

    {

    if (disposing)

    {

    Close();

    }

    }

    /// <summary>

    ///

    /// </summary>

    void IDisposable.Dispose()

    {

    Dispose(true);

    GC.SuppressFinalize(this);

    }

    }

    /// <summary>

    ///

    /// </summary>

    /// <typeparam name="TChannel"></typeparam>

    public class SafeChannelFactory<TChannel> : SafeChannelFactory

    {

    private ChannelFactory<TChannel> internalFactory;

    /// <summary>

    ///

    /// </summary>

    /// <param name="binding"></param>

    /// <param name="endpointAddress"></param>

    public SafeChannelFactory(Binding binding, EndpointAddress endpointAddress)

    {

    this.internalFactory = new ChannelFactory<TChannel>(binding, endpointAddress);

    AttachFactory(this.internalFactory);

    }

    /// <summary>

    ///

    /// </summary>

    /// <returns></returns>

    public TChannel Create()

    {

    return this.internalFactory.CreateChannel();

    }

    }

    /// <summary>

    ///

    /// </summary>

    /// <typeparam name="TChannel"></typeparam>

    public class SafeDuplexChannelFactory<TChannel> : SafeChannelFactory

    {

    private DuplexChannelFactory<TChannel> internalFactory;

    /// <summary>

    ///

    /// </summary>

    /// <param name="instanceContext"></param>

    /// <param name="binding"></param>

    /// <param name="endpointAddress"></param>

    public SafeDuplexChannelFactory(InstanceContext instanceContext, Binding binding, EndpointAddress endpointAddress)

    {

    this.internalFactory = new DuplexChannelFactory<TChannel>(instanceContext, binding, endpointAddress);

    AttachFactory(this.internalFactory);

    }

    /// <summary>

    ///

    /// </summary>

    /// <returns></returns>

    public TChannel Create()

    {

    return this.internalFactory.CreateChannel();

    }

    }

    }

     

     

     

    Friday, November 10, 2006 8:58 PM
  • In C# 3.0 there is a way to avoid constructs like:

    Code Snippet

    Client c = new Client();

    try

    {

    c.Foo();

    c.Close();

    }

    catch (Exception)

    {

    c.Abort();

    throw;

    }

     

    The solution is in using Lambda functions (new feature of C# 3.0) and Generics. Here is a helper class:

    Code Snippet

    using System;

    using System.ServiceModel;

    namespace Common

    {

    public class Util

    {

    public static void UsingWcf(T client, Action action)

    where T : ICommunicationObject

    {

    try

    {

    action(client);

    client.Close();

    }

    catch (Exception)

    {

    client.Abort();

    throw;

    }

    }

    }

    }

     

    The complete usage example:

    Code Snippet

    using System;

    using Common;

    using TestApp.YourWcfServiceReference;

    namespace TestApp

    {

    class Program

    {

    static void Main(string[] args)

    {

    try

    {

    Util.UsingWcf(new YourWcfClient(), client =>

    {

    var result = client.CallWcfMethod();

    ...

    other work with your WCF service

    ...

    });

    Console.WriteLine("Test completed. Press ENTER.");

    }

    catch (Exception ex)

    {

    Console.WriteLine(ex);

    }

    finally

    {

    Console.ReadLine();

    }

    }

    }

    }

     

    As you can see my helper class allows you to write code almost like with builtin "using" statement (see bold text).

     

    Enjoy

    Friday, April 25, 2008 1:05 PM
  • What about FaultException ??

     

    better like this ??

     
    
    catch (FaultException unknownFault) 
    
    {
    
      throw; 
    
    }
    
    catch (CommunicationException) 
    
    {
    
      _proxy.Abort();
    
    }
    
    catch (TimeoutException) 
    
    {
    
      _proxy.Abort();
    
    }
    
    catch (Exception) 
    
    {
    
      _proxy.Abort(); // ??????????
    
      throw; 
    
    }
    
    

     

     

    Should "Hi", "Thanks" and taglines and salutations be removed from posts? http://meta.stackoverflow.com/questions/2950/should-hi-thanks-and-taglines-and-salutations-be-removed-from-posts

    Wednesday, April 13, 2011 1:09 PM