Is it worth setting pointers to NULL in a destructor?

Question

Imagine I have a class that allocates memory (forget about smart pointers for now):

class Foo
{
public:
  Foo() : bar(new Bar)
  {
  }

  ~Foo()
  {
    delete bar;
  }

  void doSomething()
  {
    bar->doSomething();
  }

private:
  Bar* bar;
};

As well as deleting the objects in the destructor is it also worth setting them to NULL?

I'm assuming that setting the pointer to NULL in the destructor of the example above is a waste of time.

Answer 1

Several answer mention that it might be worthwhile to do this in a DEBUG build to aid in debugging.

Do not do this.

You'll just potentially help hide a problem in a debug build that isn't hidden in your release builds that you actually give to customers (which is the opposite effect that your debug builds should have).

If you're going to 'clear' the pointer in the dtor, a different idiom would be better - set the pointer to a known bad pointer value. That way if there is a dangling reference to the object somewhere that ultimately tries to use the pointer, you'll get a diagnosable crash instead of the buggy code avoiding use of the pointer because it notices that it's NULL.

Say that doSomething() looked like:

void doSomething()
{
    if (bar) bar->doSomething();
}

Then setting bar to NULL has just helped hide a bug if there was a dangling reference to a deleted Foo object that called Foo::doSomething().

If your pointer clean up looked like:

~Foo()
{
    delete bar;
    if (DEBUG) bar = (bar_type*)(long_ptr)(0xDEADBEEF);
}

You might have a better chance of catching the bug (though just leaving bar alone would probably have a similar effect).

Now if anything has a dangling reference to the Foo object that's been deleted, any use of bar will not avoid referencing it due to a NULL check - it'll happily try to use the pointer and you'll get a crash that you can fix instead of nothing bad happening in debug builds, but the dangling reference still being used (to ill effect) in your customer's release builds.

When you're compiling in debug mode, chances are pretty good that the debug heap manager will already be doing this for you anyway (at least MSVC's debug runtime heap manager will overwrite the freed memory with 0xDD to indicate the memory is dead/freed).

The key thing is that if you're using raw pointers as class members, don't set the pointers to NULL when the dtor runs.

This rule might also apply to other raw pointers, but that depends on exactly how the pointer is used.

Answer 2

Since the destructor is the last thing that is called on an object before it "dies," I would say there is no need to set it to NULL afterwards.

In any other case, I always set a pointer to NULL after calling delete on it.

Answer 3

Yes, it's a waste of time.

Answer 4

You shouldn't, for two reasons:

• it aids debugging, but in modern environments, deleted objects are usually overwritten already with a recognizable bit pattern in debug builds.

• In a large application, it may significantly degrade shutdown performance. In a worst-case scenario, closing your app means calling dozens of distinct destructors, and writing to hundreds of heap pages that are currently swapped to disk.

Answer 5

First of all, this is a C practice, and a controversed one. Some argue (for C) that it hides bugs that would surface sooner if it was not used, and it's impossible to distinguish the use of a freed memory portion from the use of one that was never allocated...

Now in C++ ? It's useless, but not for the same reason than in C.

In C++ it's a bug to use delete. If you were to use smart pointers, you would not be worried about this, and you would not risk leaking (ie: are you sure your copy constructor and assignement operator are exception safe ? thread safe ?)

Finally, in a destructor it really is completely useless... accessing any field of an object once a destructor has run is undefined behavior. You've released the memory, so there could perfectly be something else written there, overwriting your carefully placed NULL. And in fact, the way memory allocators work is often to reallocate first the freshly deallocated zones: this enhance caching performances... and this is even truer (haha!) if we're talking about the stack, of course.

My opinion: SAFE_DELETE is a sign of impending doom.

Answer 6

Might actually be worth it for debugging reasons.

Answer 7

Generally, no, there is no need to set pointers to NULL explicitly after deleting them in the destructor - though it can be a handy aid during debugging when inspecting to class to indicate whether a resource has been correctly freed or not.

A common idiom is to declare a SAFE_DELETE macro, which deletes the pointer and sets it NULL for you:

#define SAFE_DELETE(x) delete (x); x = NULL

SAFE_DELETE(bar)

This is especially useful in cases where the pointer may later get reused.

Answer 8

I'd avoid raw pointers at all cost, example with and without smart point where setting it to NULL explicitly is useful:

With:

 class foo
 {
   public:
      foo() : m_something( new int ) { }

      void doStuff()
      {
          // delete + new again - for whatever reason this might need doing
          m_something.reset( new int );
      }

   private:
      std::unique_ptr<int> m_something; // int as an example, no need for it to be on the heap in "real" code
 }

Without:

class foo
{
   public:
     foo() : m_something( new int ) { }
     ~foo()
     {
        delete m_something;
     }

     void doStuff()
     {
        delete m_something;

       // Without this, if the next line throws then the dtor will do a double delete
       m_something = nullptr;

       m_something = new int;
     }

  private:
     int* m_something
 }

Answer 9

I think it's always worth doing this (even though you don't technically need to). I set a pointer to NULL to indicate the memory it points to does not need to be deallocated.

Also if is useful for checking the pointer is valid before using it.

if (NULL == pSomething)
{
  // Safe to operate on pSomething
}
else
{
  // Not safe to operate on pSomething
}

Putting NULL first in the if condition guards against indavertently setting pSomething to NULL when you slip and miss out the second '='. You get a compile error rather than a bug that takes time to track down.

Answer 10

IMO its worth it in DEBUG mode. I do find it helpful frequently. In RELEASE mode it is usually skipped by the compiler due to code optimization, so you shouldn't really rely on this in your production code.

Answer 11

No, it ain't worth it.

But if you want to be consistent then you should probably do it.

What I would usually do is to create a Free function that I can use each time I need to free the allocated data (event more Free functions if needed). In those functions you are advised to set the pointers to NULL (or identifier to an invalid value).

Answer 12

It's good practice to always set pointers to NULL after deleting them. Also some code checking tools enforce this.