Crash When Deleting Pointer in Destructor

[Wheels2050]

Hello all,

I'm trying to write myself a class at the moment, and have come across a crash when I call a destructor. I'm sure it's simply my lack of detailed understanding of pointers in C++. I'm using the root plotting package, but I believe this question is simply a general C++ problem.

Here's a (cut down) version of the class:

class Skymap {
 public:
  Skymap();
  ~Skymap();
  void DrawAitoffSkymap();
 private:
  TCanvas mCanvas;

  TBox* mSkymapBorderBox;
};

And here is the source code:

#include "Skymap.h"

Skymap::Skymap()
{
  mCanvas.SetCanvasSize(1200,800);
  mMarkerType=1;
}

Skymap::~Skymap()
{
  delete mSkymapBorderBox;
}

void Skymap::DrawAitoffSkymap()
{
  TBox* mSkymapBorderBox=new TBox(-200,-100,200,100);
  //Use the mSkymapBorderBox pointer for a while
}

Now, the code seems to execute properly until the end, where I experience a crash. If I comment out the delete command in the Skymap destructor, the crash does not occur. Can anyone please enlighten me as to why this is happening?

[Wheels2050]

I should also add that the crash only occurs when I have multiple Skymap objects. I'm guessing that the problem is that the first Skymap object deletes its mSkymapBorderBox pointer when its destructor is called. Somehow the second Skymap object is pointing to the same memory address for its mSkymapBorderBox and when its delete is called (for dynamic memory that is no longer allocated) then the crash occurs.

Is that correct, and how do I avoid it?

[clanmjc]

If you are not calling DrawAitoffSkymap() then your pointer is never initialized and you are in undefined behavior territory. Using only your posted code this could happen all over the place, copy constructor, assignment, an object of this time that is leaving any scope will blow up. At the very least, in the constructor at least initialize the pointer to NULL and check for that in the deconstructor.

Skymap::Skymap() : mSkymapBorderBox(NULL)
{
  mCanvas.SetCanvasSize(1200,800);
  mMarkerType=1;
}

Skymap::~Skymap()
{
  if(mSkymapBorderBox)
     delete mSkymapBorderBox;
}

[Wheels2050]

Thanks very much for the reply. Setting mSkymapBorderBox=NULL in the constructor fixes the problem, and I see that it is good (essential) practise to do so.

However, I do not understand why my program crashes in the following case:

int main(){
  Skymap skymap1;
  Skymap skymap2;
  skymap1.DrawAitoffSkymap();
  skymap2.DrawAitoffSkymap();
  return(0);
}

with the Skymap class defined as per my original post. The pointer is being initialised for both objects, but a crash still occurs when the second destructor is called.

[Stewbond]

I think this is what is happening:

In DrawAitoffSkymap(), you create a local TBox pointer and then create an object for it to point to. At the end of DrawAitoffSkymap(), the local TBox pointer is destroyed, but the object it points to remains. Then when you call the destructor you are deleting a new pointer that belonged to the class-wide mSkymapBorderBox that doesn't point to anything while the old object doesn't have a pointer anymore and is just sort of in limbo.

Solution:
Turn this line:
TBox* mSkymapBorderBox=new TBox(-200,-100,200,100);
into:
mSkymapBorderBox=new TBox(-200,-100,200,100);

Now you aren't making a local variable which leaks at the end of the function. Instead we are referring the the private class member which will continue to exist after the function scope has ended.

I can explain this better without using pointers:

1 2 3 4 5 6 7 8 9 10

int a = 1; // Global Variable void print() { cout << "a in print() is: " << a << endl; } int main() { int a = 2; // local variable is separate and has priority in this scope. cout << "a in main() is: " << a << endl; print(); }

a in main() is 2 a in print() is 1

[Wheels2050]

Thanks very much - that absolutely was it!

I'm still new to writing classes, so I sometimes forget that the member variables have already been declared. Thanks very much for the explanation!