(C++) Weird Crash

Started by
6 comments, last by Kuraitou 15 years, 8 months ago
I am getting a strange crash while testing a geometry class I wrote. When I add a child to my class the Windows error message sound is heard, but no messagebox pops up. I figure that I'm doing something funky with my pointers but not really too sure, and skipping through the debugger is no help either. After g.SetParent is called, g has the correct values, so I have no clue what's going on. Basically, I am using my class like this:

Geometry gr; // geometry root
Geometry ch1; // child 1
Geometry ch2; // child 2
Geometry ch1_1; // child 1's child 1
Geometry gr_1; // geometry root's child 1

gr.AddChild(ch1); // Crashes here but debugger doesn't stop on anything, even inside the function
gr.AddChild(ch2);
ch1.AddChild(ch1_1);

ch1_1.GetParent()->GetParent()->AddChild(gr_1);



And the relevant parts of the class:

void Geometry::SetParent(Geometry *g)
{
	m_pGeoParent = g;
}

Geometry *Geometry::GetParent()
{
	return m_pGeoParent;
}

void Geometry::AddChild(Geometry g)
{
	m_vGeoChildren.insert(m_vGeoChildren.end(), g);
	g.SetParent(this);

	m_bHasChildren = true; 
} // stepping through the debugger skips right over the closing brace and the Windows error sound is heard


The members that keep track of parent/children are defined like this:
Geometry *m_pGeoParent;
std::vector<Geometry> m_vGeoChildren;
Anyone have any idea what's going wrong?
Advertisement

void Geometry::AddChild(Geometry *g)


I think you have to pass g as a pointer or as reference since you're changing it in the AddChild method. Now you change the local copy of g.

Uncle
Have you tried doing a rebuild all? Might just be the compiler being funky. If that doesn't fix it, you could try going to the assembly level and stepping through there (Right click on the code -> Dissasembly) and see if that makes anything more obvious?

EDIT: Ah, UncleRemus is right - there's probably some error in the Geometry classes destructor.
Probably because m_pGeoParent contains rubbish.

In AddChild, you pass the argument by value. SetParent adjusts the parent of the argument, this is NOT the same as ch1, ch2 or ch1_1, or even the Geometry copy that has been added to the vector.

If anything, what you want is m_vGeoChildren.back().SetParent(this), but that still leaves the parents of ch1, ch2 and ch1_1 untouched.

You have to seriously rethink if storing Geometry instances and pass by value is what you want. When building a tree, it tends to work with pointers. In terms of performance, it's preferable as well. Currently, if a vector at the root node needs to reallocate, the entire tree below it will be copied!
Million-to-one chances occur nine times out of ten!
Quote:Original post by UncleRemus

*** Source Snippet Removed ***

I think you have to pass g as a pointer or as reference since you're changing it in the AddChild method. Now you change the local copy of g.

Uncle


Works, but doesn't work.

Quote:Original post by Evil Steve
Have you tried doing a rebuild all? Might just be the compiler being funky. If that doesn't fix it, you could try going to the assembly level and stepping through there (Right click on the code -> Dissasembly) and see if that makes anything more obvious?

EDIT: Ah, UncleRemus is right - there's probably some error in the Geometry classes destructor.


Now that I've checked it, for some reason the objects are calling their destructor for no apparent reason. I'll look into it.

Quote:Original post by Mike nl
You have to seriously rethink if storing Geometry instances and pass by value is what you want. When building a tree, it tends to work with pointers. In terms of performance, it's preferable as well. Currently, if a vector at the root node needs to reallocate, the entire tree below it will be copied!


But isn't that just rewriting a vector class?
EDIT: Nevermind, I see what you mean...I think.



Thanks for the help, declaring my Geometry as pointers works.
Quote:Original post by Kuraitou
Quote:Original post by Evil Steve
Have you tried doing a rebuild all? Might just be the compiler being funky. If that doesn't fix it, you could try going to the assembly level and stepping through there (Right click on the code -> Dissasembly) and see if that makes anything more obvious?

EDIT: Ah, UncleRemus is right - there's probably some error in the Geometry classes destructor.


Now that I've checked it, for some reason the objects are calling their destructor for no apparent reason. I'll look into it.

When the function is done, the destructor for the local copy (the argument) gets called. When the vector re-allocates (on certain inserts), everything is moved by making a copy and destructing the old ones. So yeah, destructors are called. And your Geometry class probably doesn't have good copy semantics.

Quote:Original post by Kuraitou
Quote:Original post by Mike nl
You have to seriously rethink if storing Geometry instances and pass by value is what you want. When building a tree, it tends to work with pointers. In terms of performance, it's preferable as well. Currently, if a vector at the root node needs to reallocate, the entire tree below it will be copied!


But isn't that just rewriting a vector class?

Not at all. What I mean is having a

std::vector<Geometry*> m_vGeoChildren;
Million-to-one chances occur nine times out of ten!
Quote:Original post by Kuraitou
Now that I've checked it, for some reason the objects are calling their destructor for no apparent reason. I'll look into it.
Sounds to me like you're storing objects by value in your vector class and not implementing the copy constructor, destructor and operator= correctly.
Quote:Original post by Evil Steve
Quote:Original post by Kuraitou
Now that I've checked it, for some reason the objects are calling their destructor for no apparent reason. I'll look into it.
Sounds to me like you're storing objects by value in your vector class and not implementing the copy constructor, destructor and operator= correctly.


I found the root of most of my problems: Destroy was deleting the actual parent and not the reference to it (thus calling my destructor and it became an infinite loop of deletion).

Thanks for the help guys.

This topic is closed to new replies.

Advertisement