33

Is there any sense to set custom object to null(Nothing in VB.NET) in the Dispose() method? Could this prevent memory leaks or it's useless?!

Let's consider two examples:

public class Foo : IDisposable
{
    private Bar bar; // standard custom .NET object

    public Foo(Bar bar) {
        this.bar = bar;
    }
    public void Dispose() {
        bar = null; // any sense?
    }
}

public class Foo : RichTextBox
{
    // this could be also: GDI+, TCP socket, SQl Connection, other "heavy" object
    private Bitmap backImage; 

    public Foo(Bitmap backImage) {
        this.backImage = backImage;
    }

    protected override void Dispose(bool disposing) {
        if (disposing) {
            backImage = null;  // any sense?
        }
    }
}
serhio
  • 28,010
  • 62
  • 221
  • 374

8 Answers8

24

Personally I tend to; for two reasons:

  • it means that if somebody has forgotten to release the Foo (perhaps from an event) any downstream objects (a Bitmap in this case) can still be collected (at some point in the future - whenever the GC feels like it); it is likely that this is just a shallow wrapper around an unmanaged resource, but every little helps.
    • I really don't like accidentally keeping an entire object graph hanging around just because the user forgot to unhook one event; IDisposable is a handy "almost-kill" switch - why not detach everything available?
  • more importantly, I can cheekily now use this field to check (in methods etc) for disposal, throwing an ObjectDisposedException if it is null
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • 1
    How often do you keep references to objects after calling Dispose() on them? – Brian Rasmussen Feb 17 '10 at 11:23
  • 8
    @Brian - note the words "accidentally" and "event"; and note that **I'm** not necessarily the person writing the code that *uses* my component. I can't fix *their* code, but I can make mine well-behaved. – Marc Gravell Feb 17 '10 at 11:24
  • 2
    Sorry, if that came out wrong. I am not objecting to the practice. I just prefer the slightly simpler code without these redundancies. – Brian Rasmussen Feb 17 '10 at 11:27
  • 1
    I'm assuming this is something you do if you need to implement IDisposable anyway (some specific resource needs to be freed) and so you might as well be thorough. Presumably you aren't implementing IDisposable everywhere so you can do this. – MarkJ Feb 17 '10 at 13:52
  • 1
    +1. One should always set Events to null in the Dispose method (if it exists) just in case the subscriber forgets to unhook themselves. I've seen it happen on every project, and when events aren't unhooked, you've got a memory leak. – Lee Grissom Feb 01 '13 at 06:34
21

The purpose of Dispose() is to allow clean up of resources that are not handled by the garbage collector. Objects are taken care of by GC, so there's really no need to set the reference to null under normal circumstances.

The exception is if you expect the caller to call Dispose and hold on to the instance after that. In that case, it can be a good idea to set the internal reference to null. However, disposable instances are typically disposed and released at the same time. In these cases it will not make a big difference.

