Re: thread safe in Dispose method - C# Discussion Boards

14-May-08 2:57

Hello everyone,

Here is my code for Dispose method. Currently, I am thinking whether it is necessary to make it thread safe by adding a lock to prevent two threads to execute Dispose methods at the same time?

Notes:

1. disposing is used to indicate explicit call to Dispose or implicit call from Finalizer;

2. disposed is used to indicate whether current instance is Disposed or not, to prevent from disposing again. The thread safety consideration is used to protect this variable.

  protected virtual void Dispose(bool disposing)
  {
      lock (DisposeLock)
      {
          if ((false == disposed) && (true == disposing))
          {
// some operations here
              disposed = true;
          }
      }
  }

thanks in advance,
George

Christian Graus14-May-08 3:00

Christian Graus

14-May-08 3:00

Why would two threads call dispose on the same object ?

Christian Graus

Please read this if you don't understand the answer I've given you

"also I don't think "TranslateOneToTwoBillion OneHundredAndFortySevenMillion FourHundredAndEightyThreeThousand SixHundredAndFortySeven()" is a very good choice for a function name" - SpacixOne ( offering help to someone who really needed it ) ( spaces added for the benefit of people running at < 1280x1024 )

George_George14-May-08 15:16

14-May-08 15:16

Thanks Christian,

My concern is, if a reference is or potentially held by two threads, and they may call Dispose at the same time. Any comments?

regards,
George

Peter Josefsson Sweden14-May-08 16:08

14-May-08 16:08

They may not. Strictly speaking, if two threads or objects hold references to a single object, neither of them may call Dispose.

If they do, they are violating the fundamental rules of the IDisposable pattern.

The rules in short:

1. Dispose is meant to tell an object this: "I now know for sure that nobody has a reference to you except for myself, and I will never talk to you again - you may now safely throw away any resources you've been holding on to. If I or someone else ever talks to you again, please throw an exception.".

2. If you don't know for sure that you're the only one holding a reference to the object, you should never call Dispose. Just drop your reference (as in myRef = null;). The GC (garbage collector) will then call its finalizer (~TheClass) as soon as a) it realizes that nobody holds a reference to the object any more and b) it or your program has nothing more important to do. The finalizer will call Dispose(false), ensuring that any unmanaged (operating system handles and whatnot) will finally be released.

The only reason that the docs say that you should always allow calls to Dispose even if you are already disposed or not completely initialized is that you may in some cases get called from a catch or finally block that shouldn't have to check whether or not the try block that should have called you actually did it before the exception got thrown. This can never occur simultaneously on different threads, though.

So, the IDisposable stuff is only an optimization, enabling us to force expensive resources to be released before the GC gets around to it - but it should never be used unless we know for sure that the object is dead for good. Note that there is a lot of code (even in the framework) that just crashes spectacularly instead of throwing the proper exceptions if you try to use an object that has been disposed. Try loading a bitmap, showing it in a picture box and then disposing it (while still showing it). Results will vary - the app may exit silently, throw a protection fault or even display random graphics. At least, this was true in 1.1 - it was a while since I tested that.

Later,

--
Peter

George_George14-May-08 16:20

14-May-08 16:20

Great reply, Peter!

1.

PeterTheSwede wrote:
The only reason that the docs say that you should always allow calls to Dispose even if you are already disposed or not completely initialized is that you may in some cases get called from a catch or finally block that shouldn't have to check whether or not the try block that should have called you actually did it before the exception got thrown. This can never occur simultaneously on different threads, though.

I am very interested in the scenario you described, but still confused after reading a couple of times. Could you show some pseudo code please?

2.

"dead for good" means on one else refers the object?

3.

PeterTheSwede wrote:
Note that there is a lot of code (even in the framework) that just crashes spectacularly instead of throwing the proper exceptions if you try to use an object that has been disposed.

You mean ObjectDisposedException should be thrown -- and in your points should be the correct behavior -- in this scenario?

regards,
George

Peter Josefsson Sweden14-May-08 17:00

14-May-08 17:00

1. I'll try.

If I remember the recommendations for the IDisposable interface correctly, the requirement that duplicate (but not simultaneous, mind you) calls to Dispose() shouldn't fail is explained with something like this:

MyClass obj = new MyClass();
try
{
    obj.DoSomethingThatCanFail(); // A: we may fail here - obj is not disposed
    obj.Dispose(); // dispose it
    this.DoSomeOtherStuff(); // B: we may fail here - obj could be disposed
}
catch
{
    obj.Dispose(); // make sure obj is disposed
    throw;
}

This code should be considered correct - that is, you shouldn't have to check if things went wrong at A or B in the catch block. Note that obj could potentially be a 2GB image, and the DoSomeOtherStuff() method could also be something extremely memory-hungry. In which case it makes sense to call obj.Dispose() then and there instead of in a finally block.

