Sign in to follow this  

Smart pointers are being dumb

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

I'm having some issues with my smart pointers... they're not accounting correctly, and I end up deleting the referenced object when there's still a pointer to it! Reading the code below will definitely be the key, but I'll just explain what's happening first. Ok, so we're in the main.cpp and we create a smart pointer to an object... let's call it A. We then pass A into another function in the class EntityManager, this is all on the stack so we create a copy named B. In EntityManager, we then pass this pointer to its QuadTree, and again a copy should be made and we can call it C. Finally, the object is added to the "QuadTree" (really a linked list for test purposes) and that should be pointer D. Watching the stack unwind, C is deleted, B is deleted and A is deleted (which is what we want). The problem is that when pointer C, the one going from the EntityManager to the QuadTree, is created, it isn't being counted as a reference. The copy constructor is never called for it, even though it's being called when we pass from Main to the EntityManager. Thus, we have 3 references and when the 3/4 references delete, we're left with 0 m_refs despite the pointer in the QuadTree. Any idea what's happening here?
class MemObj
{
protected:
	int m_refs;

public:
	static std::list<MemObj*> m_deadObjects;

	MemObj()
	:m_refs(0)
	{
	}

	virtual ~MemObj()
	{
	}

	void attach()
	{
		++m_refs;
	}
	void release()
	{
		--m_refs;
		if(m_refs==0)		// nothing points to the object
		{
			m_deadObjects.push_back(this);		// mark for deletion
		}
	}
};

/////////////////////////////////////////////////////////////////////////////
//	MEMPTR
/////////////////////////////////////////////////////////////////////////////
//
// AUTHOR: ERIK GOLDMAN
// DATE:  6-3-04
// DESC: A smart pointer to a memory-managed object
// NOTES:
// TOFIX:
/////////////////////////////////////////////////////////////////////////////

template <class T>
class MemPtr
{
protected:
	T *obj;

public:
	MemPtr()
	:obj(0)
	{
	}

	template <class U>	
	MemPtr(const MemPtr<U> &p)	
	:obj(p.getObj())	
	{	   
		if (obj)
			obj->attach();
	}	

	MemPtr(T *p)
	:obj(p)
	{
		if(obj)
			obj->attach();
	}

	~MemPtr()
	{
		if(obj)
			obj->release();
		obj=0;
	}

	inline MemPtr &operator=(T *p)
	{
		if(p != obj)		// prevent against self-assignment
		{
			if(obj)
				obj->release();
			obj=p;
			if(obj)
				obj->attach();
		}
		return *this;
	}

	inline MemPtr &operator=(const MemPtr &p)
	{
		if(p.obj != obj)	// prevent against self-assignment
		{
			if(obj)
				obj->release();
			obj=p.obj;
			if(obj)
				obj->attach();
		}
		return *this;
	}

	inline T& operator*() const
	{
		assert(obj!=0);
		return *obj;
	}

	inline T *operator->() const
	{
		assert(obj!=0);
		return obj;
	}

	inline T *getObj() const
	{
		return obj;
	}

	inline bool operator==(const MemPtr<T> &p) const
	{
		return(p.obj == obj);
	}

	inline bool operator ==(const T *p) const
	{
		return(obj == p);
	}

	inline bool operator !() const
	{
		return (obj == 0);
	}

	inline bool isValid() const
	{
		return (obj != 0);
	}
};

Everything seems to work fine, and my objects get cleaned up without much fuss. Now, I wanted to test my rendering code without the quadtree system I'll be using, so I decided to just dump my objects into a linked list for now. The code to add an entity traces as follows: From the main.cpp:
        MemPtr<EntityCube> c=new EntityCube(3, 3, 3);
	c->place(2, 0, 2);
	EntityManager::getInstance()->AddEntity(c);

	c=new EntityCube(5, 5, 5);
	c->place(-5, 0, 3);
	c->setColors(0, 1, 0);
	EntityManager::getInstance()->AddEntity(c);

	c=new EntityCube(1, 1, 6);
	c->place(12, 5, -5);
	c->setColors(0, 1, 0);
	EntityManager::getInstance()->AddEntity(c);

From the EntityManager class (don't comment about the wasteful design, stuff will be added later). **THIS IS WHERE NO MEMPTR COPY CONSTRUCTOR IS CALLED**
void EntityManager::AddEntity(MemPtr<Entity> entity)
{
	m_entities->AddEntity(entity);
}

****************************************************** m_entities is an object of type QuadTree (though it's using linked list code now):
	struct EntNode
	{
		EntNode():val(){}

		MemPtr<Entity> val;
		EntNode *next;
	};
	EntNode *m_entities, *n;	
--------------------------------------------------------------------
void QuadTree::AddEntity(MemPtr<Entity> entity)
{
	if(!m_entities)
	{
		m_entities=new EntNode();
		m_entities->val=entity;
		m_entities->next=0;
		n=m_entities;
	}
	else
	{
		n->next=new EntNode();
		n->next->val=entity;
		n->next->next=0;
		n=n->next;
	}
}

Share this post


Link to post
Share on other sites
template <class U>	
MemPtr(const MemPtr<U> &p)
:obj(p.getObj())
{
if (obj) obj->attach();
}

This is not a copy constructor but a templated conversion constructor.
You will have to also explicitely write out
MemPtr(const MemPtr<T> &p)	
:obj(p.getObj())
{
if (obj) obj->attach();
}

which is your class' copy constructor.

Share this post


Link to post
Share on other sites

This topic is 4713 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.

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