why would the way I used to call destructor cause memeory issue?

Question

In the following code, if I include dealloc_dMAT into the destructor, it does not work. But if I call dealloc_dMAT specifically and then call the destructor, it works. Why would this be the case? The compiler I use is g++ 4.8.2 under linux.

The following code does not work,

#include <iostream>

using namespace std;

class QO{
  public:
    QO();
    ~QO();

    int DIM;         
    double **dMAT;   

    void alloc_dMAT();
    void dealloc_dMAT();

};


QO::QO(){

  DIM=4;
  cout << "DIM=" << DIM << endl;

  alloc_dMAT();

}
QO::~QO(){
  dealloc_dMAT();
}

void QO::alloc_dMAT(){
  dMAT=new double * [DIM];
  for(int i=0; i<DIM; i++) dMAT[i]=new double [DIM];
}
void QO::dealloc_dMAT(){
  for(int i=0; i<DIM; i++) delete [] dMAT[i];
  delete [] dMAT;
}




int main(){

  QO QB;
  //QB.dealloc_dMAT();
  QB.~QO();

  return 1;

}

But a slight modification on it works

#include <iostream>

using namespace std;

class QO{
  public:
    QO();
    ~QO();

    int DIM;
    double **dMAT;

    void alloc_dMAT();
    void dealloc_dMAT();

};


QO::QO(){

  DIM=4;
  cout << "DIM=" << DIM << endl;

  alloc_dMAT();

}
QO::~QO(){
  //dealloc_dMAT();
}

void QO::alloc_dMAT(){
  dMAT=new double * [DIM];
  for(int i=0; i<DIM; i++) dMAT[i]=new double [DIM];
}
void QO::dealloc_dMAT(){
  for(int i=0; i<DIM; i++) delete [] dMAT[i];
  delete [] dMAT;
}




int main(){

  QO QB;
  QB.dealloc_dMAT();
  QB.~QO();

  return 1;

}

EDIT: Thanks to Dave M.'s comment, it seems the code above does not fully display my issue, so here is a more accurate example of the trouble I ran into. If you compile and run it, you will notice the code abort at i=0, which should not happen if ~QO is called twice only when quitting the main.

#include <iostream>
#include <stdlib.h>

using namespace std;

class QO{
  public:
    QO();
    ~QO();

    int DIM;
    double **dMAT;

    void alloc_dMAT();
    void dealloc_dMAT();

};


QO::QO(){

  DIM=4;
  cout << "DIM=" << DIM << endl;

  alloc_dMAT();

}
QO::~QO(){
  dealloc_dMAT();
}

void QO::alloc_dMAT(){
  dMAT=new double * [DIM];
  for(int i=0; i<DIM; i++) dMAT[i]=new double [DIM];
}
void QO::dealloc_dMAT(){
  for(int i=0; i<DIM; i++) delete [] dMAT[i];
  delete [] dMAT;
}




int main(){

  for(int i=0; i<10; i++){
    QO QB;
    QB.~QO();
    cout << "i= " << i << endl;
  }

  system("pause");

  return 1;

}

You are destructing twice. ~QB is being called explicitly and implicity. You should set dMAT to NULL in the constructor. Good defensive code should check dMAT is NULL prior to destroying. Once destroyed set it back to NULL. Then regardless of how many times you call it. It will execute the deletes once. — Stephen Quan, May 08 '17 at 19:02
@StephenQuan The check for null is not needed. Just setting the pointer to null is all you need. That said, using proper RAII data structure makes all of this moot. — NathanOliver, May 08 '17 at 19:04
I was thinking if I supplied ~QO, then the implicit destructor is not invoked, maybe I was wrong on that? — bsmile, May 08 '17 at 19:09
Well, the ~Q0 you supplied replaces the implicit destructor (which wouldn't deallocate the memory allocated for `dMat`). But the "double destruction" that's causing you problems is because you call the destructor explicitly just before the return from `main`, and it is called again (automatically) because the local-scope variable `QB` is going out of scope when `main` returns. — Dave M., May 08 '17 at 19:13
@Dave M Thanks for reminding me this, I didn't realize ~QO is called again when quitting main. I had recast my actual issue to the simple case I posted, but the actual code actually invoked QO QB() and then QB.~QO() in an iteration, which gives similar error displayed in the first code and I don't understand why. — bsmile, May 08 '17 at 19:32
Your iteration has the same problem: every time the loop finishes (right after your `cout` statement) `QB` is destroyed. Since you called the destructor explicitly before `cout`, this means it is destroyed twice. (It's very rare that you need to explicitly call a destructor. `new` and `delete` are common, of course, but `~my_class()` is quite unusual.) — Dave M., May 08 '17 at 20:00
Destructor is called when a variable goes out of scope, which happens (more or less) at the closing brace of the block that you declared it in. In your modified second example, if you moved the declaration `Q0 QB;` back to the beginning of `main`, then it would only go out of scope once, but you would have (tried to!) destroy it 10 times (in the loop). Again, you rarely need to call a destructor explicitly, and in this case, you don't need to. — Dave M., May 08 '17 at 20:13
Also, search for Rule Of Three ([here](http://stackoverflow.com/a/4172961/3276027) a good start), For c++11 and following, Rule of Five, or even better, [Rule Of Zero](https://rmf.io/cxx11/rule-of-zero) — Gian Paolo, May 08 '17 at 20:26
Thank you guys all for your help. It seems implicit invoking destructor is the answer. Can you post as a formal solution so I can accept it? — bsmile, May 08 '17 at 22:11
@bsmile you never need to call destructor explicitly, ever. There are very rare cases when you need that, but you have to be experienced enough to understand what you are doing. Otherwise automatic objects would be destroyed atomatically and dynamically allocated through `delete`. — Slava, May 10 '17 at 13:58
@StephenQuan you do not need to check for `nullptr` for delete. And your suggestion to "fix" code that double call for destructor would "work" may bring more harm than good. Destructor must not be called twice. — Slava, May 10 '17 at 14:01
Thanks guys, explicit calling destructor derives from Fortran coding habit (there no destructor at all), now I learned a lesson and will remember it foroever. — bsmile, May 11 '17 at 14:39

Shadi · Accepted Answer · 2017-05-10T13:51:08.347

1

In the main, I would suggest that you comment the explicit destructor call like this:

  for(int i=0; i<10; i++)
    {
        QO QB;
        //QB.~QO();
        cout << "i= " << i << endl;
    }

Then you add a cout to the destructor like this:

QO::~QO()
{
  dealloc_dMAT();
  cout << "destructor called.";
}

This will give you a clear idea that you do not need to call the destructor explicitly as it is called implicitly each time the for scope exited.

also dMAT pointer is dangling pointer after the delete, so it is generally a good practice to assign it tonullptr although it might have no effect in some cases like when exiting the pointer scope right away after the deletion because the pointer itself no longer exists.

edited May 10 '17 at 13:51

answered May 09 '17 at 03:04

Shadi

1,701
2
14
27

Thanks, I have deleted the pointer, any special reason to assign it to nullptr? I kind of think after I delete it and then try to assign value, it will lead to run time error, which can be located accurately by the compiler. – bsmile May 10 '17 at 13:26
it is generally a good practice to assign a deleted pointer to `nullptr` although it might have no effect in some cases. – Shadi May 10 '17 at 13:50

why would the way I used to call destructor cause memeory issue?

1 Answers1