|
You shouldn't have a finalizer in the derived class, that will be inherited from the base class.
See here:
Dispose pattern guidelines[^]
If you follow this pattern, Your derived class (Foo) should have a Dispose(bool disposing) method that calls the dispose method of any private managed disposable objects (your component object) if true is passed. This method will be called by the finaliser and the dispose() methods of the base class.
Simon
|
|
|
|
|
Hi Simon,
1.
I am totally confused. In C#, we are not allowed to program Finalize method, so do you mean I should not have a destructor in derived class by below quoted comments?
2.
BTW: if the derived class manages native resource, I think it needs to have a destructor. Any comments?
Simon Stevens wrote: You shouldn't have a finalizer in the derived class, that will be inherited from the base class.
regards,
George
|
|
|
|
|
Ok,
there is no such thing as a destructor in c#. Destructors are methods that are deterministically called when an object is deleted. These exist in c++, but c# doesn't have them. Instead, in c# we have a finalize method. (with the ~ sign). When a class has a finalizer it indicates to the .net runtime that when the object is finished with, don't just free it's memory, first we need to clean up. So, when the object goes out of scope, the CLR puts the object in a finalizer queue. Periodcally, the CLR goes down the queue and calls all the finalizers. The important thing here is that you don't know when a finalizer will get called, (or it may in fact never get called).
Because of this, MS have created what is called the dispose pattern. The dispose pattern allows us to fake the behaviour of a deterministic destructor.
You need 3 methods.
1) A finalizer
2) A Dispose method that takes no parameters
3) A Dispose method that takes a boolean parameter.
From the finalizer call Dispose(false). From the dispose() method call Dispose(true).
Then make your 3rd method look like this
Dispose(bool isDisposing)
{
if(isDisposing)
{
if(component != null)
{
component.Dispose();
}
GC.SuppressFinalize(this);
}
}
Now, when you use your object, you can always make sure you call dispose() on it when your done with it. This then calls Dispose(true) and everything gets cleaned up exactly when you want it too (faking the behaviour of a deterministic destructor). If however, you forget to call dispose() when you're finished, (Or your class is used by a beginner programmer who doesn't know they have to do this) then you don't have to worry, because when the object is finished with, the CLR will automatically stick it in the finailzer queue so dispose(false) will get called eventually cleaning up any native resources to prevent memory leaks. (You don't clean up managed resources if the object is being finalized by the CLR, you just let them get finalized too)
Soooo... to answer your question. No, the derived class should not have a finalizer. Only the base class should. The derived class should override the Dispose(bool isDisposing) method to clean up the derived classes resources. The base classes finaliser will call the derived classes overridden dispose(false) method (because it's a virtual method) which will then clean up your derived classes native resources.
Really recommend you read CLR via C# to get a good idea of how this all works.
(Yes led mike, you don't have to say it... )
Simon
|
|
|
|
|
Great Simon!
Two more comments,
1. What are "MANAGED resources"? As you mentioned "// Clear up MANAGED resources here."? In my understanding, managed resource is a C# class which implements IDisposable interface or have a Finalizer, and such class may have wrapped native resource. Not sure whether my understanding of the concept of managed resource is correct?
2.
About your design pattern below, I think you mean some code like this, right?
http://msdn2.microsoft.com/en-us/library/b1yfkh5e(VS.80).aspx[^]
--------------------
No, the derived class should not have a finalizer. Only the base class should. The derived class should override the Dispose(bool isDisposing) method to clean up the derived classes resources. The base classes finaliser will call the derived classes overridden dispose(false) method (because it's a virtual method) which will then clean up your derived classes native resources.
--------------------
I think the issue of implementing Finalizer (implementation like, invokding Dispose with bool input parameter in Finalizer) in derived class is, the Dispose(bool) method in derived class will be called multiple of times from both base class's Finalizer and Derived class's Finalizer?
regards,
George
|
|
|
|
|
1. Yes, managed resources are C# classes that implement IDisposable. Or maybe for example a C# class that needs closing, like a file or database connection.
Yes, a managed class may wrap some native/unmanaged resources, but you don't have to worry about them. For example, the Form class wraps various native resources like window handles etc but you can just ignore that they are wrapped internally within the Form class and will be dealt with by the form. You can just treat any Form objects as managed resources.
2. Yes, that link is a good example of the full dispose pattern.
Yes, there are issues around the Dispose method being called twice if you implement the finalizer on dervied classes. You should always be careful to write your dispose method in such a way that it can get called twice without causing problems, because sometimes the finalizer can get kicked off before you've had chance to suppress it. This is why sometimes you see people setting a disposed flag the first time, so no work is done if it gets called again.
You seem to have an understanding of this. Disposal is complicated, just stick to the pattern on MSDN though and you can't go far wrong.
Simon
|
|
|
|
|
Thanks Simon,
I think there is another solution, which is only the most derived class implements Finalize method, and in the Finalize method, call Dispose method, in the Dispose method, call base class's Dispose method. As mentioned here,
http://msdn2.microsoft.com/en-us/library/b1yfkh5e(VS.80).aspx[^]
--------------------
Implement the dispose design pattern on a base type that commonly has derived types that hold onto resources, even if the base type does not. If the base type has a Close method, often this indicates the need to implement Dispose. In such cases, do not implement a Finalize method on the base type. Finalize should be implemented in any derived types that introduce resources that require cleanup.
--------------------
Do you think it will achieve the same effect as your method below?
--------------------
No, the derived class should not have a finalizer. Only the base class should. The derived class should override the Dispose(bool isDisposing) method to clean up the derived classes resources. The base classes finaliser will call the derived classes overridden dispose(false) method (because it's a virtual method) which will then clean up your derived classes native resources.
--------------------
regards,
George
|
|
|
|
|
Thats talking about a special case where your base class doesn't need a finalizer.
If your base class needs a finalizer, put it in the base class and don't put one in the derived class.
Simon
|
|
|
|
|
Thanks Simon,
The special case, you mean only derived class wrap real native resource, but base class does not wrap native resource, so in this case, we only need to add Finalizer in derived class?
regards,
George
|
|
|
|
|
LMAO Every time I see that it cracks me up! Yeah that's way simpler than using pointers with new/delete constructor and destructor!
Be afraid, be very afraid
led mike
|
|
|
|
|
Yeah, I agree, I've seen the dispose pattern done wrong so many times. The advantage is though even if you totally screw it up, the worst that can happen is that all your objects will have to be finalized. Disposal is really just a way of improving performance [EDIT] and releasing resources immediately [/EDIT] by cleaning things up yourself without waiting and relying the gc finalizer.
Last modified: 28mins after originally posted --
Simon
|
|
|
|
|
Thanks Simon,
You mean even if Dispose pattern is wrongly implemented, we can still rely on Finalize method to clean up memory and native resources?
Simon Stevens wrote: The advantage is though even if you totally screw it up, the worst that can happen is that all your objects will have to be finalized.
regards,
George
|
|
|
|
|
Not quite.
If you do absoutly nothing at all, no dispose method, no finalizer, nothing. Then what will happen is that all objects that needed disposing of (like the component object in your example) will eventually get cleaned up by the GC which will call the finalizers. This isn't great though. Objects that have to be finalized have to go onto the finalizer queue, which means they stay in memory longer (They survive one GC generation longer) so your memory usage is higher, and resources aren't freed up when they're finished with, it may be quite a while before the finalizer gets called. But yes, eventually all managed resources will get cleaned up by the GC finalizer thread. And thus, any native resources wrapped by the managed classes will also get cleaned up.
However, if you directly use a native resource yourself there is nothing to clean it up. If you use a native resource directly, you must make sure you clean it up! if you don't, you have a memory leak.
The handy thing is, if you implement the dispose pattern on your class, then you are wrapping the native resouce yourself, so users of your class don't have to worry about it, so if users of your class forget to call dispose, you don't have to worry because your classes finalizer will eventually do the work.
Simon
|
|
|
|
|
Cool, Simon!
I want to confirm with you an important point that, if we do not release native resource like file handle in either Dispose method or Finalizer method, GC will not release such native resource, right?
regards,
George
|
|
|
|
|
|
Thanks Zoltan,
Cool!
regards,
George
|
|
|
|
|
The thing is that in .Net we have undeterministic finalization meaning that the Finalize() method will be called at an undetermined time in the future. The solution to this is to have deterministic finalization by implementing the Dispose pattern. You should have your cleanup code in Dispose() and if you implement the Finalize() method that should call on Dispose.
George_George wrote: BTW: if the derived class manages native resource, I think it needs to have a destructor. Any comments?
No, implement the Dispose pattern and release the native resources there. Implementing a destructor is not the best solution because you don't know when the GC will finalize the object.
For more info check out this article[^]
|
|
|
|
|
Thanks Simon,
I think the correct pattern should be, the derived class should only implement Dispose, but not Finalize, right?
regards,
George
|
|
|
|
|
You can implement Finalize too and in the Finalize() method just call the Dispose() method.
|
|
|
|
|
Thanks Zoltan,
You mean from base to derived class to most derived class, all implements Finalize method and in Finalize method call Dispose?
regards,
George
|
|
|
|
|
Hi gurus, I have this method in one of my classes:
public static Forum GetForumByID(int forumID)
{
Forum forum = null;
foreach (Forum theForum in GetForums())
{
if (theForum.ID == forumID)
{
forum = theForum;
break;
}
}
if (forum == null)
{
throw new ForumNotFoundException(forumID);
}
return forum;
}
Now, I thought I'd rewrite the method to be like that:
public static Forum GetForumByID(int forumID)
{
foreach (Forum forum in GetForums())
{
if (forum.ID == forumID)
return forum;
}
throw new ForumNotFoundException(forumID);
}
My question is, is there any thing wrong with that code? It's perfectly accepted by the compiler (as you know nothing is executed after an exception is thrown). Actually I never write such code, so this is going to be the first time but I want to know whether there's any problem with that code, also whether you believe it's a bad practice or coding style and why.
Your help is much appreciated ...
|
|
|
|
|
Waleed Eissa wrote: My question is, is there any thing wrong with that code?
Maybe. If not finding a forum is an "expected" condition then you shouldn't throw an "exception" for it.
led mike
|
|
|
|
|
Some ASP.NET controls requires this anti-pattern
|
|
|
|
|
I'm sorry, I didn't quite understand what you mean, could you please explain more?
|
|
|
|
|
an anti pattern is a case of BAD design. As opposed to a normal 'design pattern' which is a best practice way of doing something.
You know, every time I tried to win a bar-bet about being able to count to 1000 using my fingers I always get punched out when I reach 4....
-- El Corazon
|
|
|
|
|
Thanks for your answer dan, well, usually you should use exceptions for handling unexpected conditions which is true in my case (at least in my understanding), I'm not using exceptions to control the flow of my application ...
|
|
|
|
|