415

C# 2008

I have been working on this for a while now, and I am still confused about the use of finalize and dispose methods in code. My questions are below:

  1. I know that we only need a finalizer while disposing unmanaged resources. However, if there are managed resources that make calls to unmanaged resources, would it still need to implement a finalizer?

  2. However, if I develop a class that doesn't use any unmanaged resource - directly or indirectly, should I implement the IDisposable to allow the clients of that class to use the 'using statement'?

    Would it be feasible to implement IDisposable just to enable clients of your class to use the using statement?

    using(myClass objClass = new myClass())
    {
        // Do stuff here
    }
    
  3. I have developed this simple code below to demonstrate the Finalize/dispose use:

    public class NoGateway : IDisposable
    {
        private WebClient wc = null;
    
        public NoGateway()
        {
            wc = new WebClient();
            wc.DownloadStringCompleted += wc_DownloadStringCompleted;
        }
    
    
        // Start the Async call to find if NoGateway is true or false
        public void NoGatewayStatus()
        {
            // Start the Async's download
                // Do other work here
            wc.DownloadStringAsync(new Uri(www.xxxx.xxx));
        }
    
        private void wc_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
        {
            // Do work here
        }
    
        // Dispose of the NoGateway object
        public void Dispose()
        {
            wc.DownloadStringCompleted -= wc_DownloadStringCompleted;
            wc.Dispose();
            GC.SuppressFinalize(this);
        }
    }
    

Question about the source code:

  1. Here I have not added the finalizer, and normally the finalizer will be called by the GC, and the finalizer will call the Dispose. As I don't have the finalizer, when do I call the Dispose method? Is it the client of the class that has to call it?

    So my class in the example is called NoGateway and the client could use and dispose of the class like this:

    using(NoGateway objNoGateway = new NoGateway())
    {
        // Do stuff here   
    }
    

    Would the Dispose method be automatically called when execution reaches the end of the using block, or does the client have to manually call the dispose method? i.e.

    NoGateway objNoGateway = new NoGateway();
    // Do stuff with object
    objNoGateway.Dispose(); // finished with it
    
  2. I am using the WebClient class in my NoGateway class. Because WebClient implements the IDisposable interface, does this mean that WebClient indirectly uses unmanaged resources? Is there a hard and fast rule to follow this? How do I know that a class uses unmanaged resources?

Niels R.
  • 7,260
  • 5
  • 32
  • 44
ant2009
  • 27,094
  • 154
  • 411
  • 609

13 Answers13

454

The recommended IDisposable pattern is here. When programming a class that uses IDisposable, generally you should use two patterns:

When implementing a sealed class that doesn't use unmanaged resources, you simply implement a Dispose method as with normal interface implementations:

public sealed class A : IDisposable
{
    public void Dispose()
    {
        // get rid of managed resources, call Dispose on member variables...
    }
}

When implementing an unsealed class, do it like this:

public class B : IDisposable
{    
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            // get rid of managed resources
        }   
        // get rid of unmanaged resources
    }

    // only if you use unmanaged resources directly in B
    //~B()
    //{
    //    Dispose(false);
    //}
}

Notice that I haven't declared a finalizer in B; you should only implement a finalizer if you have actual unmanaged resources to dispose. The CLR deals with finalizable objects differently to non-finalizable objects, even if SuppressFinalize is called.

So, you shouldn't declare a finalizer unless you have to, but you give inheritors of your class a hook to call your Dispose and implement a finalizer themselves if they use unmanaged resources directly:

public class C : B
{
    private IntPtr m_Handle;

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            // get rid of managed resources
        }
        ReleaseHandle(m_Handle);

        base.Dispose(disposing);
    }

    ~C() {
        Dispose(false);
    }
}

If you're not using unmanaged resources directly (SafeHandle and friends doesn't count, as they declare their own finalizers), then don't implement a finalizer, as the GC deals with finalizable classes differently, even if you later suppress the finalizer. Also note that, even though B doesn't have a finalizer, it still calls SuppressFinalize to correctly deal with any subclasses that do implement a finalizer.

When a class implements the IDisposable interface, it means that somewhere there are some unmanaged resources that should be got rid of when you've finished using the class. The actual resources are encapsulated within the classes; you don't need to explicitly delete them. Simply calling Dispose() or wrapping the class in a using(...) {} will make sure any unmanaged resources are got rid of as necessary.

