This is why I am starting to loathe programming
-
I get what you're saying, but have you noticed that if you don't call Dispose, even with that solution, one handle gets left behind? Put this: this.Text = HWndCounter.GetWindowHandlesForCurrentProcess().ToString(); before ShowDialog() And then test it commenting and uncommenting the frm.Dispose() without calling Dispose, with your code, I get 42 handles. Calling dispose, I get 41. What it seems to me is that after the finalizer gets called, it still does not free the handle (even calling Dispose method inside the finalizer). I also noticed, that if I call the form's Dispose method from the calling method, the finalizer does not get called. Bottom line is that I never get to reclaim the handle without explicitly Disposing it on the calling method. Even after repeatedly executing this after closing the form: GC.Collect(); GC.WaitForFullGCComplete(); GC.WaitForPendingFinalizers(); I feel there's still something missing I'm not seeing... But to be on the safe side, I still recommend everyone, always call Dispose method if the class implements IDisposable
It's likely the extra handle is for a Font or Image that is statically allocated, but only done so once it was loaded. So, I wouldn't call that a leak. It is evident by the fact that the object is being disposed and/or finalized, but the overall count is not going up (minus the initial value). As for the finalizer not being called, that's on purpose. When you implement the IDisposable pattern, you're supposed to do the following for the public Dispose method:
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}The last line will suppress the finalizer because of two reasons: 1) It's not needed anymore, the finalizer is meant to call dispose in case it wasn't called. 2) Secondly, objects that need finalizing (as seen with our example) are cleaned up at the framework's whim. This can lead to more memory pressure as an object isn't cleaned up in a timely manner. I whole-heartedly agree that if a class implements IDisposable, that you should call it. But it is important to know the underlying reasons behind that and why the IDiposable pattern works (when coded correctly). Essentially, the pattern is in place to prevent a simple oversight from turning into unusable code.
-
It's likely the extra handle is for a Font or Image that is statically allocated, but only done so once it was loaded. So, I wouldn't call that a leak. It is evident by the fact that the object is being disposed and/or finalized, but the overall count is not going up (minus the initial value). As for the finalizer not being called, that's on purpose. When you implement the IDisposable pattern, you're supposed to do the following for the public Dispose method:
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}The last line will suppress the finalizer because of two reasons: 1) It's not needed anymore, the finalizer is meant to call dispose in case it wasn't called. 2) Secondly, objects that need finalizing (as seen with our example) are cleaned up at the framework's whim. This can lead to more memory pressure as an object isn't cleaned up in a timely manner. I whole-heartedly agree that if a class implements IDisposable, that you should call it. But it is important to know the underlying reasons behind that and why the IDiposable pattern works (when coded correctly). Essentially, the pattern is in place to prevent a simple oversight from turning into unusable code.
Btw, add the following to the class as well, to help understand the flow of logic:
protected override void Dispose(bool disposing) { base.Dispose(disposing); Trace.Write("TestForm #" + FormHandle + " disposed.\\r\\n"); }
-
Fabio Franco wrote:
Wrong, the Finalizer does not assure the release of unmanaged resources, if there is a Dispose method call it. The Dispose method is not called by the garbage collector.
If you implement IDisposable incorrectly, and do not call it from the finalizer if not already disposed, you have stupid code.
Fabio Franco wrote:
Just do a little experiment and see for yourself:
I am not even going to bother.
Fabio Franco wrote:
If you uncomment GC.Collect() and comment Dispose(), it will keep the handle count from growing indefinitely
It's up to you how you deal with it. My point still stands, you dont have to do it, it is not a MUST DO OR DIE situation.
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth EditionI agree with Leppie. The IDisposable pattern (when properly implemented) has a safety net that will release native resources even if we didn't call Dispose() or wrapped our instance with a using statement. Garbage collection might take a while to kick in, and yes, it is possible to write an application where garbage collection will never occur, but if you're talking of resource exhaustion, then that means you are constantly creating objects, then THERE is memory pressure and the gc WILL eventually run (unless of course, you are unnecesarily keeping references to objects that should be collected and by doing so, inducing a memory leak). Cheers, Gerardo
-
I agree, I always call the dispose method if implemented. I couldn't find the handle leak in your test app. The handles do increase but when a GC collection occurs (for me I had to create about 30 dialogs) the count returns to what it was initially?? This is because the finalize method is executed which in turn calls dispose.
Commenting and Uncommenting frm.Dispose() makes no difference in handle count? Forcing the collection also won't make a difference?
-
Hi Fabio, Sorry, your example is not entirely correct. If you do not call Dispose() on a Form class instance, it will still be disposed (with its underlying window handle properly released) when it's garbage collected. The IDisposable pattern (which is implemented, as far as I know, in all .net Framework classes that hold native resources) takes precisely care of this. The only difference is that you don't know exactly when will the garbage collection occur and since it's triggered by memory allocation pressure, it might take a while for it to actually happen. That's why it's wise to call Dispose(). Cheers! :) Gerardo
Hi Gerardo, I know that GC can't be predicted, however you can force a collection with GC.Collect() Garbage Collection is no guarantee that Dispose gets called, it all depends on the finalizer implementation. If Dispose correctly releases unmanaged resources and if you call it from the finalizer (good code), then yes, unmanaged resource will be released when a collection occurs, if you don't, Dispose method will never get called and the unmanaged resources will never be reclaimed (not until the process ends). In another discussion in this topic I provide an example of exactly that, where the Window handle is not released even after forcing collection (finalizer gets called, you can see it throug output windows). Only way it did was through explicitly calling Dispose method afer the form was closed, inside the calling method. Regards, Fábio
-
It's likely the extra handle is for a Font or Image that is statically allocated, but only done so once it was loaded. So, I wouldn't call that a leak. It is evident by the fact that the object is being disposed and/or finalized, but the overall count is not going up (minus the initial value). As for the finalizer not being called, that's on purpose. When you implement the IDisposable pattern, you're supposed to do the following for the public Dispose method:
public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}The last line will suppress the finalizer because of two reasons: 1) It's not needed anymore, the finalizer is meant to call dispose in case it wasn't called. 2) Secondly, objects that need finalizing (as seen with our example) are cleaned up at the framework's whim. This can lead to more memory pressure as an object isn't cleaned up in a timely manner. I whole-heartedly agree that if a class implements IDisposable, that you should call it. But it is important to know the underlying reasons behind that and why the IDiposable pattern works (when coded correctly). Essentially, the pattern is in place to prevent a simple oversight from turning into unusable code.
Andrew Rissing wrote:
It's likely the extra handle is for a Font or Image that is statically allocated, but only done so once it was loaded. So, I wouldn't call that a leak. It is evident by the fact that the object is being disposed and/or finalized, but the overall count is not going up (minus the initial value).
Ok, but why does the count comes down when the form's Dispose method is called from the calling method, and not from the finalizer? I mean if it is something static, it should not come down either way right? This is what's boggling me. Thank you for your clarifications, it surely filled a few gaps. Still, there is this thing, bothering me... Did you get to reproduce what I was saying?
-
Hi Gerardo, I know that GC can't be predicted, however you can force a collection with GC.Collect() Garbage Collection is no guarantee that Dispose gets called, it all depends on the finalizer implementation. If Dispose correctly releases unmanaged resources and if you call it from the finalizer (good code), then yes, unmanaged resource will be released when a collection occurs, if you don't, Dispose method will never get called and the unmanaged resources will never be reclaimed (not until the process ends). In another discussion in this topic I provide an example of exactly that, where the Window handle is not released even after forcing collection (finalizer gets called, you can see it throug output windows). Only way it did was through explicitly calling Dispose method afer the form was closed, inside the calling method. Regards, Fábio
Hi Fabio, Yes, you are correct, good code should follow the IDisposable pattern documented by Microsoft, that way you can dispose unmanaged resources early if you want, or leave it to the GC. I'll look at your example, because if what you say is true, then Microsoft is not following its own recommended patterns! Thanks for the reply, Gerardo
-
Hi Fabio, Yes, you are correct, good code should follow the IDisposable pattern documented by Microsoft, that way you can dispose unmanaged resources early if you want, or leave it to the GC. I'll look at your example, because if what you say is true, then Microsoft is not following its own recommended patterns! Thanks for the reply, Gerardo
Thanks Gerardo, yes the point was exactly that, and I prefer not to rely on the Finalizer implementation, simply call Dispose and play the safe side.
-
Andrew Rissing wrote:
It's likely the extra handle is for a Font or Image that is statically allocated, but only done so once it was loaded. So, I wouldn't call that a leak. It is evident by the fact that the object is being disposed and/or finalized, but the overall count is not going up (minus the initial value).
Ok, but why does the count comes down when the form's Dispose method is called from the calling method, and not from the finalizer? I mean if it is something static, it should not come down either way right? This is what's boggling me. Thank you for your clarifications, it surely filled a few gaps. Still, there is this thing, bothering me... Did you get to reproduce what I was saying?
Indeed, I was able to reproduct it. I don't know though if its a memory leak or just something else we aren't aware of. If it was a true memory leak, I would imagine that it's behavior would be repeatable and the allocations would keep rising. Ultimately, one handle is pretty insignificant in the grand scheme of things.
-
Indeed, I was able to reproduct it. I don't know though if its a memory leak or just something else we aren't aware of. If it was a true memory leak, I would imagine that it's behavior would be repeatable and the allocations would keep rising. Ultimately, one handle is pretty insignificant in the grand scheme of things.
Yes, this keeps me thinking that this is just one case, for one form. What else might have this behaviour? and depending on the scenario, this might rise significantly. So I have no doubt that calling Dispose (when you desire to release resources of course) is a very strong should, no matter how the Garbage Collector gets invoked. One theory I have about this behaviour is that one of these handles is kept for performance issues. When Dispose does not get called and the object is collected, the Garbage Collector (or the Finalizer / IDisposable implementation) might think it will be reused (instatiated again at some point of time) and so the handle is kept for reuse with the same form. If you experiment a little more, you'll also notice that another handle increment will happen if you instatiate another Form implementation (besides from TestForm) and don't call Dispose. Same thing will happen for that form. Anyways, this was a nice discussion to have. Regards, Fábio
-
It's like the blind leading the blind.... http://stackoverflow.com/questions/2926869/c-do-you-need-to-dispose-of-objects-and-set-them-to-null/2926877#2926877[^] I think I will get similar responses from here too though :sigh:
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth EditionLooking at all the responses in this thread, there seem to be a lot of confusion about dispose and finalization. The basic recommendations is really not that complicated :-\ : 1) In the finalizer (i.e. ~SomeClass() ), clean up unmanaged resources. This method is called by the garbage collector, when the object can no longer be reached. However, it may not happen immediately, the garbage collector waits until it needs memory (which could be hours or days after it is no longer reachable). 2) In Dispose() (i.e. the method defined in IDisposable), cleanup managed and unmanaged resources. Furthermore, call GC.SuppressFinalize(this) (since everything is now cleaned up, there is no need for the finalizer to be called by the garbage collector, and doing this optimizes the garbage collection). The dispose pattern furthermore combines the implementation of these two, by implementing a protected Dispose(bool disposing) in the base class. The finalizer calls it with false, the Dispose() in IDisposable calls it with true. This makes it simple for derived classes, it only has to override this method and add whatever it needs, but it doesn't really change anything, it just a matter of style in the implementation. Why shouldn't the finalizer clean up managed resources as well? Because that is what the garbage collector does - managed resources is the memory used by other managed classes. Furthermore, it is very limited what a finalizer is allowed to do - it must not use references to other objects (as they may already be garbage collected). Why implement Dispose(), if the garbage collector takes care of it all? Because sometimes you need some cleanup to take place at a deterministic time, not hours or days later. The classical example is an open file. You typically want to allow the file being opened again, immediately you are done using it. Why do you need IDisposable for this, why not just implement a Close method on the class? Because with IDisposable, you have a uniform way of dealing with scenarios like this (e.g. a using block). You are free to implement a Close method as well if it feels more natural to use this name - this would simply call Dispose(). Are there any other reasons for Dispose() than deterministic cleanup (or: are there any situations where you have to call Dispose())? Yes (and this is where it gets a little more complicated :doh: ). If the object you no longer need is referenced by a longer living object, it may unintentionally keep it alive. Y
-
If the class creator wants it to be used in a particular way then they can build a factory for it. Up until that point I'll use the class however I want to.
-
Some people in the C# forum once informed me that not using "using" could lead to memory leaks due to the way .Net manages memory. They say that, because unmanaged resources are outside of the memory tracked by .Net, that memory can grow very large without triggering a garbage collection by .Net. So, maybe you have some objects with finalizers and such that make it the least often collected object. And maybe you only have a few of them and they don't take up much managed memory in .Net. However, they could still reference a ton of unmanaged memory, and .Net does not count that large amount of unmanaged memory when deciding whether or not to perform a garbage collection. That can lead to the memory growing out of hand... perhaps too far out of hand before .Net decides to do a garbage collection. Not sure if that's correct, but that's about what I think they were trying to convey to me.
Thats the exact situation that is found when using the sharepoint 2007 object model - there are .net wrappers around the unmanaged sharepoint code base, which leads .net to think there is only a small amount of memory used, when 5MB is actually used for each object! So every time you end up accessing a site your memory grows... until it dies.... :laugh:
-
Daniel Grunwald wrote:
But it does lead to excessive resource usage and potentially even to resource exhaustion (where resource != memory).
That was my point too.
Daniel Grunwald wrote:
It's a bad idea to not dispose IDisposable objects.
I know you should, but you dont have to, unless you want it to be deterministic.
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition...and sometimes it's really important to be deterministic, such as with files, serial ports (or any hardware port probably), and sockets... otherwise a future attempt to re-open the same item may fail because the resource is locked by an uncollected out of scope object that wasn't disposed. Yes, dispose can help with memory clean up, but more importantly (in some cases) it releases locks on scarce resources. True also of database connections to a point... I have seen web apps that do not correctly dispose database connections and eventually fail because the 'connection pool is full'... many of the connections aren't active, but they haven't been returned to the pool because there's some number of .Net SqlConnection objects still undisposed in memory.
-
Looking at all the responses in this thread, there seem to be a lot of confusion about dispose and finalization. The basic recommendations is really not that complicated :-\ : 1) In the finalizer (i.e. ~SomeClass() ), clean up unmanaged resources. This method is called by the garbage collector, when the object can no longer be reached. However, it may not happen immediately, the garbage collector waits until it needs memory (which could be hours or days after it is no longer reachable). 2) In Dispose() (i.e. the method defined in IDisposable), cleanup managed and unmanaged resources. Furthermore, call GC.SuppressFinalize(this) (since everything is now cleaned up, there is no need for the finalizer to be called by the garbage collector, and doing this optimizes the garbage collection). The dispose pattern furthermore combines the implementation of these two, by implementing a protected Dispose(bool disposing) in the base class. The finalizer calls it with false, the Dispose() in IDisposable calls it with true. This makes it simple for derived classes, it only has to override this method and add whatever it needs, but it doesn't really change anything, it just a matter of style in the implementation. Why shouldn't the finalizer clean up managed resources as well? Because that is what the garbage collector does - managed resources is the memory used by other managed classes. Furthermore, it is very limited what a finalizer is allowed to do - it must not use references to other objects (as they may already be garbage collected). Why implement Dispose(), if the garbage collector takes care of it all? Because sometimes you need some cleanup to take place at a deterministic time, not hours or days later. The classical example is an open file. You typically want to allow the file being opened again, immediately you are done using it. Why do you need IDisposable for this, why not just implement a Close method on the class? Because with IDisposable, you have a uniform way of dealing with scenarios like this (e.g. a using block). You are free to implement a Close method as well if it feels more natural to use this name - this would simply call Dispose(). Are there any other reasons for Dispose() than deterministic cleanup (or: are there any situations where you have to call Dispose())? Yes (and this is where it gets a little more complicated :doh: ). If the object you no longer need is referenced by a longer living object, it may unintentionally keep it alive. Y
bjarneds wrote:
In previous versions of .NET, this problem could be illustrated simply by adding e.g. a DateTimePicker to Form2. The DateTimePicker subscribed to the UserPreferenceChangedEvent, so it was kept alive, and it's Parent reference to Form2 thus kept Form2 alive. This is not the case in .NET 3.5 - maybe they changed the design, or used some sort of weak reference?
So if the event used a weak reference instead the problem would go away?
-
Yes, this keeps me thinking that this is just one case, for one form. What else might have this behaviour? and depending on the scenario, this might rise significantly. So I have no doubt that calling Dispose (when you desire to release resources of course) is a very strong should, no matter how the Garbage Collector gets invoked. One theory I have about this behaviour is that one of these handles is kept for performance issues. When Dispose does not get called and the object is collected, the Garbage Collector (or the Finalizer / IDisposable implementation) might think it will be reused (instatiated again at some point of time) and so the handle is kept for reuse with the same form. If you experiment a little more, you'll also notice that another handle increment will happen if you instatiate another Form implementation (besides from TestForm) and don't call Dispose. Same thing will happen for that form. Anyways, this was a nice discussion to have. Regards, Fábio
Indeed that is probably a good explaination. It's kind of hard to tell sometimes what they're doing under the hood.
-
Incorrect.
IDisposable
clearly is a contract with the user of the object. It's that user who is supposed to callDispose()
(granted, it's not a must call, but usually it's seen as should call). The GC isn't interested inIDisposable
at all, it only cares about finalizers. It's perfectly reasonable to have classes with finalizer that don't implementIDisposable
if you don't need deterministic release of your resources.System.WeakReference
is an example of a class that has a finalizer but offers noDispose()
or similar method.Wrong! User's are *not* necessarily "supposed" to call Dispose(). However, *if* they don't call Dispose(), then the GC will. The only time you *must* do it is when the docs of the class say you *must* do so, like in the situation where a Dispose() wraps a db transaction. Then the typical usage is in a using or try/finally statement.
Fight Big Government:
http://obamacareclassaction.com/
http://obamacaretruth.org/ -
The ratio of unmanaged memory to managed memory is pretty much insignificant, in 95%+ cases. Anyways, there are classes to inform the CLR about this, if it is excessive.
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Editionleppie wrote:
The ratio of unmanaged memory to managed memory is pretty much insignificant, in 95%+ cases.
Ummm, in some application areas (my own included) unmanaged memory usage can be many times larger than the managed classes which are controlling it. Proper use of IDisposable and SafeHandle derived types means better control of memory use and better app stability.
Days spent at sea are not deducted from one's alloted span - Phoenician proverb
-
PIEBALDconsult wrote:
In my opinion, object should have a virtual do-nothing Dispose method so that any class or struct can be used with the using statement. (We would therefore not need the IDisposable interface.)
For everybody, not just piebald: it's been well over 1.5 years since I wrote C# production code, so take my words with a large dose of scepticism. I agree with Piebald here. Any gurus care to explain why this isn't the case?
Cheers, Vikram. (Got my troika of CCCs!)
I'm no guru, but if I remember correctly what our local gurus have said, deriving from IDisposable is means the object is treated differently by the GC in some way. I could be wrong of course... :)
Days spent at sea are not deducted from one's alloted span - Phoenician proverb
-
Indeed that is probably a good explaination. It's kind of hard to tell sometimes what they're doing under the hood.
I found this about the Component Class[^], which Form derives from (not directly, but it does). "A Component should release resources explicitly by calls to its Dispose method, without waitin"g for automatic memory management through an implicit call to the Finalize method. When a Container is disposed, all components within the Container are also disposed." It's interesting that if we let the Garbage Collector deal with the form's disposal, we will be doing exactly the opposite as the above recommendation.