• Advertisement

Archived

This topic is now archived and is closed to further replies.

Problem with object with allocated array (c++)

This topic is 5153 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I’ve been triing to figure this one out for quite a while: I have an object that has a private variable that is an object that has a pointer that during construction is allocated as the following: explode = new part2 [60]; I have a destructor that checks to make sure the pointer isn’t == to null as shown, then deletes the array. if (explode != NULL) delete [] explode; The problem is with this in my destructor my program gives me an assertion error “_CrtIsValidHeapPointer(pUserData)” as soon as I start my GL looping. With it removed everything works fine (except for a memory leak I assume). I try debugging with a breakpoint at the start of the destructor, but the breakpoint is never hit. So from what I understand is the destructor is never called but some code in it is causing me to get the assertion error. Wow this one has me cofused.

Share this post


Link to post
Share on other sites
Advertisement
If code in the destructor is causing an error, then it''s getting run.

My first suspicion is that you''re deleteing it more than once. Try this:

if( explode != NULL )
{
delete [] explode;
explode = NULL;
}

And, in your constructor, set the pointer to NULL to start with. That way the pointer is always either pointing to valud data or NULL, no questions asked.

I like pie.

Share this post


Link to post
Share on other sites
Well, it is being deleted somewhere else. You would have to show more code for anyone to figure it out.

Share this post


Link to post
Share on other sites
My guess is that you haven''t written your own copy constructor or assign operator, and you make copies of the said object. So delete[] explode; will be called twice on same memory.

so if your class is like this:

class X {
public:
X() {
explode = new part2[60];
}

~X() {
if (explode != NULL)
delete [] explode;
}
private:
...
};


try adding private copy constructor and assignment operators and see if it still compiles:


class X {
public:
X() {
explode = new part2[60];
}

~X() {
if (explode != NULL)
delete [] explode;
}
private:
X(const X& x);
X& operator=(const X& x);
...
};

If it doesn''t, it tells you''re using either of those two somewhere, and that part causes the pointer "explode" to be duplicated in two objects yet the memory isn''t duplicated.

Share this post


Link to post
Share on other sites
quote:
Original post by RenderTarget
if( explode != NULL )
{
delete [] explode;
explode = NULL;
}
This won''t help anything, because after the destructor is done (in following line probably), the variable "explode" ceases to exist, and assignment to it''s value was useless.

Share this post


Link to post
Share on other sites
quote:
Original post by civguy
quote:
Original post by RenderTarget
if( explode != NULL )
{
delete [] explode;
explode = NULL;
}
This won''t help anything, because after the destructor is done (in following line probably), the variable "explode" ceases to exist, and assignment to it''s value was useless.




That''s not true. Deleting a pointer frees the memory pointed to by the number in the pointer. It doesn''t change the value of the pointer itself. Setting explode to NULL is a good thing.

Share this post


Link to post
Share on other sites
Wow I love this place, fast good responces. I''ll try some of the above and report back.

Thanks.

Share this post


Link to post
Share on other sites
quote:
Original post by Rick Scott
That's not true. Deleting a pointer frees the memory pointed to by the number in the pointer. It doesn't change the value of the pointer itself. Setting explode to NULL is a good thing.
What I meant that setting explode to NULL affects nothing. I know it sets explode's value to 0, but that's meaningless. It's like this piece of code:

{
int* x = new int[10];
...
delete[] x;
x = 0;
}

What does x=0 help here? The variable x ceases to exist right after the assignment, so the assignment x=0 was meaningless. In original poster's example, we were in the destructor of the class, so the variable 'explode' will vanish soon, from his description I'd the destructor contains no more code and 'explode' will vanish right away.

[edited by - civguy on January 11, 2004 5:20:29 PM]

Share this post


Link to post
Share on other sites
if( explode != NULL )
{
delete [] explode;
explode = NULL;
}

Did nothing for me, but creating a copy constructor fixed the assertion error. I have a new problem where the array I make is now empty, but that is another bug ill find.

Thanks for the help.

Share this post


Link to post
Share on other sites
quote:
Original post by civguy

{
int* x = new int[10];
...
delete[] x;
x = 0;
}

What does x=0 help here? The variable x ceases to exist right after the assignment, so the assignment x=0 was meaningless.


No it doesn't. The *memory pointed to by x* is freed (the data doesn't even cease to exist! It's still there - though probably unreachable - until something overwrites it). The pointer is - apparently - left dangling.

Try:

{
int* x = new int[10];
...
delete[] x;
delete[] x;
}


If I remember c++ worth a damn, the second delete[] will crash, because x is still pointing at the location where the int[] was, but the memory has already been freed.

Try:

{
int * x = new int[10];
...
delete[] x;
x = NULL;
delete[] x;
}


Now it works, because deleting NULL is ok (as a special case).