Omer
  • 8,194
  • 13
  • 74
  • 92
thecoop
  • 45,220
  • 19
  • 132
  • 189
  • 29
    I agree with thecoop. Note that you don't need a finalizer if you are only dealing with managed resources (in fact, you should NOT try to access managed objects from within your finalizer (other than "this"), because there is no guaranteed order in which the GC will clean up objects. Also, if you are using .Net 2.0 or better, you can (and should) use SafeHandles to wrapper unmanaged handles. Safehandles greatly reduce your need to write finalizers for your managed classes at all. http://blogs.msdn.com/bclteam/archive/2005/03/16/396900.aspx – JMarsch May 22 '09 at 18:30
  • 2
    as extra computing effort is needed in the CLR to keep track of classes with active finalizers. - Implementing a finalizer causes this to happen. Calling GC.SuppressFinalize means that the Finalizer should not be called by the runtime. It still goes Gen2 regardless. Don't add a finalizer if you aren't dealing with managed resources. Sealed or unsealed class modifiers are irrelevant to that point. – Ritch Melton Mar 09 '11 at 14:53
  • 3
    @Ritch: citation? That's not necessarily a bad thing; if you're implementing `IDisposable`, chances are it'll hang around for a while anyway. You're saving the CLR the effort of having to copy it from Gen0 -> Gen1 -> Gen2 – thecoop Mar 09 '11 at 15:30
  • @thecoop - Rotor source for the mechanics of finalizers. Various articles for Gen1, Gen2: http://msdn.microsoft.com/en-us/library/ms973837.aspx. Don't add a finalizer if you don't need one: Framework Design Guidelines and BillW: http://www.srtsolutions.com/why-no-finalizer-example-in-effective-c-2nd-edition. I'm not sure why sealed was even mentioned. – Ritch Melton Mar 09 '11 at 15:38
  • 1
    @thecoop: I disagree. I use disposables that are transactional in nature and don't last longer than a single fucntion call. – Ritch Melton Mar 09 '11 at 15:40
  • Please see my responses below for an improved version of the Dispose Pattern from MSDN. I have also posted this implementation on my blog at: http://dave-black.blogspot.com/2011/03/how-do-you-properly-implement.html – Dave Black Sep 27 '11 at 16:56
  • @thecoop: Classes with finalizers start out in Gen0, but no object with a non-suppressed finalizer, nor any object that's referred to directly or indirectly by an object with a non-suppressed finalizer, can be removed from memory. If an object with a non-suppressed finalizer would otherwise be eligible to be removed from memory, it will be resurrected and placed in a queue to run its finalizer (its finalizer will be suppressed at this time, so once the finalizer has run it's likely no more references will exist to the object and it can be swept from memory). – supercat Dec 13 '11 at 22:19
  • 2
    @thecoop: The net effect is that every object with an unsuppressed finalizer survives at least one collection longer than would otherwise be necessary, and every object to which a finalizable object holds a direct or indirect reference will last at least as long as that finalizable object itself. – supercat Dec 13 '11 at 22:21
  • @thecoop - If your class only has a member that impelments IDisposable and has no unmanaged resources of it's own AND the class is not sealed, is it necessary to use the second pattern? Could you explain when and why it's ok to just implement the single Dispose() method and do cleanup within it? I've scoured the internet and haven't found a good explanation of this. – Tyler Nielsen Dec 18 '18 at 16:32
  • Maybe I missed something but where is the locking from calling Dispose twice? eg: `if (disposed) return; `? – SerjG Mar 04 '19 at 06:07
  • Maybe I am missing something, but based on the chapter about implementing `IDisposable` for a non-sealed class (potential base class), `Dispose()` must not call `GC.SuppressFinalize(this)`. However, in the article it is also stated that calling `GC.SuppressFinalize(this)` has no effect, for a class without a finalizer. So why should I add the call to `Dispose()`? – royalTS Aug 14 '20 at 12:20
126

The official pattern to implement IDisposable is hard to understand. I believe this one is better:

public class BetterDisposableClass : IDisposable {

  public void Dispose() {
    CleanUpManagedResources();
    CleanUpNativeResources();
    GC.SuppressFinalize(this);
  }

  protected virtual void CleanUpManagedResources() { 
    // ...
  }
  protected virtual void CleanUpNativeResources() {
    // ...
  }

  ~BetterDisposableClass() {
    CleanUpNativeResources();
  }

}

