Sign in to follow this  
Kuraitou

(C++) Weird Crash

Recommended Posts

Kuraitou    250
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?

Share this post


Link to post
Share on other sites
Evil Steve    2017
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.

Share this post


Link to post
Share on other sites
Mike nl    390
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!

Share this post


Link to post
Share on other sites
Kuraitou    250
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.

Share this post


Link to post
Share on other sites
Mike nl    390
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;

Share this post


Link to post
Share on other sites
Evil Steve    2017
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.

Share this post


Link to post
Share on other sites
Kuraitou    250
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.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this