Can I delete an object within itself?

[Demonpiggies]

Originally I wanted to call the destructor from the constructor but what I really wanted to do is delete the object(vc++ 6.0 dialog object)if it does not meet a certain condition within the constructor itself. We're pretty sure an object cannot be delete from within itself but can the destructor be called from within its constructor (nulling the members). The idea is to free the memory as fast as possible. If it is possible, then would there be a stack pointer issue where the top of the stack is empty and the pointer points to this empty space? I'm not going down this root (mainly because the scope of the object is so little) but it's caused some discussion and I'm really curious about the true answer.

 

[xwb]

Just throw an exception in the constructor.

If you create the object as an auto_ptr, it will get wiped when it goes out of scope. Only problem is you can't add it to containers.

 

[Demonpiggies]

Does the exception stop the object from being created?

 

[xwb]

It does and it doesn't. It could be half created before you decide that it doesn't really want to be created, in which case you throw an exception. However, the stuff already created doesn't get freed. If you use an auto_ptr, it will get freed.

The problem is that new works in two parts: first allocate the memory then run through your constructor. If you throw, it gets out of your constructor but doesn't free the memory. With an auto_ptr, it will free the memory as soon as it goes out of scope.

 

[cpjust]

If you throw an exception in the constructor, doesn't it immediately call the destructor?

If the object in question is being created on the heap (i.e. with new) then you can do:

delete this;

But then you have no way of knowing whether it deleted itself. I've seen this used in a Delete() method that you explicitely call when you want to delete an object pointer that was allocated in a different DLL.

 

[uolj]

If the constructor fails you can throw an exception.

If you are doing this:

MyClass* p = new MyClass;

You just need to catch and handle the exception thrown from the constructor. There will be no memory leak because there is no way to save the pointer returned from new if the constructor from MyClass throws. Using an auto_ptr here will not matter.

If you are worried about freeing memory allocated inside the constructor like this:

class MyClass
{
    int* data;
public:
    MyClass()
    {
        data = new int[10];

        // Other processing fails and requires an exception
    }
};

In that case, data will be leaked. The way to fix this is to use proper RAII. For example, use a vector instead of a dynamic array. Another possible solution which is bad design but will still work is to catch the exception and delete all the pointers that need to be deleted, then re-throw. You have to make sure and initialize them to null first, so that if they have not been alocated the delete will be a no-op. How you do this exactly depends on your exact code.

If you don't want to use an exception, then you have to finish the construction and flag the object as bad, then let calling code destroy the object appropriately.

Can you clarify if you meant the first version or the second version or something else?

 

[Demonpiggies]

The idea was to immediately free the just allocated memory used for in the creation of the object. I'm using the DialogBox code to check if it even needs to be used so I was trying to halt its creation if possible instead of heaving to duplicate the same check that is already in the dlg.

.
.
.
DialogBox dlg(argument1, argument2);
.
.
.

DialogBox::DialogBox( ..., ... )
{
  if(...)
  {
    //delete object here and return
  }
  //else keep executing/initialing object
}

There is no version of this behavior or anything because I never implemented it. I was debugging and it came to me as an idea but after some discussion I decided against it mainly because it's probably considered bad practice and may cause a crash or some other bad behavior.

Tho I do like the idea of an exception but I think I'm gonna stick with a get function (to see if it's needed) and let it go out of scope because its scope is so small.

 

[uolj]

Since it is a local object on the stack, there is nothing for you to delete, and so you don't have to do anything. As you said you can just let it go out of scope.

I wouldn't worry about this. Just make sure you don't keep using the dlg variable if construction fails but you don't throw an exception.

 

[Demonpiggies]

We found out that if an exception is thrown from within the constructor but the destructor is not called until it falls out of scope so there will be a memory leak from the object itself because it will have been created but just not initialized. Plus there are a few classes in my code base that others have gone this route of an exception in the constructor and its going to be overhauled very soon. Thanks all for your input!

 

[uolj]

I don't think that's correct, unless your example above is different than what you are talking about. Can you post a better example? In the one you gave, there is no memory to delete, and there is nothing to do in the destructor, so there is no problem.

If an exception is thrown from within the constructor, the destructor is not called ever because there is nothing to destruct, the object has not been created.

See this link for more information, specifically 17.2-17.4:

http://www.parashift.com/c++-faq-lite/exceptions.html

 

[Demonpiggies]

The code I gave is very overly simplified (I am sorry I just assumed that everyone would get that sorry).
The dialogbox sits within its own class.
There is a destructor that nulls ALL members.
In the constructor there are delclared variables and such.
...
Now the problem is if I were to call an exception inside the constructor the destructor would not be called by the system until it is fully out of scope. From that exception I would have to delete/nullify any variable or objects used/created in that constructor otherwise when the exception is hit the mem. allocated for those members would still be in use until I hit the scope of the busted object. Then what of the original object? Isn't its allocated memory still in use? And if 'new' is not used?

 

[uolj]

I know the example is simplified, but it is too simple to show the issue. When you say the destructor nulls ALL members, I don't know if you are confused (there is no need for a destructor to set a member to null) or using different terminology (maybe your destructor calls delete to free memory and destroy objects).

If you look above at my second example, there is a call to new in the constructor. Is that what happens in your code (or something similar)? Here is a more detailed (and complete) example:

#include <iostream>

class DialogException { };