What's happening to the OP, according to the theory people are proposing, is that multiple instances of the object have their own variable explode, but each has the same value in that variable - the address of a *shared* part2[] array - because copying/assignment is shallow by default. So, when the destructor is called for each object:
- When the first is destroyed, the memory of the part2[] array gets deleted. The value of 'explode' in the other objects is not touched, and thus still points at that memory.
- When the second is destroyed, the code tries to free the memory again, and crashes and burns.

You're right insofar as assigning NULL to 'explode' can't help matters. But I felt the need to be pedantic. If it were possible in the destructor to find every other instance of the object and set their 'explode' pointers to NULL, then things would work out ok.

So the question becomes how the part2[] array *should* be handled. There are a couple of possibilities:
- you might really want a separate copy of it for every instance of the object, so they can't write over each other's data. In that case, you need to implement (and make public) a copy constructor and an assignment operator (i.e. overload 'operator='), which clones the part2[] array and sets explode to point at the clone.
- you might want to share the data, either to save memory on constant data, or because collaboratively writing the data is the whole point of the objects. In that case you can:
- make part2[] a static member of the class.
- wrap part2[] in a class and reference-count it; in the X constructor/destructor you send addUser/removeUser() messages to a Part2ArrayWrapper instance, and when the count reaches 0, the Part2ArrayWrapper delete[]s the part2[]. deleting the Part2ArrayWrapper itself is an exercise for the reader.
- Use a garbage-collected language like Java and save yourself at least part of the trouble.


[edited by - Zahlman on January 11, 2004 9:08:28 PM]

Share this post


Link to post
Share on other sites
quote:
Original post by Zahlman
Try:

{
int* x = new int[10];
...
delete[] x;
delete[] x;
}


You''re still seriously missing the point, even though I so carefully tried to explain it. There''s no second delete in the code I posted, nor in the OP''s code. The scope where ''x'' exists ends right after the delete. Setting x to 0 doesn''t help anything, but simply wastes resources (insignificantly little though).

Here''s a similar example:

int x = 42;
x = 3;
...

That x=42 is most useful, because if the code was like this:

int x;
x = 3;
...

And someone added a strange line there:

int x;
cout << "Here is number 42: " << x << endl;
x = 3;
...

The program''s output would be wrong! Now clearly, setting x first to 42 and then to 3 was the right solution even in the original case where there was no cout.. Right? Riiiiight.
quote:
If it were possible in the destructor to find every other instance of the object and set their ''explode'' pointers to NULL, then things would work out ok.
BS. Then the other live objects would be accessing memory at location 0.

Share this post


Link to post
Share on other sites
Another reason for the described error might be that you are writing outside the [60] units. Remember that you cannot access index 60, only 0 including 59. I am not entirely sure why this occurs, but I assume that after each array a descriptor, or terminator is stored. How else would delete[] know how many entries are to be deleted?

Edit: And the most enjoyable part is that it's not only the array you are deleting that might have been used incorrectly. Any "array index out of bounds" bug might have caused it.

---------------------------
I may be getting older, but I refuse to grow up

[edited by - Dreamforger on January 12, 2004 2:51:05 AM]

Share this post


Link to post
Share on other sites
quote:
Original post by civguy
(a bunch of correct stuff)



Yes, the second half of my post was much less brain-fart-y than the first. With the two delete[]s I was trying to construct an example of what happens when you destroy two of the objects - each tries to delete[] on its own, thus multiple calls.

And yeah, you can''t logically get rid of a shared resource until all its users are gone.

It''s been so long since I actually had to implement this kind of stuff

Share this post


Link to post
Share on other sites
quote:
That''s not true. Deleting a pointer frees the memory pointed to by the number in the pointer. It doesn''t change the value of the pointer itself. Setting explode to NULL is a good thing.


It''s never a good thing.

If the pointer you have is merely a copy of the pointer then setting it to null is pointless, as the original still retains its (bad) value.

If the pointer is the only version of the pointer it''s still not a good thing. Double deleting is a mistake. You should not make your logic errors innocuous. You should make them blow up as loudly and quickly as possible. You want the double delete to fail. Setting the pointer to null prevents this; instead, it hides your double delete bug.

This is entirely undesirable. So the moral is, don''t set your pointers to null.

Share this post


Link to post
Share on other sites
quote:
Original post by DrPizza
If the pointer you have is merely a copy of the pointer then setting it to null is pointless, as the original still retains its (bad) value.

If the pointer is the only version of the pointer it''s still not a good thing. Double deleting is a mistake. You should not make your logic errors innocuous. You should make them blow up as loudly and quickly as possible. You want the double delete to fail. Setting the pointer to null prevents this; instead, it hides your double delete bug.

This is entirely undesirable. So the moral is, don''t set your pointers to null.



It is *far* more common to try to READ a deleted pointer then to doulbe-delete it. If you read a previously-deleted pointer, you will might get your values back, or some slightly different values, or who knows what. But the bug will be harder to track down then a simple "memory at 0x00000 could not be read."

So yes, it is a good thing if you actually READ your variables.

Share this post


Link to post
Share on other sites

  • Advertisement