[SOLVED] Casting and memory management

[azanin]

Hello,

 

I'm having a crash due to the memory management and I'd like to understand what is happening.

 

In our class MeshHelper ctor:

NodePtr pNode = World::get()->loadNode("myNode.node", 1);

_mesh = loadMesh(pNode); // _mesh being a ObjectPtr member of my class
if (_mesh->getNode()->getType() == ObjectMeshSkinned::type())
{
    _meshAnim = std::make_unique<AnimationHelper>(_mesh); // _meshAnim being an other member of our class
}

In this snippet, loadMesh is a factory that create the proper type of ObjectMesh according to pNode type and returns it as an ObjectPtr. For instance, if the Node type is ObjectMeshStatic, it will do:

ObjectPtr loadMesh(NodePtr pNode)
{
    if (pNode->getType() == ObjectMeshStatic::type())
        return ObjectMeshStatic::create(pNode)->getObject();
    else if (...)
    {...}
}

AnimationHelper is one of our classes that handles several animation-related operations. Here's AnimationHelper ctor:

AnimationHelper(const ObjectPtr& pObject)
{
    _mesh = ObjectMeshSkinned::create(pObject); // _mesh being an ObjectMeshSkinned member
   // ...
}

So far, so good, but the crash occurs as soon as I destroy the MeshHelper instance. I have tried two things.

void destroyChildren(NodePtr pNode)
{
    while (pNode->getNumChildren() > 0)
    {
        NodePtr child = pNode->getChild(0);
        child->grab();
        destroyChildren(child);
        child.destroy();
    }
}

~MeshHelper()
{
    NodePtr pNode = _mesh->getNode();
    pNode->grab();
    destroyChildren(pNode);
    pNode.destroy();
}

In that case, if _meshAnim was created in MeshHelper ctor, it crashes with the following message:

HeapChunk::deallocate(): chunk is not allocated

The other thing I've tried is:

~MeshHelper()
{
    if (_mesh)
    {
        _mesh->grab();
        _mesh.destroy();
    }
}

This fixes the crash. However, I have an enabled particle system bound to my mesh. If I delete the mesh using that method, the particle system is not destroyed.

 

Can you explain me what is broken in the way I'm managing my objects?

Thank you.

[maxi]

Hi Alexandre,

 

I didn't saw nothing criminal in your code, I've also made some sample and it is working fine:

void destroyChildren(NodePtr pNode)
{
	while (pNode->getNumChildren() > 0)
	{
		NodePtr child = pNode->getChild(0);
		child->grab();
		destroyChildren(child);
		child.destroy();
	}
}

class AnimationHelper
{
public:
	AnimationHelper(const ObjectPtr &obj)
	{
		anim_obj = ObjectMeshSkinned::create(obj);
	}
	~AnimationHelper()
	{
	}

	ObjectMeshSkinnedPtr anim_obj;
};

class Helper
{
public:
	Helper()
	{
		NodePtr pNode = World::get()->loadNode("data/box2.node", 1);

		mesh = ObjectMeshSkinned::create(pNode)->getObject();
		if (mesh->getNode()->getType() == ObjectMeshSkinned::type())
		{
			anim = std::make_unique<AnimationHelper>(mesh);
		}
	}

	~Helper()
	{
		NodePtr pNode = mesh->getNode();
		pNode->grab();
		destroyChildren(pNode);
		pNode.destroy();
	}

	ObjectPtr mesh;
	std::unique_ptr<AnimationHelper> anim;
};

// in world init
Helper *h = new Helper();
delete h;

Looks like you are deleting some of your already deleted objects somewhere else

[azanin]

Thank you for your answer.

 

What might be happening is:

- I create a node

- I keep a Ptr on one of this node children in one of my members

- I delete the node in my object destructor

- After that destruction, my object destroys the rest of its member, including the one holding a Ptr on the node's child. That Ptr tries to access that child, that has been destroyed by my first object destructor.

 

I think I shouldn't call manually the destruction code, there's probably something I'm missing about the ownership system.

Let's say my MeshHelper class is responsible for a Node lifetime and that it owns a member AnimationHelper, that owns a NodePtr on one of the node children. What is the best practice for handling this?

 

I see there's a grab/release mechanism. What I understand from the documentation is that calling

NodePtr myNode = World::get()->loadNode("data/box2.node", 1);
myNode->grab();

in C++ creates a Node instance and tells the engine that the C++ side is responsible for the lifetime of the object (not the script). If I copy myNode through my classes, I increment the reference count. When that reference count reaches 0, the object is automatically destroyed. The same goes with its children. If another object holds a Ptr on one of these children, it should be destroyed once that object is destroyed as well.

 

Where am I mistaking?

[maxi]

Hi Alexandre,

unfortunatelly, node's methods grab and release doesn't have reference counter. Api's node is a wrapper around engine's node and has an owner flag (Node::isOwner), and is actually destroing engine's node in destructor only if the owner flag is set. Grab and release methods sets owner flag to 1 and 0 accordinally. When you delete an owning NodePtr, other Ptr's will not be deleted automatically and from this moment they are having invalid poiners to the engine's node. So the rules are:

- Only one NodePtr can be an owner, otherwise they all will try to delete the same pointer on destruction.

- If an owning NodePtr was deleted, all other NodePtr's now have invalid pointers, so you can't call api's methods with them (but you can still call managment related methods: grab, release, isOwner). So to correctly destroy such invalid NodePtr you can call: node_ptr->release(); node-ptr.destroy();

 

Also note, if you create new engine node by calling NodePtr ptr = NodeDummy::create() for example, an owning NodePtr will be created, and if you call cast operationg, for example: ObjectMeshStaticPtr ptr = ObjectMeshStatic::create(node_ptr), created pointer will not be owning.

[azanin]

Thanks, that's a bit more clear to me now.

But I think I've understood one more thing since my last question.

 

From my undestanding, there are two levels of indirection. NodePtr is a pointer to a NodeInterface which is a pointer to the actual Node. NodeInterface ownership system (grab/release) is used to handle the lifetime of the actual Node, while NodePtr reference counting is used to handle the lifetime management of one NodeInterface. Am I right?

 

Now if I have:

ObjectMeshStaticPtr pMesh;
{
    // Creation of a new Node, a new NodeInterface and a new Ptr pointing to that NodeInterface.
    NodePtr pNode = World::get()->loadNode("myNode.node", 1);
    std::cout << pNode->isOwner() // Print false
    // Creation of a new NodeInterface on the same Node. Returns a new Ptr pointing to that new NodeInterface
    pMesh = ObjectMeshStatic::create(pNode);
    std::cout << pMesh->isOwner() // Print false
    pMesh->grab();
}
// By that point pNode is destroyed => the first NodeInterface is destroyed. But the actual object still exist 
// because I grabbed it with the second NodeInterface I created with ObjectMeshStatic::create. For the whole scope
// both pMesh and pNode counters are equal to 1 because they are pointing to different NodeInterface.

Is there any mistake in my reasonning?

 

Also, when I call Ptr::destroy, what exactly is being destroyed? The NodeInterface or the actual Node?

[maxi]

Yes, you are right. One more elaborating: engine's node will not be destroyed out of scope no matter you call grab or not, because no owning NodeInterface will be destroyed.

Ptr::destroy will delete NodeInterface and NodeInterface in it's turn will delete Node only if ownership is set.

[azanin]

Alright, thank you very much for these precisions! :)