Properly Avoid ObjectDisposedException

Question

I ran into an issue where an ObjectDisposedException is being thrown roughly 50% of the time. The code within the try (within the finally) below, is causing the exception. I'm not sure how to handle this. I could just eat the exception, like shown below, but is there a way to check and close the object without the exception happening?

    public static FindResponse Discover(FindCriteria findCriteria, 
                                        DiscoveryEndpoint discoveryEndpoint = null)
    {
        DiscoveryClient discoveryClient = null;

        try
        {
            if (discoveryEndpoint == null) { 
                 discoveryEndpoint = new UdpDiscoveryEndpoint(); 
            }

            discoveryClient = new DiscoveryClient(discoveryEndpoint);

            return discoveryClient.Find(findCriteria);
        }
        finally
        {
            try
            {
                if (discoveryClient != null)
                {
                    discoveryClient.Close();
                }
            }
            catch (ObjectDisposedException)
            {
                // Eat it.
            }
        }
    }

Can you please post the Stack Trace ? – Rohit Prakash Feb 05 '15 at 06:54 — Rohit Prakash, Feb 05 '15 at 06:54

Richard Schneider · Accepted Answer · 2015-02-05T07:27:48.630

2

How about

public static FindResponse Discover(FindCriteria findCriteria, DiscoveryEndpoint discoveryEndpoint = null)
{
    if (discoveryEndpoint == null) 
      discoveryEndpoint = new UdpDiscoveryEndpoint();

    using (var client = new DiscoveryClient(discoveryEndpoint))
    {
        return client.Find(findCriteria);
    }
}

Update

Seems DiscoveryClient.Dispose() will throw exceptions. OP's original approach seems like the only acceptable answer.

edited Feb 05 '15 at 07:27

answered Feb 05 '15 at 06:55

Richard Schneider

34,944
9
57
73

I should have thought of that. :) Thanks. – Bob Horn Feb 05 '15 at 07:03
Ahhhhhh!! I'm getting the exception again! I may just have to catch it and eat it. – Bob Horn Feb 05 '15 at 07:18
Oh, I've seen this same issue (Close and Dispose) with WCF clients. Best to just eat it as you originally suggested. See http://stackoverflow.com/questions/23867/closing-and-disposing-a-wcf-service – Richard Schneider Feb 05 '15 at 07:24

Siva Gopal · Answer 2 · 2015-02-05T07:48:58.083

1

Though I am not quite sure why you run into it, but you may try the following for normal WCF client:

If you have "State" property available on discoveryClient then, please try with following defensive checks:

finally
        {
            try
            {
                if (discoveryClient != null) 
                { 
                  if(discoveryClient.State == CommunicationState.Faulted)
                  {
                     discoveryClient.Abort();
                  }
                  else if(discoveryClient.State != CommunicationState.Closed )
                  {
                     discoveryClient.Close();
                  }
            }
            catch (ObjectDisposedException)
            {

            }
        }

Hope this help you.

edited Feb 05 '15 at 07:48

answered Feb 05 '15 at 07:30

Siva Gopal

3,474
1
25
22

Can't the state change between your `if` check and what's inside the `if`? – Bob Horn Feb 05 '15 at 07:34
@BobHorn You are right. But from [MSDN](https://msdn.microsoft.com/en-us/library/system.servicemodel.communicationstate.aspx) , "Closing" state is transient and instead it would be better to do a check on non-transient state "Closed" and updated my answer accordingly. Also "Faulted" state is another where we should close the channel with Abort only. + As you mentioned originally, if all these states are slipped out for some reason, then better to swallow it :) – Siva Gopal Feb 05 '15 at 07:53
Thanks. With your example, we'd still need to catch and eat the exception, but at least the code is explicit in what it's doing. +1 for the tip, but I'm just going to catch and eat the exception without worrying about the other states. Sometimes you just get tired of writing code like this. :) – Bob Horn Feb 05 '15 at 07:59
1

@BobHorn But if you go the route of defining a custom wrapper around your communication channel and then you can encapsulate this state handling into it so that you least bothered of spilling these state checks across code base. – Siva Gopal Feb 05 '15 at 08:08

Rohit Prakash · Answer 3 · 2015-02-05T07:03:57.370

-1

Also I would recommend to use "using" for the IDisposable objects. Considering DiscoveryClient is IDisposable,

public static FindResponse Discover(FindCriteria findCriteria, DiscoveryEndpoint discoveryEndpoint = null)
    {

        FindResponse response = null;
        try
        {
            if (discoveryEndpoint == null) { discoveryEndpoint = new UdpDiscoveryEndpoint(); }

            using (DiscoveryClient discoveryClient = new DiscoveryClient(discoveryEndpoint))
            {
                response = discoveryClient.Find(findCriteria);
                discoveryClient.Close();
            }
        }
        finally
        {   
            // other finalizing works, like clearing lists, dictionaries etc.
        }
        return response;
    }

edited Feb 05 '15 at 07:03

answered Feb 05 '15 at 06:58

Rohit Prakash

1,975
1
14
24

Sorry but `finally {}` is a no-op. – Richard Schneider Feb 05 '15 at 07:01
@RichardSchneider, I think, I have edited the code right way. . .. . – Rohit Prakash Feb 05 '15 at 07:05

Properly Avoid ObjectDisposedException

3 Answers3