This is why I am starting to loathe programming

[hammerstein05]

I think that .NET makes it a lot easier for people to get into programming, you don't need to know a lot about what it does, you can put buttons down on forms or web pages and wire up some code and you're good to go. Unfortunately, we they become actual programmers, and they don't understand some of the core functionality of what they're working with. The attitude of "you can't create memory leaks in .NET" is really common. It's oddly apt that this week I've been tasked with finding out why a couple of .NET websites we have consume up to 100mb a page load(eventually causing out of memory exceptions on customer servers) and I'm finding so many interesting discussions on IDisposable and GC at the same time! I've been busily going over objects, implementing finalize and Dispose, trying to figure out what is going wrong and it doesn't seem to make the slightest bit of difference whether I explicitly call Dispose or whether I just leave it be, although I think I have to get a better understanding of what is truly happening to stop some of the objects actually being collected. I like to think that implementing an interface shows my intent. So I don't quite agree with the idea of making object support Dispose by default. I like the using statement, if I create write a class that has a db connection or reads files etc, I'll always make it disposable and in my code wrap it in using. I've been advising the none .NET developers at work (starting to develop .NET applications) to work like this also.

modified on Saturday, May 29, 2010 10:03 AM

[leppie]

peterchen wrote:

You misspelled "if".

No, I meant 'when'. Dispose should always be called from a finalizer.

peterchen wrote:

Omitting Disposal of a resource holdign a file handle:

Again, you should call Close if you want to release the file handle. Dispose will Close the file handle if still open. Not calling Dispose after Close will not cause a resource leak. Please look my example on SO again. What you are almost saying is IDisposable objects 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 Edition

[leppie]

hammerstein05 wrote:

It's oddly apt that this week I've been tasked with finding out why a couple of .NET websites we have consume up to 100mb a page load(eventually causing out of memory exceptions on customer servers) and I'm finding so many interesting discussions on IDisposable and GC at the same time!

I am willing to bet it aint resource/memory leaks, but rather stupid code causing this. Like having 200 controls on a page, and tonnes of event handlers initialized on every request. But anyways, unless you see a 100mb per page load increase, that is not what is being used.

xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition

[bob16972]

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.

[leppie]

bob16972 wrote:

Deterministic destruction is so much simpler IMO. Throw in a good memory leak detector and debug assertion checks and you've got it made.

C/C++ has the best GC in the world. The stack :)

xacc.ide
IronScheme - 1.0 RC 1 - out now!
((λ (x) `(,x ',x)) '(λ (x) `(,x ',x))) The Scheme Programming Language – Fourth Edition

[Dave Parker]

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.

[PIEBALDconsult]

leppie wrote:

you should call Close if you want to release the file handle.

Exactly; Dispose should not be required simply to close a file and release its handle.

[Member 96]

If you have ever worked with MS Office automation through .net (particularly Outlook) you will get intimately familiar with this issue and all too accustomed to nulling and calling dispose. This can go *very* wrong very quickly in that environment if you aren't careful about disposing. It's almost the perfect learning environment for this kind of thing. Aside from resources and unmanaged code wrappers though I agree with you, however the beauty of .net is that those people doing it the other way aren't causing anything to go wrong where with unamanged code we all know that years later a serious security incident can occur from a simple mistake with buffers etc.

---

Yesterday they said today was tomorrow but today they know better. - Poul Anderson

[peterchen]

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 Document object, 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.

[Daniel Grunwald]

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.

[Daniel Grunwald]

Incorrect. IDisposable clearly is a contract with the user of the object. It's that user who is supposed to call Dispose() (granted, it's not a must call, but usually it's seen as should call). The GC isn't interested in IDisposable at all, it only cares about finalizers. It's perfectly reasonable to have classes with finalizer that don't implement IDisposable if you don't need deterministic release of your resources. System.WeakReference is an example of a class that has a finalizer but offers no Dispose() or similar method.

[Daniel Grunwald]

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.

[Mladen Jankovic]

RAII FTW! :)

[Genetic Algorithm Library] [Wowd]

[S Senthil Kumar]

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

[Vikram A Punathambekar]

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!)

[leppie]

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

[Adriaan Davel]

Why not rather change the using construct 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 the using construct, but I'm usually unconfortable while doing it :) )

____________________________________________________________ Be brave little warrior, be VERY brave

[Fabio Franco]

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.

[Fabio Franco]

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()

[leppie]

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