An even better solution is to have a rule that you always have to create a wrapper class for any unmanaged resource that you need to handle:

public class NativeDisposable : IDisposable {

  public void Dispose() {
    CleanUpNativeResource();
    GC.SuppressFinalize(this);
  }

  protected virtual void CleanUpNativeResource() {
    // ...
  }

  ~NativeDisposable() {
    CleanUpNativeResource();
  }

}

With SafeHandle and its derivatives, these classes should be very rare.

The result for disposable classes that don't deal directly with unmanaged resources, even in the presence of inheritance, is powerful: they don't need to be concerned with unmanaged resources anymore. They'll be simple to implement and to understand:

public class ManagedDisposable : IDisposable {

  public virtual void Dispose() {
    // dispose of managed resources
  }

}
Jordão
  • 55,340
  • 13
  • 112
  • 144
  • 7
    Though one thing i want to note is it does not preventing from being called second time. – HuseyinUslu Mar 16 '11 at 20:52
  • 6
    @HuseyinUslu: this is just the _essence_ of the pattern. You can certainly add a `disposed` flag and check accordingly. – Jordão Mar 18 '11 at 01:30
  • @Jordão: Except that the implementation of the isDisposed flag becomes a lot more complicated this way. Actually I can't even think of a way that you could add it in and keep the pattern pretty. Can you show a way to do so using your pattern? – Didier A. Jun 21 '12 at 18:22
  • 2
    @didibus: it's a simple matter of adding a `disposed` flag, check it before disposing and set it after disposing. Look [here](http://pastebin.com/ujrPRphM) for the idea. You should also check the flag before any methods of the class. Makes sense? Is it complicated? – Jordão Jun 21 '12 at 20:59
41

