This is why I am starting to loathe programming
-
peterchen wrote:
You misspelled "if".
No, I meant 'when'.
Disposeshould always be called from a finalizer.peterchen wrote:
Omitting Disposal of a resource holdign a file handle:
Again, you should call
Closeif you want to release the file handle.DisposewillClosethe file handle if still open. Not callingDisposeafterClosewill not cause a resource leak. Please look my example on SO again. What you are almost saying isIDisposableobjects stay alive regardless, which is not true. If the programmer makes this object non-GC'able, obviously the GC cant do it's job. Stupid code leads to stupid bugs. You might want to ask why your object is not being GC'ed instead. Like why it is being assigned to an instance variable where a local variable would suffice, etc.xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth EditionThere's no guarantee for DIspose to run, but you are right: the finalizer does. Finalizers should be used only for unmanaged resources (as there is a real cost to them), IDisposable pattern is there to allow a deterministic call of that finalizer.
leppie wrote:
Again, you should call Close if you want to release the file handle.
That's ok if the object you have represents a file handle. However, if you have an
Documentobject, that may hold a file, or an internet connection, or somethign yu don't even know about, this doesn't help.leppie wrote:
What you are almost saying is IDisposable objects stay alive regardless
I am absolutely not saying that. I see IDisposable as the default way to implement deterministic/early cleanup, and as an indicastor that you actually need it. I'd generally let the GC make the call on managed resources - it knows best, may be configured by the server admin, and whatnot. (The only exception would be a point where the user would prefer a delay nwo rather than later, and I am releasing a big chunk of data). However, you may run out of unmanaged resources, and the GC idnling happily because it doesn't even know those tiny .NET objects hold on to huge unmanaged resource hogs.
I stick with my previous statement as a general rule: If the class implements IDisposable, call Dispose as soon as you are done with it, unless you know exactly what you are doing (i.e. know implementation details), or the author of the class explicitely documented in what circumstances Dispose() can be omitted.
Agh! Reality! My Archnemesis![^]
| FoldWithUs! | sighist | µLaunch - program launcher for server core and hyper-v server. -
OK, let me put it in a slightly different way: If MS had implemented it Piebald's way, how would we be worse off?
Cheers, Vikram. (Got my troika of CCCs!)
We would have to dispose each and every object manually, because the contract "call dispose when finished" would suddenly apply to everything instead of just
IDisposable. You'd have to write additional code, possibly even your own reference counting implementation, just to know when to call Dispose() in non-trivial cases. Basically, the GC would be useless and you'd be back in the land of manual memory management. -
You are basically correct, leppie, but getting the great unwashed to understand that the dispose pattern is *not* a contract that *requires* the user to call it is like leading a horse to water. The dispose pattern is a contract with the GC, not with the user of the object.
Fight Big Government:
http://obamacareclassaction.com/
http://obamacaretruth.org/Incorrect.
IDisposableclearly 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 inIDisposableat all, it only cares about finalizers. It's perfectly reasonable to have classes with finalizer that don't implementIDisposableif you don't need deterministic release of your resources.System.WeakReferenceis an example of a class that has a finalizer but offers noDispose()or similar method. -
I'm always astonished by the amount of code I encounter where the author seems to think setting a local reference to null just before the function returns makes any difference whatsoever. Though at least it doesn't do any harm I guess even though it's unnecessary. To me the main purpose of dispose / finalization is to free up other things than memory such as a lock on a mutex, a Win32 GDI handle, that kind of thing. Most memory leaks I've seen in .NET programs have been along the lines of a function that processes a long list of items and for each one retrieves other pieces of information related to the item but doesn't release the references to those extra pieces until the entire loop has completed rather than before the end of the iteration. Yes, the garbage collector will free that stuff up once it goes out of scope but that's not until the entire script or whatever has finished. I don't do much work in C++ but most memory leaks I've personally encountered there have been caused by the author never having heard of virtual destructors and so only freeing up the members of the base class and not also including those in derived classes.
Dave Parker wrote:
I'm always astonished by the amount of code I encounter where the author seems to think setting a local reference to null just before the function returns makes any difference whatsoever. Though at least it doesn't do any harm I guess even though it's unnecessary.
I once had to work with code from a developer who did that. He also always explicitly removed all references in lists, so to "free" any ArrayList he would write:
list.Clear(); list = null;
Of course, there were some cases where he did the same for lists passed in as parameter - clearing lists that were still being used by the calling method.
-
I've become convinced over the years that the rules for you "managed" folks are much more complicated than for us "unsafe" folks. Deterministic destruction is so much simpler IMO. Throw in a good memory leak detector and debug assertion checks and you've got it made.
RAII FTW! :)
-
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 EditionWell, there are cases like http://msmvps.com/blogs/senthil/archive/2010/04/11/why-you-should-always-dispose-datagridviews.aspx[^] - a DataGridView control, of all things, causes memory leaks unless you Dispose. You could argue it's poor design, but still..
Regards Senthil _____________________________ My Home Page |My Blog | My Articles | My Flickr | WinMacro
-
We would have to dispose each and every object manually, because the contract "call dispose when finished" would suddenly apply to everything instead of just
IDisposable. You'd have to write additional code, possibly even your own reference counting implementation, just to know when to call Dispose() in non-trivial cases. Basically, the GC would be useless and you'd be back in the land of manual memory management.Yeah, but it's not mandated to call Dispose(). Well, I ensure I call it where applicable, but strictly speaking, it's not necessary.
Cheers, Vikram. (Got my troika of CCCs!)
-
Well, there are cases like http://msmvps.com/blogs/senthil/archive/2010/04/11/why-you-should-always-dispose-datagridviews.aspx[^] - a DataGridView control, of all things, causes memory leaks unless you Dispose. You could argue it's poor design, but still..
Regards Senthil _____________________________ My Home Page |My Blog | My Articles | My Flickr | WinMacro
Perfect example of stupid code... But thanks, tomorrow I will add a dispose for each of our 400 or so in the app :)
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition -
Yeah, but it's not mandated to call Dispose(). Well, I ensure I call it where applicable, but strictly speaking, it's not necessary.
Cheers, Vikram. (Got my troika of CCCs!)
Why not rather change the
usingconstruct to accomodate classes that do not implement IDisposable? Implementing IDisposable should indicate that disposing is required... (I also implement IDisposable just to be bale to use theusingconstruct, but I'm usually unconfortable while doing it :) )____________________________________________________________ Be brave little warrior, be VERY brave
-
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.
using "using", certifies that the Dispose method gets called on classes that implement IDisposable interface. You don't really need to use it as long as you call Dispose method manually. The most common example is the Window Handle of the Form class. If you use ShowDialog to view a form and don't call the Dispose method, the handle will not be released even when the Form is GCed. GC does not handle unmanaged resources automatically, it does not call Dispose method of objects when they are collected, so yes, it can lead to memory leak. This rarely becomes a problem, but I've ran into this once and it was a pain to pinpoint the leak source. Controls also generate handles, if you have an application that creates lots of controls dinamically it's something you should pay a lots of attention. So, if a class implements IDisposable, call Dispose always, to make sure it releases unmanaged resource just in case it uses any.
-
peterchen wrote:
the class creator explicitely told you to call Dispose().
No, he actually said: 'If you want to cleanup the resources immediately, then call Dispose(), but if you forget or dont want to, I'll do it anyways when the object finalizer is called.'
xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Editionleppie wrote:
No, he actually said: 'If you want to cleanup the resources immediately, then call Dispose(), but if you forget or dont want to, I'll do it anyways when the object finalizer is called.'
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. Just do a little experiment and see for yourself: public partial class Form1 : Form { public Form1() { InitializeComponent(); } private void button1_Click(object sender, EventArgs e) { Form frm = new Form(); frm.ShowDialog(); //frm.Dispose(); //GC.Collect(); } private void button2_Click(object sender, EventArgs e) { this.Text = HWndCounter.GetWindowHandlesForCurrentProcess().ToString(); } public class HWndCounter { [DllImport("kernel32.dll")] private static extern IntPtr GetCurrentProcess(); [DllImport("user32.dll")] private static extern uint GetGuiResources(IntPtr hProcess, uint uiFlags); public static int GetWindowHandlesForCurrentProcess() { IntPtr processHandle = GetCurrentProcess(); uint userObjects = GetGuiResources(processHandle, 1); // 1 for User resource, disregard GDI return Convert.ToInt32(userObjects); } } } You'll notice that the only way to keep the handle count to a minimum, is through calling Dispose method. If you uncomment GC.Collect() and comment Dispose(), it will keep the handle count from growing indefinitely, but won't stay as small as when calling Dispose() and commenting GC.Collect()
-
leppie wrote:
No, he actually said: 'If you want to cleanup the resources immediately, then call Dispose(), but if you forget or dont want to, I'll do it anyways when the object finalizer is called.'
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. Just do a little experiment and see for yourself: public partial class Form1 : Form { public Form1() { InitializeComponent(); } private void button1_Click(object sender, EventArgs e) { Form frm = new Form(); frm.ShowDialog(); //frm.Dispose(); //GC.Collect(); } private void button2_Click(object sender, EventArgs e) { this.Text = HWndCounter.GetWindowHandlesForCurrentProcess().ToString(); } public class HWndCounter { [DllImport("kernel32.dll")] private static extern IntPtr GetCurrentProcess(); [DllImport("user32.dll")] private static extern uint GetGuiResources(IntPtr hProcess, uint uiFlags); public static int GetWindowHandlesForCurrentProcess() { IntPtr processHandle = GetCurrentProcess(); uint userObjects = GetGuiResources(processHandle, 1); // 1 for User resource, disregard GDI return Convert.ToInt32(userObjects); } } } You'll notice that the only way to keep the handle count to a minimum, is through calling Dispose method. If you uncomment GC.Collect() and comment Dispose(), it will keep the handle count from growing indefinitely, but won't stay as small as when calling Dispose() and commenting GC.Collect()
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 Edition -
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 Editionleppie wrote:
If you implement IDisposable incorrectly, and do not call it from the finalizer if not already disposed, you have stupid code.
Well, then whoever from Microsoft implemented the Form class is stupid. In fact, MSFT doesn't even recommend anyone to implement the Finalizer explicitly. Blindly relying on how someone impliments a finalizer or anything is even more stupid.
leppie wrote:
I am not even going to bother.
I will take that as someone who is not willing to be wrong.
leppie wrote:
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.
Well, then you have never experienced running out of handles. Just because you've not ran into a problem, it doesn't mean it can't happen. Beleive me, it does happen.
-
There's no guarantee for DIspose to run, but you are right: the finalizer does. Finalizers should be used only for unmanaged resources (as there is a real cost to them), IDisposable pattern is there to allow a deterministic call of that finalizer.
leppie wrote:
Again, you should call Close if you want to release the file handle.
That's ok if the object you have represents a file handle. However, if you have an
Documentobject, that may hold a file, or an internet connection, or somethign yu don't even know about, this doesn't help.leppie wrote:
What you are almost saying is IDisposable objects stay alive regardless
I am absolutely not saying that. I see IDisposable as the default way to implement deterministic/early cleanup, and as an indicastor that you actually need it. I'd generally let the GC make the call on managed resources - it knows best, may be configured by the server admin, and whatnot. (The only exception would be a point where the user would prefer a delay nwo rather than later, and I am releasing a big chunk of data). However, you may run out of unmanaged resources, and the GC idnling happily because it doesn't even know those tiny .NET objects hold on to huge unmanaged resource hogs.
I stick with my previous statement as a general rule: If the class implements IDisposable, call Dispose as soon as you are done with it, unless you know exactly what you are doing (i.e. know implementation details), or the author of the class explicitely documented in what circumstances Dispose() can be omitted.
Agh! Reality! My Archnemesis![^]
| FoldWithUs! | sighist | µLaunch - program launcher for server core and hyper-v server.peterchen wrote:
However, you may run out of unmanaged resources, and the GC idnling happily because it doesn't even know those tiny .NET objects hold on to huge unmanaged resource hogs.
Exactly, I've seen that happen for Window Handles. Calling Dispose would have avoided this problem and the huge amount of time trying to pinpoint where the leak was happening.
-
peterchen wrote:
You misspelled "if".
No, I meant 'when'.
Disposeshould always be called from a finalizer.peterchen wrote:
Omitting Disposal of a resource holdign a file handle:
Again, you should call
Closeif you want to release the file handle.DisposewillClosethe file handle if still open. Not callingDisposeafterClosewill not cause a resource leak. Please look my example on SO again. What you are almost saying isIDisposableobjects stay alive regardless, which is not true. If the programmer makes this object non-GC'able, obviously the GC cant do it's job. Stupid code leads to stupid bugs. You might want to ask why your object is not being GC'ed instead. Like why it is being assigned to an instance variable where a local variable would suffice, etc.xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Editionleppie wrote:
peterchen wrote: You misspelled "if". No, I meant 'when'. Dispose should always be called from a finalizer.
There is no telling that the GC will run even during a lifetime of the application, you can run into problems before that happens. I agree with the 'if'. This when the finalizer calls the Dispose method correctly, which you can never tell if the code isn't yours.
-
leppie wrote:
If you implement IDisposable incorrectly, and do not call it from the finalizer if not already disposed, you have stupid code.
Well, then whoever from Microsoft implemented the Form class is stupid. In fact, MSFT doesn't even recommend anyone to implement the Finalizer explicitly. Blindly relying on how someone impliments a finalizer or anything is even more stupid.
leppie wrote:
I am not even going to bother.
I will take that as someone who is not willing to be wrong.
leppie wrote:
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.
Well, then you have never experienced running out of handles. Just because you've not ran into a problem, it doesn't mean it can't happen. Beleive me, it does happen.
The Form class is implemented correctly, it inherits from Component which calls Dispose(false) from the Finalize method. The Form class overrides the Dispose(bool disposing) method and has it's own clean up code in there [which in turn calls the base class dispose(false)]. Which is the documented way of implementing IDisposable.
-
The Form class is implemented correctly, it inherits from Component which calls Dispose(false) from the Finalize method. The Form class overrides the Dispose(bool disposing) method and has it's own clean up code in there [which in turn calls the base class dispose(false)]. Which is the documented way of implementing IDisposable.
I beleive that too, but leppie is saying we shouldn't call Dispose as the finalizer will always do that for us. In the experiment I proposed, if we don't call Dispose, some handles will be left behind after a few repeated calls of the form's ShowDialog method, even after collection occurs. My point is, call Dispose always and don't rely on finalizer or garbage collection as you can run into trouble. Calling the Form's Dispose method eliminates the extra handles problem, that was my point.
-
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 EditionI loathe programming as well, but for completely different reasons. I loathe programming because of programmers. Programmers are lazy, stupid and don't understand abstractions, even when they leak. Manage resources frustrate me. Manage resource tedious. Me hate manage resources. Me invent garbage collect. Ooga, booga, garbage collect! Some resources can't garbage collect. Cleanup some resources can't automate. Cleanup some resources need intelligence. Same class both managed resources unmanaged resources. Me confused. Ooga, booga, me confused! Programmers deserve to be punished by making them program ERPs in assembly (optimizing CPU cycles, of course) and efficient operating systems in Scheme.
If you can play The Dance of Eternity (Dream Theater), then we shall make a band.
-
OK, let me put it in a slightly different way: If MS had implemented it Piebald's way, how would we be worse off?
Cheers, Vikram. (Got my troika of CCCs!)
If every object supported
IDisposable, then there would be no easy way to know which really needed it and which didn't - so it would be impossible to know which to wrap in ausingblock and which could safely be used without one. Personally, I'm glad that only things that need to be disposed implementIDisposable. -
By implementing IDisposable, the class creator explicitely told you to call
Dispose(). If the documentation of a class said "You need to call Init() before using an instance of this class", would you reply with "Ah, I don#t feel like it today"?Agh! Reality! My Archnemesis![^]
| FoldWithUs! | sighist | µLaunch - program launcher for server core and hyper-v server.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.