Jump to content
  • Advertisement
Sign in to follow this  
Sol Blue

Object can be deleted in one dtor but not another! Why?

This topic is 3853 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

Hi all, I have a very puzzling problem that has been hard to find any help with on Google or on the archives here. I have a particular object which can be deleted correctly in one destructor -- but if instead that deletion occurs in a different destructor, I get a debug assertion failure. I'll post some code and try to explain. A HumanGameView is new'd in another part of the program, and then HumanGameView::init() is called. init() creates two objects: a KeyboardInterpreter and a ProcessManager. My class KeyboardInterpreter derives from class Process, and so it gets loaded into the ProcessManager, which will eventually be responsible for updating it every game cycle. The ProcessManager, in its dtor, goes through all the processes it has been handed (the KeyboardInterpreter is the only one so far) and deletes them. This is where the problem lies! When the ProcessManager dtor tries to delete the KeyboardInterpreter, I get a debug assertion failure in dbgdel.cpp, with the following expression: "_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)". If, relenting, I never add the KeyboardInterpreter to the ProcessManager's list, and just delete it myself in HumanGameView, it works fine -- but this breaks my design. I hope someone with more expertise than me can tell me what's going on!
HumanGameView::HumanGameView() : Process(), mProcessManager(0), mKeyboardInterpreter(0)
{
	// This line isn't involved in the problem.
	HR(D3DXCreateSprite(gDevice, &mSprite));
}

HumanGameView::~HumanGameView()
{
	// This line isn't involved in the problem.
	RELEASECOM(mSprite);

	if (mProcessManager)
	{
		delete mProcessManager;
		mProcessManager = 0;
	}

	// If instead of being given to the ProcessManager, whose dtor
	// deletes it, the KeyboardInterpreter is deleted here, there's no
	// problem.
	if (mKeyboardInterpreter)
	{
		delete mKeyboardInterpreter;
		mKeyboardInterpreter = 0;
	}
}

bool HumanGameView::init()
{
	mKeyboardInterpreter = new KeyboardInterpreter();

	if (!mKeyboardInterpreter->init())
		return false;

	mProcessManager = new ProcessManager();

	// Commenting this next line fixes the problem, because the
	// ProcessManager dtor never tries to delete mKeyboardInterpreter.
	mProcessManager->addProcess(mKeyboardInterpreter);

	return true;
}

ProcessManager::~ProcessManager()
{
	// Delete all the processes in our list. The ProcessManager is
	// responsible for handling pointer deletion, not the Process creator.
	std::list<Process*>::iterator it;
	for (it = mProcessList.begin(); it != mProcessList.end(); ++it)
	{
		Process* prcs = *it;
		
		if (prcs)
		{
			// This delete line, when prcs is the KeyboardInterpreter,
			// causes the debug assertion failure.
			delete prcs;
			prcs = 0;
		}
	}
}

void ProcessManager::addProcess(Process* process)
{
	mProcessList.push_back(process);
}

Share this post


Link to post
Share on other sites
Advertisement
I might have missed something, but it seems like you're deleting the same object twice.

Is ~HumanGameView being called before ~ProcessManager?

Share this post


Link to post
Share on other sites
If you are sharing pointers between classes, best to make it explicit by using boost::shared_ptr<>. In idiomatic C++, you rarely call delete, instead you use RAII pointer objects to manage memory. You express ownership semantics by choosing between the different types of RAII pointer objects.

Remember, setting a deleted pointer to 0 cannot help when there are additional pointers pointing at the (now deallocated) memory.

Share this post


Link to post
Share on other sites
Quote:
Is ~HumanGameView being called before ~ProcessManager?


Yes. ~HumanGameView calls ~ProcessManager, both of which will delete the KeyboardInterpreter if it is 0. It won't be deleted twice, though. The ProcessManager destructor deletes the KeyboardInterpreter and sets the pointer to 0. Because the pointer is 0, nothing tries to delete it again.

I guess I should learn Boost, but I didn't think I was writing code complex enough to need it yet. It seems like what I'm doing here should work.

Does anyone know what that assert failure even means? Does it imply a bad pointer, or something's out of scope, or...? From my testing, I haven't been able to find anything I know how to check...:(

Share this post


Link to post
Share on other sites
The KeyboardInterpreter is deleted in your ProcessManager, but mKeyboardInterpreter in HumanGameView still points to this deleted memory (it isn't set to 0 by ProcessManager). kaboom.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sol Blue
Because the pointer is 0, nothing tries to delete it again.


Incorrect. You have two distinct pointers. While you delete one and set it to 0, the other still holds a value.

You need to define: who owns the pointer. Only the pointers owner should delete it. In the case of shared pointers, it can be difficult to assign an owner. In this case, boost::shared_ptr<> is the best solution. Download boost, it will do no end of good for your C++ programming.

Share this post


Link to post
Share on other sites
Quote:
Original post by boersch
The KeyboardInterpreter is deleted in your ProcessManager, but mKeyboardInterpreter in HumanGameView still points to this deleted memory (it isn't set to 0 by ProcessManager). kaboom.


I think the assertion fails before I get to that second deletion, but I'll try removing it when I get back to my room again. You and rip-off are right about the pointers, so I'll take it out anyway. Thanks!


Quote:
Original post by
And, incidentally, doing

if (pFoo)

{

delete pFoo;

pFoo = 0;

}



is pointless. It's perfectly safe to delete a NULL pointer.


Ah, I didn't know that, thanks. It doesn't give you an access violation error for trying to access 0x00000000? I saw things deleted like this in a couple of books, so I imitated it. Is there another reason people sometimes put that in there?

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!