14
  • How should I manage static classes with disposable items? Are there any rules of thumb?

  • Basically, should I refactor and make the following DisposableDataManager class non- static or is it fine to leave everything to GC?

.

public static class DisposableDataManager
{
    // ImageList is an 'IDisposable'.
    public static ImageList FirstImageList { get; private set; }
    public static ImageList SecondImageList { get; private set; }

    static DisposableDataManager()
    {
        FirstImageList = CreateFirstImageList();
        SecondImageList = CreateSecondImageList();        
    }

    // ...
}
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
Yippie-Ki-Yay
  • 22,026
  • 26
  • 90
  • 148
  • 6
    that design seems possibly flawed. – Mitch Wheat Aug 23 '12 at 13:22
  • 12
    The GC won't collect your ImageLists here, since they are always referenced from the moment the type is initialized to the moment the AppDomain is unloaded... – Thomas Levesque Aug 23 '12 at 13:22
  • @ClaudioRedi: [**`ImageLists`**](http://msdn.microsoft.com/en-us/library/system.windows.forms.imagelist.aspx) are `WinForms`. – Tim Schmelter Aug 23 '12 at 13:28
  • @Tim Schmelter: thanks man, my fault. – Claudio Redi Aug 23 '12 at 13:30
  • Actually your `DisposableDataManager` does the opposite of what it appears to do. It prevents the images from being disposed until the appdomain unloads. But it seems that it's your desired behaviour. (Consider renaming to `ImageRepository`) – Tim Schmelter Aug 23 '12 at 13:37
  • @TimSchmelter Okay, let's say I rename it to `ImageRepository` and use it as it is. Do I have a guarantee (and if yes, how strong is it) that the `CLR` will take care of my `ImageLists` after I exit my application? Basically, I want to know, what would happen, if, for example, we extrapolate this class to hold 5000 ImageLists - **would they be disposed upon application exit?** – Yippie-Ki-Yay Aug 23 '12 at 13:43
  • C# is broken here. – John Z. Li Nov 28 '18 at 03:41

7 Answers7

19

It really depends on how important it is to you that the resources are disposed. When your application closes, all handles it had open (files, network connections, graphics etc) will be released anyway, so that's not a problem. It's more of a problem if you want disposal for a more orderly release - e.g. flushing a stream before closing it. The CLR makes a "best effort" to run finalizers before the process exits, which will in turn call Dispose in some cases - but that's not something I'd want to rely on for anything important.

So in the case of ImageList objects, it really shouldn't be an issue. You definitely won't leak any resources - the operating system will take care of that.

Having said that, I'd still try to refactor - simply because global state tends to be a bad idea. It makes dependencies implicit, and testing harder. How hard would it be to provide the relevant information to each object that needed it at construction time?

(Note: static variables are really associated with an AppDomain rather than the process as a whole. This makes the whole question more complex in applications where AppDomains are brought up and down, but I doubt that it's relevant to your scenario.)

Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
5

as a static class you are saying that everything is available to the application. So why would you ever want to dispose of it?

Richard Schneider
  • 34,944
  • 9
  • 57
  • 73
3

You can hook the AppDomain.DomainUnload event and call dispose on anything you want to make sure is cleaned up before you exit.

Oliver Mellet
  • 2,389
  • 12
  • 10
0

What I see from that code is you will not be able to dispose ImageList since the DisposableDataManager is a static class that will be accessible until the app is closed.

jonsca
  • 10,218
  • 26
  • 54
  • 62
0

I believe your current way of working may work, but there are definitely better ways of solving what ever your program is intending to do. If it does work, it would be an abuse of the language - using features against what they were intended to be.

If you continue on this architecture path, your code will be difficult to understand and modify.

It would probably be worth writing up what you're after with this method and ask for alternative ideas.

SamuelDavis
  • 3,312
  • 3
  • 17
  • 19
-1

Disposable resources should never be held in a static class anyway. Since they are unlikely to be disposed if the program ends (for instances if the program crashes). So you have the possibility to add and release the resources yourself in an object which will be existent as long your application runs and

  • is wrapped in a using statement before entering your program loop.
  • or you take care of it by yourself, and calling the dispose in a finally construct as soon as your program ends.

Or you may change the ImageList to be not be disposable at all. This would be an option only if you can dispose all resources it holds by yourself.

Alex
  • 5,240
  • 1
  • 31
  • 38
  • Why shouldn't disposable resources be held in a static class? What about an `EventSource`? Furthermore, `ImageList` is from the BCL, making it non-`IDisposable` is not an option. – Mark Oct 21 '14 at 16:43
-2

I don't think that you need to do any disposable objects manager. GC already manage memory for you. As you probably already know, Dispose method is the one coming from the IDisposable interface include in .Net framework. But it's a method like any one else*. The garbage collector don't wait that Dispose to be called to free the memory of an object. He monitor if the object is always reachable from somewhere or not. This help him to determine which object are alive and which are dead.

Before to continue read a bit about GC generation. http://msdn.microsoft.com/en-us/library/ms973837.aspx

I'm not sure, but when new() is call, and GC reach the capacity of the actual used generation, he clear the next generation "dead" objects. Probably at other times that i don't. GC is such a mystic and mysterious implementation, because is probably rote in C++. But you don't have to care about it.

In some cases (mono-develop), I heard that you should care more about it. But not if you code in a Microsoft environment.

*I told:

method like any one else

but the using block give you the freedom to don't care to call Dispose method of IDisposable objects and call the method for you. The existing being of method Dispose is to release resources that needs to be released before stop using an object.

ex:

    public class Test : IDisposable
    {
        public void Dispose()
        {
            // release other necessary ressources if needed

            Console.WriteLine("Disposed");
        }
    }


    {
        using (IDisposable disposable = new Test())
        {

        }
    }

same as:

{
    IDisposable disposable = new Test()
    disposable.Dispose();
}

So you can be confident about GC. If you want to be sure that GC will free you object memory, just make all references to it at null. And GC will consider your object as "dead" :P

  • IDisposable is about releasing unmanaged *resources*, not managed memory. We're talking about memory allocated by unsafe code, Windows handles, network connections - things like that. The OS may reclaim some resources when the application terminates, but that's not guaranteed. It is the developer's responsibility to dispose of IDisposable objects when they are no longer needed. – TrueWill Aug 31 '12 at 00:49