Brian Rasmussen
  • 114,645
  • 34
  • 221
  • 317
  • In my second example I have a Bitmap, that is recommended to Dispose(). Now, as the Bitmap is not created by the object Foo but just passed in parameter, I can't do it. I thought set it at least to Null... – serhio Feb 17 '10 at 11:13
  • 4
    You always have to decide where clean up of dependencies takes place. If you know that the bitmap isn't used anywhere else, Foo should call `Dispose()`. Otherwise it should just leave it and let the caller handle the clean up. There's no additional benefit in setting the local reference to null. When the instance of Foo is reclaimed so is the instance of Bitmap unless the caller still holds a reference to it. – Brian Rasmussen Feb 17 '10 at 11:18
  • 2
    @serhio - if you want to free up resources used by your Bitmap object as soon as you're done using it in Foo (no one else is using it), then Foo.Dispose should call backImage.Dispose – Gishu Feb 17 '10 at 11:19
  • Perhaps this refers also to heavy objects like TCP Sockets, SQL connections etc? As I just don't know when designing my Foo object will or will not be my "heavy object" externally used, perhaps I can't call Dispose. – serhio Feb 17 '10 at 11:48
  • @serhio - that's correct, only call `Dispose` on an object that is "owned" entirely by your class. – Daniel Earwicker Feb 18 '10 at 12:54
  • 1
    Setting a field to null unroots the field. You may not be doing anything with it in Dispose; but that reference is rooted to it's container until the GC decides it's not. Setting it to null alleviates that decision from the GC and unroots the object at the earliest possible moment.See Marc's answer. – Peter Ritchie Apr 18 '12 at 18:30
  • @PeterRitchie true, but the question was if this would prevent memory leaks or not. For managed objects setting references to null may enable the GC to collect the object sooner, but it isn't needed. Once an object is unrooted GC will claim it. – Brian Rasmussen Apr 18 '12 at 19:55
  • Arguably, there are no "memory leaks" in managed code so it doesn't matter what you do... But, if you define a "memory leak" as memory that doesn't get reclaimed as soon as possible, then not unrooting an object means you have a "memory leak". – Peter Ritchie Apr 19 '12 at 22:13
  • @PeterRitchie: Are you talking about this specific case or are you arguing that in general you should null all refs as soon as possible? – Brian Rasmussen Apr 19 '12 at 22:42
  • @BrianRasmussen the dispose pattern suggests null-ing large objects; but "large" is such an ambiguous term in .NET. In general it's beneficial with large objects, worse case you have an extra assignment; not much in the say of negative side-effects. – Peter Ritchie Apr 25 '12 at 19:39
  • @PeterRitchie: I agree that there are cases where this may be helpful. However, the OP specifically asked if not null-ing the ref could lead to memory leaks. Objects will be collected in either case. I would hate to have to remember this in all cases as it runs counter to the whole idea of automatic garbage collection. – Brian Rasmussen Apr 25 '12 at 19:51
  • Objects are collected when not implementing Dispose too;. The fact that the OP is asking about Dispose means his definition of "memory leak" means collecting managed objects as quickly as possible, and that setting a field to null unroots that object from the parent giving the GC the ability to collect it sooner, means setting fields to null applies, IMHO. YMMV – Peter Ritchie Apr 25 '12 at 22:26
  • @PeterRitchie: That may be the case, but that's certainly not how I read the question. I agree that null-ing refs can make sense for fields if the instance holding the refs is expected to be around a lot longer than the referenced objects. However, since we're talking about Dispose() I would not assume that to be the case following a call to Dispose() and thus null-ing the refs may have little or no effect. – Brian Rasmussen Apr 25 '12 at 22:50
  • @Brain Setting the value to null will always have no measurable adverse affects. As it turns out, *sometimes* setting the value to null has benefits, sometimes it doesn't. http://bit.ly/Itk9n2 As with most things: measure to find out if it makes a difference in your circumstances. – Peter Ritchie Apr 27 '12 at 16:07
  • @PeterRitchie: I agree and given that I would only recommend this practice for very specific cases. – Brian Rasmussen Apr 27 '12 at 16:40
  • You should call dispose if it's an object that is low level ( I'e.. images ) otherwise set it to null to release it. If you work on a supercompter then be sloppy, but on limited resource like mobile, this answer is just plain bad and wrong. When you deal with image the GC is never fast enough. – Nick Turner Oct 22 '14 at 21:51
  • @NickTurner please read my answer again. I'm *not* saying you shouldn't call `Dispose`. – Brian Rasmussen Oct 22 '14 at 21:57
4

It's just about useless. Setting to NULL back in the old COM/VB days, I believe, would decrement your reference count.

That's not true with .NET. When you set bar to null, you aren't destroying or releasing anything. You're just changing the reference that bar points to, from your object to "null". Your object still exists (though now, since nothing refers to it, it will eventually be garbage collected). With few exceptions, and in most cases, this is the same thing that would have happened had you just not made Foo IDisposable in the first place.

The big purpose of IDisposable is to allow you to release unmanaged resources, like TCP sockets or SQL connections, or whatever. This is usually done by calling whatever cleanup function the unmanaged resource provides, not by setting the reference to "null".

