Dangling pointers in vectors

Started by
13 comments, last by 3DModelerMan 11 years, 7 months ago

...
which iterates every gameloop and renders images if needed. If the Unit is deleted its component Renderable is deleted, but the class Renderer now has a dangling pointer in renderableEntities. This can be fixed by always letting Renderable knowing in which vectors it exists in by having a pointer to the renderableEntities vector... In ~Renderable()
...


Just be careful not to remove a Renderable while you're iterating over the renderableEntities vector.
Advertisement
I use a reference counting system. It's a simple ReferenceCounted interface with grab() and release() methods. If you want to see a good implementation look at Irrlicht's. The difference though is that I wrote a RefHandle template. I use it to replace pointers. All it does is grab()s the object in it's constructor, and release()s it in it's destructor (there's some code for copying too, but that's the basic idea). It works great and keeps messy pointer management out of the way.

To summarize: When an Unit is created, its component Renderable sends a pointer of itself to Renderer. Renderer has a vector renderableEntities


There's your problem.

The Renderer should not contain a vector of renderable entities. Instead, you should get the renderable when needed, by iterating over the global units container:


for (int i = 0; i < units.size(); ++i){
renderer.drawRenderable(units.getRenderable());
}

+---------------------------------------------------------------------+

| Game Dev video tutorials -> http://www.youtube.com/goranmilovano | +---------------------------------------------------------------------+

[quote name='Kuxe' timestamp='1348492844' post='4983211']
To summarize: When an Unit is created, its component Renderable sends a pointer of itself to Renderer. Renderer has a vector renderableEntities


There's your problem.

The Renderer should not contain a vector of renderable entities. Instead, you should get the renderable when needed, by iterating over the global units container:


for (int i = 0; i < units.size(); ++i){
renderer.drawRenderable(units.getRenderable());
}

[/quote]
Thanks Goran! It really makes more sence now that I think of it. As you might notice Im still getting the hang of OOP and general program architecture, but I must say that this thread has given me alot of nice pushes in the right direction and also concrete tips which I appreciate a lot.

Offtopic: This community is great.

If you don't understand the stuff written here, please sharpen your C++ skills.

By the way: you can use the reference counting class that I wrote if you want to.

[source lang="cpp"]#ifndef I_REFERENCE_COUNTED_H
#define I_REFERENCE_COUNTED_H

///@brief The IReferenceCounted interface is used for reference counted memory management. It is recommended to use the RefHandle class
///with IReferenceCounted instead of pointers.
///@brief TODO: implement ref counting as atomic operations.
class IReferenceCounted
{
public:

IReferenceCounted()
:m_refCount(0)
{}

virtual ~IReferenceCounted(){}

///@brief Returns the current reference count.
const unsigned int getReferenceCount(){return m_refCount;}

///@brief Adds one reference.
void retain() const
{
m_refCount++;
}

///@brief Subtracts one from the reference counter, and deletes the object if refcount drops to 0.
void release() const
{
m_refCount--;

if ( m_refCount <= 0 )
delete this;
}

///@brief Resets the reference counter to 0 without deleting the object (used to clone objects)
void reset()
{
m_refCount = 0;
}

protected:

mutable int m_refCount;
};

///@brief RefHandle provides a smart pointer for reference counted objects only use this handle with classes derived from IReferenceCounted
template<typename T>
class RefHandle
{
public:

RefHandle()
:m_obj(NULL)
{}

RefHandle(const RefHandle& oth)
{
m_obj = oth.m_obj;

if ( m_obj )
m_obj->retain();
}

RefHandle(T* obj)
:m_obj(obj)
{
if ( m_obj )
m_obj->retain();
}

~RefHandle()
{
if ( m_obj )
m_obj->release();
}

///@brief Releases the current object and sets the pointer to a different object.
///Passing NULL or 0 will release the object and won't add a new one.
void reset(T* obj)
{
if ( m_obj )
m_obj->release();

m_obj = obj;

if ( m_obj )
m_obj->retain();
}

///@return False if the handle points to no object. True if the handle points to a valid object.
bool isValid()
{
return m_obj != NULL;
}

///@brief Assigns a pointer to this object, equivalent to calling RefHandle::reset(T* obj)
RefHandle& operator = (T* obj){reset(obj); return *this;}

///@return The actual pointer.
T* get(){return m_obj;}

T* operator->() const {return m_obj;}
T& operator*() const {return *(get());}

///@brief Allows regular pointers to be assigned to from a RefHandle
operator T*() const {return m_obj;}
///@brief Allows RefHandle instances to be checked for validity. Equivalent to if ( ptr.isValid() )
operator bool() const {return m_obj!=NULL;}

bool operator==(const RefHandle& handle) {return handle.m_obj == m_obj;}
bool operator==(const T* obj) {return obj == m_obj;}

private:

T* m_obj;
};

#endif[/source]

This topic is closed to new replies.

Advertisement