2. Exactly.

No one but you refers to it and you aren't ever going to use it again. If you can't guarantee those conditions, you're essentially not allowed to Dispose() the object.

3. Yes.

According to Microsofts current recommendations, ObjectDisposedException should be thrown if you try to do anything with an object that is disposed (except calling Dispose() again). But like I said, there's a lot of code out there that just crashes instead. Understandably - it's easy to forget if (disposed) throw new ObjectDisposedException(); in a method or property somewhere.

Which makes it even more important to never use Dispose() unless you're absolutely sure that the object is dead. People who get buried alive don't play nice... Smile | :)

Later,

--
Peter

George_George14-May-08 17:26

14-May-08 17:26

Thanks Peter,

Your reply is great!

1.

In your sample, it is possble to call Dispose twice on the same object if there is exception thrown from DoSomeOtherStuff, in this case, there will be ObjectDisposedException from catch block?

2.

Is it always a program bug when we met with ObjectDisposedException? Means someone Disposed the object instance, but some other guys may still refer it and call the method on it.

3.

I like your below code,

if (disposed) throw new ObjectDisposedException();

Currently, I think of two solutions when met with ObjectDisposedException. Solution 1 is to do operations on object straigtforward without checking IsDisposed property, and wrap catch block to catch ObjectDisposedException, solution 2 is to check IsDisposed property as you showed above, and it is set, do nothing but throw ObjectDisposedException to propogate out or write a log without rethrow. I am wondering which pros and cons and your suggestions of which solution is better?

regards,
George

Peter Josefsson Sweden15-May-08 8:51

15-May-08 8:51

Thanks for the appreciation!

When English isn't your own language (which I assume it isn't for neither of us), it can be hard to understand even the good C#/CLR books sometimes. I have read a lot of good texts about the IDisposable interface over the last few years (there are even some excellent articles on CodeProject), but I must admit that it wasn't until recently that all the pieces of the puzzle fell into place. Sometimes you have to read the same thing (phrased differently) several times before you get it.

So, I'll try to summarize the most important points again (it should answer your follow-up questions as well), as short and concise as I can:

A: It is always legal to call Dispose() on an object more than once, for exactly one reason: The example I gave (and similar code) should always work even if something happens in DoSomeOtherStuff(). This means that a call to Dispose() should never cause an ObjectDisposedException - if the object is already disposed, Dispose() should just silently do nothing. Note that usually, the only place where this is acceptable is in an exception handler (as in my example).

B: It is always illegal to call any other method or property on an object after Dispose() has been called. And ideally, an ObjectDisposedException should always be thrown if that happens (but like I said, this is not always the case - sometimes very bad things such as unmanaged memory corruption or similar happen instead - even in some .NET Framework classes).

Some conclusions (some obvious, some not):

C: You may only call Dispose() on an object if a) you created the object, b) you haven't given away the reference to anyone and c) you're done with it. Can't be said too many times...

D: If you ever see an ObjectDisposedException, it is a bug. It should be fixed (yesterday), so it's fine to just crash (as long as you leave a clue as to why, meaning that if you're a service or a web application, you should log the exception before crashing - if you're a Windows app, you should display it and/or log it).

E: A repetition of rule C from a different angle: If you ever expose your reference to a disposable object (by returning it from a property or using it in a call into a class you don't control), you effectively give up your right to dispose of it. There are cases when it is OK to do differently, but they must be documented. For example: "If you dispose of a MyParent object, all MyChild objects created by it will also be disposed of, and may not be used again" (if I remember correctly, there are such relationships in the SqlClient namespace in the framework). In such cases it does make sense to deviate a bit from the basic rules.

F: Answering your original question - making the Dispose() method thread-safe is meaningless. It should only be called by the class that created the object, only when it will not be used again (even though additional Dispose() calls may be made by the same caller due to exception handlers) and only when nobody else can hold a reference to the object. Hence, the class creating the object has no reason whatsoever to call Dispose() from more than one thread. If someone comes up with a reason (I once did, actually - I called the same termination and cleanup routine from a worker thread that I exposed to clients when it died due to external problems), the caller is free to implement his own synchronization (I did in my termination routine).

G: A public IsDisposed property is entirely meaningless. If the code calling Dispose() doesn't know it did, it's broken (the example with the exception handler is the only exception - but since an extra Dispose() should cost less than a property check, the property still doesn't make sense).

H: The IDisposable interface is meant for classes holding unmanaged resources (including other disposable objects). Don't bother otherwise (unless you're writing an abstract class that would typically be implemented using unmanaged resources or disposable objects).

Finally, an example from the framework: If you create an Image from a BinaryStream you may think that you can dispose of the BinaryStream when that is done. Not so. The Image will not read the entire stream until it has to, and if the stream is a file it is even free to read the same part of the file several times when needed. Look at rule E. You showed the stream to the image, so you're no longer in control of it, and may not call Dispose(). In this particular case, you can dispose of the image (after making sure that nothing that has seen it is still around) and then of the stream.

Clear as mud?

Smile | :)

