Relaunch an exception in a constructor later instruction delete this

Question

I have following code,

my_class.cpp

#include "my_class.h"

       my_class::my_class(int m)
       {
          try
          {
              my_method();
              throw My_Exception();
          }
          catch(exception &e)
          {
             delete this;
             throw;
          }
       }

       my_class::~my_class()
       {
           for(auto &el : my_member)
              if(el!=NULL)
                 delete el;
       }

       void my_class::my_method()
       {
          for(int i ; i<5 ; i++)
             my_member.push_back(new dfa(i)); //dfa is another class
       }

my_class.h

#include "dfa.h"
#include "exception.h"

using namespace std;

class my_class
{
    public :
    my_class();
    ~my_class();
    void my_method();

    private :
    vector<dfa *> my_member;
};

exception.h

#include <exception>
class My_Exception : public exception
        {
          public:
            const char * what () const throw ()
            {
                return "Generic Exception";
            }
        };

main.cpp

#include <iostream>
#include "my_class.h"
int main()
{
   try
   {
   my_class *ptr = new my_class(3);
   }
   catch(exception &e)
   {
      cout<<e.what()<<endl;
   }
}

the class dfa is missing, but is a normal class. The problem is in the catch, when i do delete this the denstructor is invoked and dynamics objects of class dfa deallocated but the exception isn't relaunched. The execution flow doesn't return to main (in the catch block of the main) because there is a segmentation fault. What could be the problem?

(I can't use shared or auto ptr to handle memory because I'm using a big library) (I'm using C++11)

Why on earth you want to `delete this;` in that case? Can you elaborate please? — user0042, Aug 18 '17 at 20:28
_"I can't use shared or auto ptr to handle memory because I'm using a big library"_ Huh?? I don't get that reasoning. Use smart pointers. — user0042, Aug 18 '17 at 20:28
@user0042 To free memory. I already answer because I can't use smart pointers. — Nick, Aug 18 '17 at 20:31
And as I mentioned, I don't get your reasoning why you can't use smart pointers. — user0042, Aug 18 '17 at 20:32
@user0042 Use smart pointer means change declaration in a lot of places in the code, identifiers and method signature. However this isn't the point, but why the code above doesn't work. — Nick, Aug 18 '17 at 20:35
If a constructor throws (or bubbles up) an exception, the memory is automatically freed. The destructor won't be called, and you are responsible for internal cleanup, but the object itself will be fine. — , Aug 18 '17 at 20:35
This is completely broken. If the object gets constructed in automatic scope, `delete this` will be undefined behavior, and a guaranteed crash. And in even in dynamic scope, the C++ standard itself will take care of deallocating the memory, before the exception gets propagated, so `delete this` will be double-freeing the memory. No matter how this is used, it is wrong. — Sam Varshavchik, Aug 18 '17 at 20:35
@Umbert _"Use smart pointer means change declaration in a lot of places in the code ..."_ Can't you hand out a `std::unique_ptr::get()` for the stuff depending on getting a `dfa*`? Otherwise smart pointers behave like pointers in most places syntactically, so usually exchanging them goes smoothly. — user0042, Aug 18 '17 at 20:39
`delete this` is legal, as object suicide. However, you must be cautious to be certain that it was allocated via `new` and not a stack allocation or a `new[]` allocation. My suggestion would be a `static` member to hold the object's state, and then a proxy object in order to create and delete it properly, the proxy can `throw` without committing suicide. Also, it should be noted `delete this;` must be the last operation on that object if you use it. — M4rc, Aug 18 '17 at 20:41
@Frank Excuse me, It seems contradictory that memory is automatically freed and that I am responable to internal cleanup. Can you elaborate? Furthermore if the denstructor isn't called how happen automatic free? — Nick, Aug 18 '17 at 20:44
@Umbert think of `new` as a function that does a) `malloc()` for the object, b) calls the constructor, c) if the constructor throws an exception, it `free()`s the memory and rethrows the exception, otherwise, d) it returns a pointer. — , Aug 18 '17 at 20:46
@SamVarshavchik Thus it's enough not use delete this instruction, and I have warranty that dynamics memory is automatically deallocated less have memory leak — Nick, Aug 18 '17 at 20:47
@M4rc `delete this` is NEVER legal inside a constructor since the object is not complete. — , Aug 18 '17 at 20:48
@Frank I understood, but I'm afraid that "nested" dynamics memory isn't freed (namely the free on dfa pointer). — Nick, Aug 18 '17 at 20:52
@Frank Not sure about C++11 explicitly, however according to §3.8/5: `Before the lifetime of an object has started [...] If the object will be or was of a class type with a non-trivial destructor, and the pointer is used as the operand of a delete-expression, the program has undefined behavior.` — M4rc, Aug 18 '17 at 20:54
@Frank A quick search yields this https://stackoverflow.com/a/5303689/1715586 However, upon usage of new it has a valid memory address, and thus a valid trivial deconstructor. Also, mentioned here https://isocpp.org/wiki/faq/freestore-mgmt#delete-this not noting *anything* about a constructor omission only. — M4rc, Aug 18 '17 at 21:08
Modern C++ code uses `new` and `delete` very infrequently. With `std::shared_ptr` and `std::unique_ptr`, you let the C++ library itself handle memory allocation, and correctly release all allocated memory when exceptions are thrown, or when the object goes out of scope. — Sam Varshavchik, Aug 18 '17 at 21:20

score 2 · Accepted Answer · 2017-08-18T20:53:15.340

delete does a lot more work than just calling the destructor.

You cannot call delete on an incomplete object, which is the case since the constructor never completed successfully.

The implication is that when handling exceptions in a constructor, you need to do the cleanup yourself, but you cannot simply call delete, as this will conflict with the way new deals with exceptions.

Here's a way to fix this:

my_class::my_class(int m)
{
   try
   {
       my_method();
       throw My_Exception();
   }
   catch(exception &e)
   {
      cleanup();
      throw;
   }
}

void my_class::cleanup() {
  for(auto &el : my_member)
    if(el!=NULL)
      delete el;
}

my_class::~my_class()
{
  cleanup();
}

Thus, I have to free from myself nested memory. Now it's clear. I'm going to try in release code. — Nick, Aug 18 '17 at 20:58

Relaunch an exception in a constructor later instruction delete this

1 Answers1