Note that any IDisposable implementation should follow the below pattern (IMHO). I developed this pattern based on info from several excellent .NET "gods" the .NET Framework Design Guidelines (note that MSDN does not follow this for some reason!). The .NET Framework Design Guidelines were written by Krzysztof Cwalina (CLR Architect at the time) and Brad Abrams (I believe the CLR Program Manager at the time) and Bill Wagner ([Effective C#] and [More Effective C#] (just take a look for these on Amazon.com:

Note that you should NEVER implement a Finalizer unless your class directly contains (not inherits) UNmanaged resources. Once you implement a Finalizer in a class, even if it is never called, it is guaranteed to live for an extra collection. It is automatically placed on the Finalization Queue (which runs on a single thread). Also, one very important note...all code executed within a Finalizer (should you need to implement one) MUST be thread-safe AND exception-safe! BAD things will happen otherwise...(i.e. undetermined behavior and in the case of an exception, a fatal unrecoverable application crash).

The pattern I've put together (and written a code snippet for) follows:

#region IDisposable implementation

//TODO remember to make this class inherit from IDisposable -> $className$ : IDisposable

// Default initialization for a bool is 'false'
private bool IsDisposed { get; set; }

/// <summary>
/// Implementation of Dispose according to .NET Framework Design Guidelines.
/// </summary>
/// <remarks>Do not make this method virtual.
/// A derived class should not be able to override this method.
/// </remarks>
public void Dispose()
{
    Dispose( true );

    // This object will be cleaned up by the Dispose method.
    // Therefore, you should call GC.SupressFinalize to
    // take this object off the finalization queue 
    // and prevent finalization code for this object
    // from executing a second time.

    // Always use SuppressFinalize() in case a subclass
    // of this type implements a finalizer.
    GC.SuppressFinalize( this );
}

/// <summary>
/// Overloaded Implementation of Dispose.
/// </summary>
/// <param name="isDisposing"></param>
/// <remarks>
/// <para><list type="bulleted">Dispose(bool isDisposing) executes in two distinct scenarios.
/// <item>If <paramref name="isDisposing"/> equals true, the method has been called directly
/// or indirectly by a user's code. Managed and unmanaged resources
/// can be disposed.</item>
/// <item>If <paramref name="isDisposing"/> equals false, the method has been called by the 
/// runtime from inside the finalizer and you should not reference 
/// other objects. Only unmanaged resources can be disposed.</item></list></para>
/// </remarks>
protected virtual void Dispose( bool isDisposing )
{
    // TODO If you need thread safety, use a lock around these 
    // operations, as well as in your methods that use the resource.
    try
    {
        if( !this.IsDisposed )
        {
            if( isDisposing )
            {
                // TODO Release all managed resources here

                $end$
            }

            // TODO Release all unmanaged resources here



            // TODO explicitly set root references to null to expressly tell the GarbageCollector
            // that the resources have been disposed of and its ok to release the memory allocated for them.


        }
    }
    finally
    {
        // explicitly call the base class Dispose implementation
        base.Dispose( isDisposing );

        this.IsDisposed = true;
    }
}

//TODO Uncomment this code if this class will contain members which are UNmanaged
// 
///// <summary>Finalizer for $className$</summary>
///// <remarks>This finalizer will run only if the Dispose method does not get called.
///// It gives your base class the opportunity to finalize.
///// DO NOT provide finalizers in types derived from this class.
///// All code executed within a Finalizer MUST be thread-safe!</remarks>
//  ~$className$()
//  {
//     Dispose( false );
//  }
#endregion IDisposable implementation

Here is the code for implementing IDisposable in a derived class. Note that you do not need to explicitly list inheritance from IDisposable in the definition of the derived class.

public DerivedClass : BaseClass, IDisposable (remove the IDisposable because it is inherited from BaseClass)


protected override void Dispose( bool isDisposing )
{
    try
    {
        if ( !this.IsDisposed )
        {
            if ( isDisposing )
            {
                // Release all managed resources here

            }
        }
    }
    finally
    {
        // explicitly call the base class Dispose implementation
        base.Dispose( isDisposing );
    }
}

I've posted this implementation on my blog at: How to Properly Implement the Dispose Pattern

Dave Black
  • 7,305
  • 2
  • 52
  • 41
  • 3
    Microsoft seems to like setting the "disposed" flag at the end of the disposed method, but that seems wrong to me. Redundant calls to "Dispose" are supposed to do nothing; while one wouldn't normally expect Dispose to get called recursively, such things could happen if one is trying to Dispose an object which has been left in an invalid state by an exception which occurred during construction or some other operation. I would think using an `Interlocked.Exchange` on an integer `IsDisposed` flag in the non-virtual wrapper function would be safer. – supercat Feb 23 '12 at 23:33
  • 1
    @DaveBlack: What if your base class does not use unmanaged resources, but your derived class does? Do you have to implement the Finalizer in the derived class then? And if so, how do you know that the base class did not already implement it if you don't have access to the source? – Didier A. Jun 21 '12 at 18:38
  • why do you say objects with finalizers go to gen 2? They go up one gen but can die in gen 1. I confirmed it with sos. – Piotr Perak Aug 14 '14 at 05:34
23

I agree with pm100 (and should have explicitly said this in my earlier post).

You should never implement IDisposable in a class unless you need it. To be very specific, there are about 5 times when you would ever need/should implement IDisposable:

  1. Your class explicitly contains (i.e. not via inheritance) any managed resources which implement IDisposable and should be cleaned up once your class is no longer used. For example, if your class contains an instance of a Stream, DbCommand, DataTable, etc.

  2. Your class explicitly contains any managed resources which implement a Close() method - e.g. IDataReader, IDbConnection, etc. Note that some of these classes do implement IDisposable by having Dispose() as well as a Close() method.

  3. Your class explicitly contains an unmanaged resource - e.g. a COM object, pointers (yes, you can use pointers in managed C# but they must be declared in 'unsafe' blocks, etc. In the case of unmanaged resources, you should also make sure to call System.Runtime.InteropServices.Marshal.ReleaseComObject() on the RCW. Even though the RCW is, in theory, a managed wrapper, there is still reference counting going on under the covers.

  4. If your class subscribes to events using strong references. You need to unregister/detach yourself from the events. Always to make sure these are not null first before trying to unregister/detach them!.

  5. Your class contains any combination of the above...

A recommended alternative to working with COM objects and having to use Marshal.ReleaseComObject() is to use the System.Runtime.InteropServices.SafeHandle class.

The BCL (Base Class Library Team) has a good blog post about it here http://blogs.msdn.com/bclteam/archive/2005/03/16/396900.aspx

One very important note to make is that if you are working with WCF and cleaning up resources, you should ALMOST ALWAYS avoid the 'using' block. There are plenty of blog posts out there and some on MSDN about why this is a bad idea. I have also posted about it here - Don't use 'using()' with a WCF proxy

StayOnTarget
  • 11,743
  • 10
  • 52
  • 81
Dave Black
  • 7,305
  • 2
  • 52
  • 41
  • 3
    I believe there is a 5th case: If your class subscribes to events using strong references, then you should implement IDisposable and unregister yourself from the events in the Dispose method. – Didier A. Jun 21 '12 at 17:28
  • Hi didibus. Yes, you are correct. I forgot about that one. I've modified my answer to include that as a case. Thanks. – Dave Black Aug 29 '12 at 13:56
  • The MSDN documentation for the dispose pattern adds another case: "CONSIDER implementing the Basic Dispose Pattern on classes that themselves don’t hold unmanaged resources or disposable objects but are likely to have subtypes that do. A great example of this is the System.IO.Stream class. Although it is an abstract base class that doesn’t hold any resources, most of its subclasses do and because of this, it implements this pattern." – Gonen I Nov 20 '14 at 11:05
13

Using lambdas instead of IDisposable.

I have never been thrilled with the whole using/IDisposable idea. The problem is that it requires the caller to:

  • know that they must use IDisposable
  • remember to use 'using'.

My new preferred method is to use a factory method and a lambda instead

Imagine I want to do something with a SqlConnection (something that should be wrapped in a using). Classically you would do

using (Var conn = Factory.MakeConnection())
{
     conn.Query(....);
}

New way

Factory.DoWithConnection((conn)=>
{
    conn.Query(...);
}

In the first case the caller could simply not use the using syntax. IN the second case the user has no choice. There is no method that creates a SqlConnection object, the caller must invoke DoWithConnection.

DoWithConnection looks like this

void DoWithConnection(Action<SqlConnection> action)
{
   using (var conn = MakeConnection())
   {
       action(conn);
   }
}

MakeConnection is now private

pm100
  • 48,078
  • 23
  • 82
  • 145
  • 2
    Wrapping things in lambdas can be a good approach, but it has limits. It's not too bad for situations where, in fact, all consumers of a class would employ a "using" block, but it would disallow situations where a method would store an IDisposable in a class field (either directly, or in something like an iterator). – supercat Dec 13 '11 at 22:25
  • @supercat you can argue that disallowing storage of resource hogging things is a Good Thing. THe borrowing model I propose here forces you to be lean with your usage of the resource – pm100 Dec 19 '11 at 23:43
  • It can be a good thing, but it can also make some very reasonable operations very difficult. For example, suppose that a database-reader type, instead of implementing IEnumerable, exposes a method `DoForAll(Action) where T:IComparable`, calling the indicated delegate on each record. Given two such objects, both of which will return data in sorted order, how would one output all the items that exist in one collection but not the other? If the types implemented `IEnumerable`, one could perform a merging operation, but that won't work with `DoForAll`. – supercat Dec 20 '11 at 00:04
  • The only way I can figure to merge two `DoForAll` collections without having to first copy one, in its entirety, into some other structure, would be to use two threads, which would be rather more hoggish of resources than simply using a couple IEnumerable's and being careful to release them. – supercat Dec 20 '11 at 01:07
12

nobody answered the question about whether you should implement IDisposable even though you dont need it.

Short answer : No

Long answer:

This would allow a consumer of your class to use 'using'. The question I would ask is - why would they do it? Most devs will not use 'using' unless they know that they must - and how do they know. Either

  • its obviuos the them from experience (a socket class for example)
  • its documented
  • they are cautious and can see that the class implements IDisposable

So by implementing IDisposable you are telling devs (at least some) that this class wraps up something that must be released. They will use 'using' - but there are other cases where using is not possible (the scope of object is not local); and they will have to start worrying about the lifetime of the objects in those other cases - I would worry for sure. But this is not necessary

You implement Idisposable to enable them to use using, but they wont use using unless you tell them to.

So dont do it

pm100
  • 48,078
  • 23
  • 82
  • 145
  • 1
    the point is that a dev would have to write all the calls to dispose in all the code pathways that result in the unreferencing of it. SO for example if i put an instance in a Dictionary, when I delete entries from the dictionary I have to call dispose. Its a lot of hassle that is not needed in this case - the object doesnt need to be disposed – pm100 Jan 13 '10 at 23:05
  • 3
    @pm100 Re: Needlessly implementing IDisposable -- There is detailed article at http://www.codeproject.com/KB/dotnet/idisposable.aspx which discusses some rare cases when you might want to think about this (very rare, I'm sure). In short: If you can foresee the need for IDisposable in the future, or in a derived object, then you might think about implementing IDisposable as a "no-op" in your base class to avoid "slicing" issues where some derived objects require disposal and others don't. – Kevin P. Rice Jul 20 '11 at 03:37
5

Dispose pattern:

public abstract class DisposableObject : IDisposable
{
    public bool Disposed { get; private set;}      

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~DisposableObject()
    {
        Dispose(false);
    }

    private void Dispose(bool disposing)
    {
        if (!Disposed)
        {
            if (disposing)
            {
                DisposeManagedResources();
            }

            DisposeUnmanagedResources();
            Disposed = true;
        }
    }

    protected virtual void DisposeManagedResources() { }
    protected virtual void DisposeUnmanagedResources() { }
}

Example of inheritance:

public class A : DisposableObject
{
    public Component components_a { get; set; }
    private IntPtr handle_a;

    protected override void DisposeManagedResources()
    {
        try
        {
          Console.WriteLine("A_DisposeManagedResources");
          components_a.Dispose();
          components_a = null;
        }
        finally
        { 
          base.DisposeManagedResources();
        }
    }

    protected override void DisposeUnmanagedResources()
    {
        try
        {
          Console.WriteLine("A_DisposeUnmanagedResources");
          CloseHandle(handle_a);
          handle_a = IntPtr.Zero;
        }
        finally
        { 
          base.DisposeUnmanagedResources();
        }
    }
}

public class B : A
{
    public Component components_b { get; set; }
    private IntPtr handle_b;

    protected override void DisposeManagedResources()
    {
        try
        {
          Console.WriteLine("B_DisposeManagedResources");
          components_b.Dispose();
          components_b = null;
        }
        finally
        { 
          base.DisposeManagedResources();
        }
    }

    protected override void DisposeUnmanagedResources()
    {
        try
        {
          Console.WriteLine("B_DisposeUnmanagedResources");
          CloseHandle(handle_b);
          handle_b = IntPtr.Zero;
        }
        finally
        { 
          base.DisposeUnmanagedResources();
        }
    }
}
Andrei Krasutski
  • 4,913
  • 1
  • 29
  • 35
4

Some aspects of another answer are slightly incorrect for 2 reasons:

First,

using(NoGateway objNoGateway = new NoGateway())

actually is equivalent to:

try
{
    NoGateway = new NoGateway();
}
finally
{
    if(NoGateway != null)
    {
        NoGateway.Dispose();
    }
}

This may sound ridiculous since the 'new' operator should never return 'null' unless you have an OutOfMemory exception. But consider the following cases: 1. You call a FactoryClass that returns an IDisposable resource or 2. If you have a type that may or may not inherit from IDisposable depending on its implementation - remember that I've seen the IDisposable pattern implemented incorrectly many times at many clients where developers just add a Dispose() method without inheriting from IDisposable (bad, bad, bad). You could also have the case of an IDisposable resource being returned from a property or method (again bad, bad, bad - don't 'give away your IDisposable resources)

using(IDisposable objNoGateway = new NoGateway() as IDisposable)
{
    if (NoGateway != null)
    {
        ...

If the 'as' operator returns null (or property or method returning the resource), and your code in the 'using' block protects against 'null', your code will not blow up when trying to call Dispose on a null object because of the 'built-in' null check.

The second reason your reply is not accurate is because of the following stmt:

A finalizer is called upon the GC destroying your object

First, Finalization (as well as GC itself) is non-deterministic. THe CLR determines when it will call a finalizer. i.e. the developer/code has no idea. If the IDisposable pattern is implemented correctly (as I've posted above) and GC.SuppressFinalize() has been called, the the Finalizer will NOT be called. This is one of the big reasons to properly implement the pattern correctly. Since there is only 1 Finalizer thread per managed process, regardless of the number of logical processors, you can easily degrade performance by backing up or even hanging the Finalizer thread by forgetting to call GC.SuppressFinalize().

I've posted a correct implementation of the Dispose Pattern on my blog: How to Properly Implement the Dispose Pattern

StayOnTarget
  • 11,743
  • 10
  • 52
  • 81
Dave Black
  • 7,305
  • 2
  • 52
  • 41
4
  1. If you are using other managed objects that are using unmanaged resources, it is not your responsibility to ensure those are finalized. Your responsibility is to call Dispose on those objects when Dispose is called on your object, and it stops there.

  2. If your class doesn't use any scarce resources, I fail to see why you would make your class implement IDisposable. You should only do so if you're:

    • Know you will have scarce resources in your objects soon, just not now (and I mean that as in "we're still developing, it will be here before we're done", not as in "I think we'll need this")
    • Using scarce resources
  3. Yes, the code that uses your code must call the Dispose method of your object. And yes, the code that uses your object can use using as you've shown.

  4. (2 again?) It is likely that the WebClient uses either unmanaged resources, or other managed resources that implement IDisposable. The exact reason, however, is not important. What is important is that it implements IDisposable, and so it falls on you to act upon that knowledge by disposing of the object when you're done with it, even if it turns out WebClient uses no other resources at all.

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
2

Pattern from msdn

public class BaseResource: IDisposable
{
   private IntPtr handle;
   private Component Components;
   private bool disposed = false;
   public BaseResource()
   {
   }
   public void Dispose()
   {
      Dispose(true);      
      GC.SuppressFinalize(this);
   }
   protected virtual void Dispose(bool disposing)
   {
      if(!this.disposed)
      {        
         if(disposing)
         {
            Components.Dispose();
         }         
         CloseHandle(handle);
         handle = IntPtr.Zero;
       }
      disposed = true;         
   }
   ~BaseResource()      
   {      Dispose(false);
   }
   public void DoSomething()
   {
      if(this.disposed)
      {
         throw new ObjectDisposedException();
      }
   }
}
public class MyResourceWrapper: BaseResource
{
   private ManagedResource addedManaged;
   private NativeResource addedNative;
   private bool disposed = false;
   public MyResourceWrapper()
   {
   }
   protected override void Dispose(bool disposing)
   {
      if(!this.disposed)
      {
         try
         {
            if(disposing)
            {             
               addedManaged.Dispose();         
            }
            CloseHandle(addedNative);
            this.disposed = true;
         }
         finally
         {
            base.Dispose(disposing);
         }
      }
   }
}
softwarematter
  • 28,015
  • 64
  • 169
  • 263
2

1) WebClient is a managed type, so you don't need a finalizer. The finalizer is needed in the case your users don't Dispose() of your NoGateway class and the native type (which is not collected by the GC) needs to be cleaned up after. In this case, if the user doesn't call Dispose(), the contained WebClient will be disposed by the GC right after the NoGateway does.

2) Indirectly yes, but you shouldn't have to worry about it. Your code is correct as stands and you cannot prevent your users from forgetting to Dispose() very easily.

Jesse C. Slicer
  • 19,901
  • 3
  • 68
  • 87
1
using(NoGateway objNoGateway = new NoGateway())

is equivalent to

try
{
    NoGateway = new NoGateway();
}

finally
{
    NoGateway.Dispose();
}

A finalizer is called upon the GC destroying your object. This can be at a totally different time than when you leave your method. The Dispose of IDisposable is called immediately after you leave the using block. Hence the pattern is usually to use using to free ressources immediately after you don't need them anymore.

Daniel Fabian
  • 3,828
  • 2
  • 19
  • 28
  • 1
    A finalizer is not called upon the GC destroying the object. If "Finalize" is overridden, then when the GC *would otherwise have destroyed the object*, it will be placed on a queue of objects needing finalization, temporarily creating a strong reference to it and--at least temporarily--"resurrecting" it. – supercat Dec 13 '11 at 22:29
-5

From what I know, it's highly recommended NOT to use the Finalizer / Destructor:

public ~MyClass() {
  //dont use this
}

Mostly, this is due to not knowing when or IF it will be called. The dispose method is much better, especially if you us using or dispose directly.

using is good. use it :)

Nic Wise
  • 8,061
  • 2
  • 31
  • 30
  • 2
    You should follow the link in thecoop's answer. Yes, using/Dispose is better but a Disposable class should definitely implement both. – H H May 22 '09 at 20:17
  • 1
    Interesting, all the docs I've read from microsoft - eg the framework design guidelines - say dont EVER use a destructor. Always use IDisposable. – Nic Wise May 22 '09 at 23:46
  • 5
    Just distinguish between _using_ a class and _writing_ the class, the read them again. – H H May 23 '09 at 09:18
  • http://stackoverflow.com/questions/2605412/why-should-we-call-suppressfinalize-when-we-dont-have-a-destructor may help – Alex Nolasco Feb 27 '13 at 18:39