Code crash when trying to create an object

[LittleGiant]

Code: Select all

		IAnimatedMesh*	m_pWorldMesh;
		ISceneNode*		m_pWorldNode;

		m_pWorldMesh = GetSceneManager()->getMesh("20kdm2.bsp");
		m_pWorldNode = NULL;

		if (m_pWorldMesh)
		{
			m_pWorldNode = GetSceneManager()->addOctTreeSceneNode(m_pWorldMesh->getMesh(0));
			if( m_pWorldNode )
			{
				m_pWorldNode->setPosition( InnerLib::vector3f( -1300,-144,-1249 ) );
			}
		}

		m_pWorldNode->drop();
		
		m_pWorldMesh = GetSceneManager()->getMesh("20kdm2.bsp");
		m_pWorldNode = NULL;

		if (m_pWorldMesh)
		{
			m_pWorldNode = GetSceneManager()->addOctTreeSceneNode(m_pWorldMesh->getMesh(0));
			if( m_pWorldNode )
			{
				m_pWorldNode->setPosition( InnerLib::vector3f( -1300,-144,-1249 ) );
			}
		}

This crashes on this line:

Code: Select all

m_pWorldNode->setPosition( InnerLib::vector3f( -1300,-144,-1249 ) );

the second time it's being executed, appears that the pointer returned by

Code: Select all

GetSceneManager()->addOctTreeSceneNode(m_pWorldMesh->getMesh(0));

is not null but also not valid.

Any insight into whats going on would be great.

Note:
This is obviously contrived code, but it reproduces the error i'm getting without having to throw all sorts of code and classes at you.

[LittleGiant]

Further information:

I've stepped into this and here is the code with added comment explaining:

Code: Select all

//! Adss a scene node for rendering using a octtree. This a good method for rendering
//! scenes with lots of geometry. The Octree is built on the fly from the mesh, much
//! faster then a bsp tree.
ISceneNode* CSceneManager::addOctTreeSceneNode(IMesh* mesh, ISceneNode* parent,
		s32 id, s32 minimalPolysPerNode, bool alsoAddIfMeshPointerZero)
{
	if (!alsoAddIfMeshPointerZero && !mesh)
		return 0;

	if (!parent)
		parent = this;

	COctTreeSceneNode* node = new COctTreeSceneNode(parent, this, id, minimalPolysPerNode);

	if (mesh)
		node->createTree(mesh);

	node->drop();  // Inside this drop it deletes itself

	return node;  // here is where it returns a trashed pointer
}

I clearly don't fully understand all the reference counting nuances in Irrlitch yet, but since drop() returns a bool shouldn't this at least check it before return a possibly thrashed pointer?

[rogerborg]

This is an interesting one.

The call to m_pWorldNode->drop(); is the root cause. Your user application should only drop() references that are returned from createFoo() methods, not addFoo().

The node is returned to you with a reference count of 1, so the drop() causes the node to delete itself. Unfortunately, while a deleted scene node does remove its children, it doesn't remove itself from its parent (because it assumes that it's being removed by its parent), so its parent node still thinks it's present in the scene. (If you do want to remove the node from the scene, call remove() on it).

The interesting part is the behaviour that you are then seeing. The second call to new COctTreeSceneNode() is - in your particular development environment - re-using the same memory as the node that you just inadvertently deleted. The new node's parent (the root scene node) does a pointer comparison, and decides that the new node is already in its list of children (because it doesn't know that the old node was deleted). It therefore doesn't take a reference to it, so the new node's ref count is only 1 instead of the expected 2.

The drop() inside CSceneManager::addOctTreeSceneNode() then causes the new node to be unexpectedly deleted.

I've previously argued that this behaviour is wrong, for the very reason that you've just found: it's not tolerant of error conditions. The Irrlicht code shouldn't make assumptions about reference counts. At the very least, it should assert expected values to try and make the points of failure clearer.

That said, I'm not volunteering to fix it, so I won't complain too loudly.

[LittleGiant]

Thanks! That was a great reply, very clearly explained.

I agree with you on your points about such assumptions, i mean before linking to the debug library there is no way to really figure out whats going on let alone find the root of the issue.

I'll fix this tonight when I'm home from work, but thanks for the information about proper use of drop() and remove() that will prove to be the most important thing i learn from this problem.

[MasterGod]

This is an interesting one.

The call to m_pWorldNode->drop(); is the root cause. Your user application should only drop() references that are returned from createFoo() methods, not addFoo().

The node is returned to you with a reference count of 1, so the drop() causes the node to delete itself. Unfortunately, while a deleted scene node does remove its children, it doesn't remove itself from its parent (because it assumes that it's being removed by its parent), so its parent node still thinks it's present in the scene. (If you do want to remove the node from the scene, call remove() on it).

The interesting part is the behaviour that you are then seeing. The second call to new COctTreeSceneNode() is - in your particular development environment - re-using the same memory as the node that you just inadvertently deleted. The new node's parent (the root scene node) does a pointer comparison, and decides that the new node is already in its list of children (because it doesn't know that the old node was deleted). It therefore doesn't take a reference to it, so the new node's ref count is only 1 instead of the expected 2.

The drop() inside CSceneManager::addOctTreeSceneNode() then causes the new node to be unexpectedly deleted.

I've previously argued that this behaviour is wrong, for the very reason that you've just found: it's not tolerant of error conditions. The Irrlicht code shouldn't make assumptions about reference counts. At the very least, it should assert expected values to try and make the points of failure clearer.

That said, I'm not volunteering to fix it, so I won't complain too loudly.

Interesting indeed.
Such problems are very interesting IMO, how reused memory causes unexpected behavior.

P.S
How the hell did you (rogerborg) knew that this was the problem?

And really nice answer.

[rogerborg]

Such problems are very interesting IMO, how reused memory causes unexpected behavior.

P.S
How the hell did you (rogerborg) knew that this was the problem?

I didn't, but the debugger did, and I plied him with strong liquor and loose women until he told me.