views:

98

answers:

5

Here is a sample code that I have:

void test()
{
   Object1 *obj = new Object1();
   .
   .
   .
   delete obj;
}

I run it in Visual Studio, and it crashes at the line with 'delete obj;'. Isn't this the normal way to free the memory associated with an object? I realized that it automatically invokes the destructor... is this normal?

+1  A: 

if it crashes on the delete line then you have almost certainly somehow corrupted the heap. We would need to see more code to diagnose the problem since the example you presented has no errors.

Perhaps you have a buffer overflow on the heap which corrupted the heap structures or even something as simple as a "double free" (or in the c++ case "double delete").

Also, as The Fuzz noted, you may have an error in your destructor as well.

And yes, it is completely normal and expected for delete to invoke the destructor, that is in fact one of its two purposes (call destructor then free memory).

Evan Teran
+3  A: 

Isn't this the normal way to free the memory associated with an object?

Yes, it is.

I realized that it automatically invokes the destructor... is this normal?

Yes

Make sure that you did not double delete your object.

Prasoon Saurav
So, why is this normal?Since it is a pointer variable, how is this normal to get deleted automatically?What happens is first I delete the object, and then reaching at line '}' it again calls the destructor, which causes the problem.My question is why is it call the destructor automatically?If it is normal, then why people delete an object in this case?
Nima
Read these : http://www.parashift.com/c++-faq-lite/dtors.html#faq-11.9 and http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.9`Then the destructor Fred::~Fred() will *automagically* get called when you delete ......`Show us the complete code.
Prasoon Saurav
+3  A: 

Your code is indeed using the normal way to create and delete a dynamic object. Yes, it's perfectly normal (and indeed guaranteed by the language standard!) that delete will call the object's destructor, just like new has to invoke the constructor.

If you weren't instantiating Object1 directly but some subclass thereof, I'd remind you that any class intended to be inherited from must have a virtual destructor (so that the correct subclass's destructor can be invoked in cases analogous to this one) -- but if your sample code is indeed representative of your actual code, this cannot be your current problem -- must be something else, maybe in the destructor code you're not showing us, or some heap-corruption in the code you're not showing within that function or the ones it calls...?

BTW, if you're always going to delete the object just before you exit the function which instantiates it, there's no point in making that object dynamic -- just declare it as a local (storage class auto, as is the default) variable of said function!

Alex Martelli
+7  A: 

Isn't this the normal way to free the memory associated with an object?

This is a common way of managing dynamically allocated memory, but it's not a good way to do so. This sort of code is brittle because it is not exception-safe: if an exception is thrown between when you create the object and when you delete it, you will leak that object.

It is far better to use a smart pointer container, which you can use to get scope-bound resource management (it's more commonly called resource acquisition is initialization, or RAII).

As an example of automatic resource management:

void test()
{
    std::auto_ptr<Object1> obj1(new Object1);

} // The object is automatically deleted when the scope ends.

Depending on your use case, auto_ptr might not provide the semantics you need. In that case, you can consider using shared_ptr.

As for why your program crashes when you delete the object, you have not given sufficient code for anyone to be able to answer that question with any certainty.

James McNellis
This surely deserves a +1 for the content and the link. :)
Prasoon Saurav
A: 

Here is some code snippet:

    if(node->isleaf())
    {
        vector<string> vec = node->L;
        vec.push_back(node->code);
        sort(vec.begin(), vec.end());

        Mesh* msh = loadLeaves(vec, node->code);

        Simplification smp(msh);

        smp.simplifyErrorBased(errorThreshold);

        int meshFaceCount = msh->faces.size();

        saveLeaves(vec, msh);

        delete msh;
    }

Loadleaves() is a function that reads a mesh from disk and creates a Mesh object and returns it.(think of vec and node->code are just information about the file to be opened)

do you think I should remove the 'delete msh;' line?

Nima
This should have been edited into your question. But in addition to that we need to see the implementation of loadLeaves to discount it as a possible problemm. Beyond that, `Simplification` and `saveLeaves` may also be relevent.
Evan Teran