One more suggestions: Do read the articles on implementing the IDisposable interface here on CodeProject as well. There are some aspects (such as how exceptions in Dispose() should be handled, and how object inheritance comes into play) to it that I haven't mentioned. It's too complex for a forum post, and I don't remember all of it anyway...

Good Night!

--
Peter

George_George16-May-08 20:42

16-May-08 20:42

Thanks Peter,

Your reply is so great!

After reading your reply, I think it should be a bug -- either my code or CLR code, if ObjectDisposedException is thrown. Smile | :)

My detailed scenario is like this,

(I am writing an Http Server service using HttpListner class of C#, and handles coming request in an asynchronous way.)

1. The exception from server side, when I call EndGetContext, not from client side;

2. On the server side, I create an HttpListener object, and waiting for incoming client request;

3. When I received a request, I will process the request asynchronously by using BeginGetContext, and in this way HttpListener object instance can still listen for incoming calls, while processing backend at the same time;

4. When sending the request to server from client, the client will close the connection immediately -- during the asynchronous processing period;

5. The server will throw exception when I call EndGetContext. At the same time, ObjectDisposedException will happen.

Any ideas why the exception will happen? I am also interested in whose Disposed method is called, any ways to detect which object instance's Disposed method is invoked?

regards,
George

Peter Josefsson Sweden16-May-08 21:46

16-May-08 21:46

Hi!

Again, thanks for the appreciation. I usually don't write such long answers but I've been at a customer site (staying at a hotel) this week, and nights get a bit boring... Smile | :)

Sorry, no idea what is happening. I have never used asynchronous calls seriously so I don't have any experience to draw from. Then again, try looking for asynchronous samples (I don't find any using HttpListeners, but there are bound to be others) and understand what they do. Then perhaps it will become clearer.

--
Peter

Peter Josefsson Sweden14-May-08 5:57

14-May-08 5:57

Hi,

I've seen others have this concern and have seen a multitude of solutions, but like Christian says, why? I only realized this recently (and felt pretty stupid about that), but like he says, there is NO circumstance whatsoever where it is correct for two different threads to invoke Dispose() on the same object.

In fact, if Dispose() is ever invoked twice on the same object (even from the same thread), it is a design fault. It is explicitly wrong to invoke Dispose() on an object if there is a chance (no matter how slim) that someone else is holding a reference to it (meaning that if you ever directly or indirectly expose a reference to an object outside of your class, you should never dispose the object).

Further, Dispose() cannot be invoked unless the object is alive, so it can never conflict with the finalizer being invoked by the GC.

The most common pattern for implementing IDisposable that I've seen is:

protected virtual void Dispose(bool disposing)
{
    if (!disposed)
    {
        if (disposing)
        {
            // Clean up managed resources here - invoke Dispose() on privately held objects that
            // implement IDisposable, and null out other (expensive) object references so that the
            // GC can reclaim them even if the caller keeps its reference to us:
            // ...
        }
        // Clean up unmanaged (allocated via interop calls etc) resources here:
        // ...
        // If this is a derived class and the base class implements IDisposable as well:
        // base.Dispose(disposing);
        disposed = true;
    }
}

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

~MyClass()
{
    Dispose(false);
}

Note: Written from memory, beware of spelling mistakes. Google for "IDisposable pattern" and you will get all the examples you need (good and bad).

Later,

--
Peter

George_George14-May-08 15:21

14-May-08 15:21

Thanks Peter,

Any comments to? I write my confusions and why I think two threads may call Dispose at the same time there. Smile | :)

http://www.codeproject.com/script/Forums/View.aspx?fid=1649&msg=2553475[^]

regards,
George

DllImport question

matt23lucier14-May-08 2:20

matt23lucier

14-May-08 2:20

I was wondering if there was any way to see what functions are availble in dll's. For example how can I find out the available functions for the user32.dll?

Christian Graus14-May-08 2:38

Christian Graus

14-May-08 2:38

www.pinvoke.net has a good list.

Dario Solera14-May-08 2:46

Dario Solera

14-May-08 2:46

Dependency Walker[^] can do that.

If you truly believe you need to pick a mobile phone that "says something" about your personality, don't bother. You don't have a personality. A mental illness, maybe - but not a personality. - Charlie Brooker
My Photos/CP Flickr Group - ScrewTurn Wiki

carbon_golem14-May-08 4:28

carbon_golem

14-May-08 4:28

If you really want to get oldschool, use the dumpbin[^] utility.

Scott P

“It is practically impossible to teach good programming to students that have had a prior exposure to BASIC: as potential programmers they are mentally mutilated beyond hope of regeneration.”
-Edsger Dijkstra