class DialogBox
{
    int* member1;
    int* member2;
public:
    DialogBox(int argument1, int argument2)
    {
        if (argument1 < 0)
        {
            // ERROR! Should not be less than 0.
            throw DialogException();
        }
        member1 = new int(argument1); // will be leaked

        if (argument2 < 0)
        {
            // ERROR! Should not be less than 0.
            throw DialogException();
        }
        member2 = new int(argument2);
    }
    ~DialogBox()
    {
        std::cout << "In destructor.\n"; // never called
        delete member1;
        delete member2;
    }
};

int main()
{
    try
    {
        DialogBox dlg(1, -1);
    }
    catch (const DialogException& ex)
    {
        std::cout << "error initializing dialog.\n";
    }
}

The problem there is that member1 will get leaked when argument2 is less than 0. Is this the kind of problem you are talking about?

To fix this, you have several options:

First, if possible, perform all tasks that might indicate a failed construction before you allocate anything that needs to be cleaned up. In this example, you can check argument2 before allocating space for member1. If you don't call new for member1 before you check argument2, then you won't leak that memory.

Sometimes that's not possible, so the second option is to have each member cleanup after itself. This is good design practice anyway, and for just this reason. If you use a smart pointer instead of a raw pointer, then the smart pointer will automatically cleanup when the exception is thrown:

#include <iostream>
#include <memory>

class DialogException { };

class DialogBox
{
    std::auto_ptr<int> member1;
    std::auto_ptr<int> member2;
public:
    DialogBox(int argument1, int argument2)
    {
        if (argument1 < 0)
        {
            // ERROR! Should not be less than 0.
            throw DialogException();
        }
        member1.reset(new int(argument1)); // will not be leaked

        if (argument2 < 0)
        {
            // ERROR! Should not be less than 0.
            throw DialogException();
        }
        member2.reset(new int(argument2));
    }
    // cleanup is automatic, no need for destructor.
    // ~DialogBox()
private:
    // disable copying.
    DialogBox(const DialogBox&);
    void operator=(const DialogBox&);
};

int main()
{
    try
    {
        DialogBox dlg(1, -1);
    }
    catch (const DialogException& ex)
    {
        std::cout << "error initializing dialog.\n";
    }
}

Note that auto_ptr might not be the best choice for a smart pointer, since it has strange copy semantics, but you probably won't be copying dialogs anyway so you can disable copying and be happy.

A third option, which is the most clumsy and error prone is to check for the error state and cleanup as you go. You can do this before throwing the exception:

#include <iostream>

class DialogException { };

class DialogBox
{
    int* member1;
    int* member2;
public:
    DialogBox(int argument1, int argument2)
    {
        if (argument1 < 0)
        {
            // ERROR! Should not be less than 0.
            throw DialogException();
        }
        member1 = new int(argument1);

        if (argument2 < 0)
        {
            // Cleanup first member before throwing exception
            delete member1;
            throw DialogException();
        }
        member2 = new int(argument2);
    }
    ~DialogBox()
    {
        std::cout << "In destructor.\n"; // never called
        delete member1;
        delete member2;
    }
};

int main()
{
    try
    {
        DialogBox dlg(1, -1);
    }
    catch (const DialogException& ex)
    {
        std::cout << "error initializing dialog.\n";
    }
}

Hopefully those examples make sense and generally apply to your situation. I like the second option best.

 

[Demonpiggies]

The problem there is that member1 will get leaked when argument2 is less than 0. Is this the kind of problem you are talking about?

 member1 = new int(argument1); // will be leaked

yes. To do the check I have to initialize and create quite a few pointers and objects. The destructor actual nulls another object's member that used for something trivial and deletes a few other internal members. I really just wanted to know if there was a way to complete destroy the object being created as it is being created (the object itself and its members). I ended up having to create a boolean helper function and using it as an if argument.

 

[cpjust]

You only have a memory leak when you use new to allocate memory and then forget to call delete when you're done with it. If your object doesn't new anything, then there is nothing to do in the destructor.

If you do use new to allocate memory, one other thing you can do is move all your destructor code out into its own function, then call that in the destructor, as well as before the constructor throws an exception.

class CTest
{
public:
   CTest() : m_Int(NULL), m_Str(NULL)
   {
      m_Int = new int;
      m_Str = new char[256];
      ...
      if (...)
      {
         destruct();
         throw some_exception( "Construction halted." );
      }
   }

   ~CTest()
   {
      destruct();
   }

private:
   int*  m_Int;
   char* m_Str;

   void destruct()
   {
      delete    m_Int;
      delete [COLOR=red][][/color] m_Str;
   }
};

 

[xwb]

I can't understand why you don't want to use an auto_ptr. You seem to have avoided the topic completely. It is standard STL and it does the memory management for you, especially in cases where constructors throw exceptions. It will fix most of the problems you've mentioned.

#include <memory>
...

DialogBox::DialogBox( ..., ... )
{
  if(...)
  {
    // throw exception
  }
  //else keep executing/initialing object
}

..
   try {
      auto_ptr<Dialog> dlg(new Dialog(...));
      ... do whatever with dlg

      // do not delete dlg.  auto_ptr does that for you
   }
   catch (...)
   {
      // do nothing - dlg now out of scope so memory wiped
   }

 

[uolj]

xwb, the auto_ptr in that case has no effect. The OP is creating the variable on the stack anyway. Even if he wasn't the memory would be cleaned up automatically if the constructor throws an exception.

The place to use auto_ptr is inside the class.

 

[xwb]

I really should remember to hit the Submit post button when I've finished. It was actually a reply to the 8:31 message before the OP clarified what was happening.

 

[dEVooXiAm]

To be honest, what is the problem of moving whole memory allocation to Initialize type of function? Use constructors just to set everything to default values, and perform allocation in a seperate function - you'll be off with current problems in no time.

------------------
When you do it, do it right.