Why is my object destroying?

[yhu420]

Hello everyone,
Today just had the weirdest bug I ever had. I managed to get the debugger to work, and then it tells me that the the program crashes because of the destructor of one of my classes. The thing is I have no idea why such an object would be deleted O_o. Here's some code:

Shop::Shop() : m_NavHoveredShape(0U)
{
	//Things
	m_nav= new MenuNav;
	MenuButton ttt1(m_navExtraTex, m_navExtraTex, m_navExtraTex, sf::Vector2f(0.f,0.f));
	m_nav->addButton(ttt1);/*bug on this line boys*/
	std::cerr << "OIOIIOIOIOIOIO" << std::endl; //never shows up in the console
	m_nav->addButton(MenuButton(m_navGunzTex, m_navGunzTex, m_navGunzTex, sf::Vector2f(0.f,0.f)));
	m_nav->addButton(MenuButton(m_navHullTex, m_navHullTex, m_navHullTex, sf::Vector2f(0.f,0.f)));
	m_nav->addButton(MenuButton(m_navBackTex, m_navBackTex, m_navBackTex, sf::Vector2f(0.f,0.f)));
	m_nav->sort();
	m_nav->giveFocusToShape(0U);
}

more fun:

void MenuNav::addButton(const MenuButton button)
{
	m_buttons.push_back(button);
	std::cerr << "shiet" ; //displays on the console
}

and here is my call stack:

msvcr110.dll!free(void * pBlock) Line 51 C
DatBlaster.exe!MenuButton::~MenuButton() Line 14 C++
DatBlaster.exe!Shop::Shop() Line 40 C++

It means an object of type MenuButton has been destroyed...before the end of the constructor?! Or more precisely at the end of the addbutton method?
If you see something wrong with my code or just want to ask something, feel free to answer :)
Thank you

[LB]

The parameter to addButton is passed by value, resulting in a copy and later a destroy. I think you should investigate your destructor code.

[SIK]

seems like you should change object [MenuButton ttt1] from a stack based one to a heap based object. ie. create a pointer of type MenuButton as a member of class Shop. you should then delete this pointer in destructor of Shop.

Doing this should ensure that ttt1 does not get destructed at end of Shop constructor.

[LB]

No, don't do that - resorting to dynamic memory should be the last option.

[Computergeek01]

Just for the heck of it, you might want to double check your copy constructor for "MenuButton" while you're at it if the destructor doesn't appear to be doing anything weird. You could of course post the code for both and we could help you look at them.

[yhu420]

I'm really stuck with that bug maybe this can help you out:
hpp

class MenuButton : public RoundRect
{
public:
	MenuButton(sf::Texture* NormalTexture, sf::Texture* hoveredTexture, sf::Texture* clicktex, const sf::Vector2f& position, const sf::Vector2f& dimensions = sf::Vector2f(162,100));
	~MenuButton();
	//More methods

private:
	bool isHovered;
	bool isUsed;

	FullSprite* m_attribute;

	sf::Texture* m_normTex;
	sf::Texture* m_hoverTex;
	sf::Texture* m_clickedTex;
};

cpp

MenuButton::MenuButton(sf::Texture* NormalTexture, sf::Texture* hoveredTexture, sf::Texture* clicktex, const sf::Vector2f& position, const sf::Vector2f& dimensions) : RoundRect(position , dimensions ,5.f,3U, NormalTexture), isHovered(false), isUsed(false)
{
	m_normTex = NormalTexture;
	m_hoverTex = hoveredTexture;
	m_clickedTex = clicktex;
}

MenuButton::~MenuButton()
{
	delete m_attribute;
	delete m_hoverTex;
	delete m_clickedTex;
	delete m_normTex;

	m_attribute = nullptr;
	m_hoverTex = nullptr;
	m_clickedTex = nullptr;
	m_normTex = nullptr;
}

I really can't figure out what causes the issue, tell me if you need more code :/

[LB]

m_attribute is never initialized, so it contains garbage data and is most likely not going to be the null pointer. So, when you call delete on it, your program crashes because you're calling delete on random memory that you didn't allocate.

To fix, just initialize it to nullptr in your constructor.

Also, this looks like abuse of pointers and dynamic memory - if they are bound to the lifetime of the MenuButton object anyway, why are they dynamically allocated?

EDIT: Woah, I completely missed the fact that you didn't implement a correct copy constructor! The one generated for you by the compiler just copies the pointers, so your pointers get deleted twice!

[cire]

I really can't figure out what causes the issue, tell me if you need more code :/

http://www.drdobbs.com/c-made-easier-the-rule-of-three/184401400

[Disch]

IMO, if you're manually deleting, you're doing it wrong. Any kind of resource allocation should be in an RAII container. Here, you could have smart pointers (std::unique_ptr) for your textures, eliminating any need for a destructor, and alleviating any worry about initialization or cleanup.

[yhu420]

Thanks for all your replies. I fixed the memory allocating issue, but I think we're getting away from the first point. Why is my destructor called anyway? I never wanted to destroy any MenuButton before the
std::cerr << "OIOIIOIOIOIOIO" << std::endl; //never shows up in the console
line so why is the destructor even called?

[Computergeek01]

Because your "AddButton()" function receives a copy of the instance as an argument. There is nothing to keep this instance of the object alive once that function exits so it gets destroyed when execution leaves that scope.

[MikeyBoy]

Why is my destructor called anyway?

L B explained this in the very first reply you got in this thread.

[yhu420]

Thanks everyone for answering my question.
Thanks to all of you guys I enhanced my code, you really helped me out :)