Dave Markle
  • 95,573
  • 20
  • 147
  • 170
  • 1
    OK, what if instead *Bar* I have a *TCP socket*? Should it be useless to set it to null? cause it's passed by parameter and "somebody" could use this object... – serhio Feb 17 '10 at 11:27
  • 1
    Yes, it would be useless. If you had a TCP socket, you would free it up by calling the socket's .Close() method. Same thing goes with SQL Connections. Setting to "null" actually does nothing but change your reference to the object you're using. – Dave Markle Feb 17 '10 at 11:49
  • 1
    -1, setting to nothing allows garbage collector to clean up on first pass. – AMissico Mar 12 '10 at 01:36
  • 2
    @AMissico: setting to nothing as opposed to letting it fall out of scope? That would only matter if it was in-scope but unused for a long period of time. – John Saunders Mar 12 '10 at 01:53
  • 1
    Falling out of scope happens when the object is cleaned up, not when Dispose is called. Therefore, setting to nothing helps, especially for short-lived objects as Aaronaught pointed out. – AMissico Mar 12 '10 at 02:19
  • 2
    @AMissico: falling out of scope happens when the reference falls out of scope. At the end of a method for a local variable, for instance. – John Saunders Mar 12 '10 at 02:43
  • @John Saunders, you are killing me. :O) – AMissico Mar 12 '10 at 02:45
  • @AMissico: I'm so tempted to just come back with, "you don't know what you're talking about". But instead, you might want to read a good book on the CLR to educate yourself. I recommend Jeff Richter's excellent "CLR via C#". – Dave Markle Mar 12 '10 at 12:13
1

This can make sense if you want to somehow prevent the disposed owned instance to be re-used.

When you set references to disposable fields to null, you are guaranteed to not use the instances any more.

You will not get ObjectDisposedException or any other invalid state caused by using owned disposed instance (you may get NullReferenceException if you do not check for nulls).

This may not make sense to you as long as all IDisposable objects have a IsDisposed property and/or throw ObjectDisposedException if they are used after they are disposed - some may violate this principle and setting them to null may prevent unwanted effects from occuring.

Marek
  • 10,307
  • 8
  • 70
  • 106
1

In C# setting an object to null is just release the reference to the object.

So, it is theoretically better to release the reference on managed objects in a Dispose-Method in C#, but only for the possibility to the GC to collect the referenced object before the disposed object is collected. Since both will most likely be collected in the same run, the GC will most propably recognize, that the referenced object is only referenced by a disposed type, so both can be collected.

Also the need to release the reference is very small, since all public members of your disposable class should throw an exception if the class is alreay disposed. So no access to your referenced object would success after disposing the referenced method.

Oliver Friedrich
  • 9,018
  • 9
  • 42
  • 48
  • Thx Dave, changed the info about VB.NET – Oliver Friedrich Feb 17 '10 at 11:56
  • So, what is the difference between C# and VB.NET when setting Nothing? I exposed the question in C# for readability, but my real project is in VB.NET. – serhio Feb 17 '10 at 11:58
  • 2
    For your purposes, there is no difference. But VB.NET is a horrible language. In VB.NET, if you set Dim x as integer = nothing, and then print the value of "x", you get 0. In C# it just doesn't compile because "int" is a value type and "null" is strictly a reference. So they don't behave exactly the same. But for reference types like IDisposable objects, they behave the exact same way. – Dave Markle Feb 17 '10 at 12:06
1

In VB.NET there is sense to set to Nothing declared Private WithEvents objects.

The handlers using Handles keyword will be removed in this way from these objects.

serhio
  • 28,010
  • 62
  • 221
  • 374
0

In general no need to set to null. But suppose you have a Reset functionality in your class.

Then you might do, because you do not want to call dispose twice, since some of the Dispose may not be implemented correctly and throw System.ObjectDisposed exception.

private void Reset()
{
    if(_dataset != null)
    {
       _dataset.Dispose();
       _dataset = null;
    }
    //..More such member variables like oracle connection etc. _oraConnection
 }
Munish Goyal
  • 1,379
  • 4
  • 27
  • 49
0

The purpose of the dispose() is to cleanup the resources that are unmanaged. TCP connections, database connections and other database objects and lots of such unmanaged resources are supposed to be released by the developer in the dispose method. So it really makes sense.

Kangkan
  • 15,267
  • 10
  • 70
  • 113
  • for both examples with a GDI+ Bitmap and a simple custom .NET object Bar? I don't dispose them, cause passed in parameter and not created by the object. – serhio Feb 17 '10 at 11:15