Jump to content
  • Advertisement
Sign in to follow this  
Arcibald Wearlot

std::vector - deleting pointers

This topic is 5412 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 really cannot understand why this is not working:
class IsEmitterDead
{
public:
	bool operator()(const CParticleEmitter* Em)	 const
	{
		return Em->IsDead();
	}
};

class DeleteEmitter
{
public:
	void operator()(CParticleEmitter* Em)	
	{
		delete Em;
	}
};

vector<CParticleEmitter*>::iterator iDeadStart=remove_if(m_Emitters.begin(),m_Emitters.end(),IsEmitterDead());
for_each(iDeadStart,m_Emitters.end(),DeleteEmitter());
m_Emitters.erase(iDeadStart,m_Emitters.end());

if I comment out the for_each() everything works, but there is a memory leak since the emitter pointers are not deleted, just removed from the vector. if i call DeleteEmitter() using for_each(), the program throws an unknown exception or crashes. If I use this, the same happens:
for(vector<CParticleEmitter*>::iterator iEm=iDeadStart;iEm!=m_Emitters.end();iEm++)
{
	delete (*iEm);
}

Am I doing something wrong? how do you delete pointers from a STL vector?

Share this post


Link to post
Share on other sites
Advertisement
When you use remove_if all the elements that evaluate to true from the predicate they get destroyed but the memory is not deallocated so the original addressess are gone.

If there is a real need to store pointers in STL containers i recommend you use smart pointers instead, such as boost's smart pointers in particular shared_ptr it will take-care of how & when to delete the instances they refer to for you thus making life easier & harmonious [smile].

EDIT: note if you do download boost library you call also use boost's pool allocator for STL containers, this will be good to use with smart pointers aswell.

Share this post


Link to post
Share on other sites
Could you post more code?
the second method (the loop) should work, not sure about the
first, remove_if is a bit erm... interesting....
Quote:
SGI STL Documentation "So, for example, if V is a vector, remove_if(V.begin(), V.end(), pred) does not change V.size(): V will contain just as many elements as it did before. Remove_if returns an iterator that points to the end of the resulting range after elements have been removed from it; it follows that the elements after that iterator are of no interest, and may be discarded."

Share this post


Link to post
Share on other sites
Quote:
Original post by silvermace
Could you post more code?
the second method (the loop) should work, not sure about the
first, remove_if is a bit erm... interesting....
Quote:
SGI STL Documentation "So, for example, if V is a vector, remove_if(V.begin(), V.end(), pred) does not change V.size(): V will contain just as many elements as it did before. Remove_if returns an iterator that points to the end of the resulting range after elements have been removed from it; it follows that the elements after that iterator are of no interest, and may be discarded."


Yes both should work but like i've already said, he uses remove_if before trying to delete the instances the pointers refer to, and just as you've quoted remove_if will destroy the elements that are evaluated true from the predicate but does not deallocate the memory, its basically invalidated the pointers.

Share this post


Link to post
Share on other sites
remove_if overwrites 'removed' elements with elements from later in the container. However the actual number of elements is not modified. Which means that, at the end of the container, there still are duplicates left over from the move. When you erase from begin to end, those duplicates cause a double delete error.


v (write)
ABCDEF remove the underlined items
^ (read)

v
ABCDEF Keep element.
^
v
ABCDEF Element must be removed, only advance the read head.
^
v
ACCDEF Overwrite.
^
v
ACDDEF Overwrite.
^
v
ACDDEF Element must be removed, only advance the read head.
^
v
ACDFEF Overwrite
^
v returned iterator
ACDFEF leftover garbage
^ c.end() reached - terminate


One way you could deal with things would be to use std::partition instead of std::remove_if. It will separate elements for which the predicate is true from those for which the predicate is false. Then you can do whatever you wish to each of the subranges.

Or you could write a new algorithm: for_each_if
Or a custom functor that tests whether to perform the deletion or not - possibly taking another functor in its constructor to use as a predicate.

The possibilities are almost endless.

edit - whoops I messed up, duplicate posts removed.

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
remove_if overwrites 'removed' elements with elements from later in the container.


Are you sure it overwrites? on GCC 3.4.2 & VC++ 7.1 the elements that are to be removed get destroyed (there destructors called) but the memory not deallocated, maybe its implementation dependent?

anyways basically those pointers become invalid before they reach his code that trys to delete the instances that are no longer referenced by anything, i really think a container of boost::shared_ptr in combination with boost::pool_allocator/fast_pool_allocator is the way to go.

[Edited by - snk_kid on September 26, 2004 6:41:49 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by snk_kid
Are you sure it overwrites? on GCC 3.4.2 & VC++ 7.1 the elements that are to be removed get destroyed (there destructors called) but the memory not deallocated, maybe its implementation dependent?


Well, it obviously is implementation-dependent, but I believe they could just assign over elements to be removed. That would be consistent with the Assignable requirement for elements, and the erase-remove idiom. It also seems to me that it's about the most efficient way to do it.

Quote:
anyways basically those pointers become invalid before they reach his code that trys to delete the instances that are no longer referenced by anything


He is deleting elements from the 'garbage' section, which aren't those elements he has removed, but leftover copies from the move (e.g. F in my example). The crash could be due to his later use of the deleted objects. And he'd still have a memory leak since he's not deleting the pointers he removed from the container.

Pointers don't have a destructor anyway, and it would be a grievous mistake for the library to call delete on them, as it would assume some very specific ownership semantics, severly limiting the generality of the library (scalar vs. array delete, pointers to static or stack variables, multiple containers holding the same pointers).

Quote:
i really think boost::smart_ptr in combination with boost::pool_allocator is the way to go.


Yeah, boost::smart_ptr solves a lot of the problems you may have with containers of pointers. Since you had already broached the topic, I wanted to offer an alternative, namely using std::partition to separate elements to be deleted from elements to be kept.


#include <functional>

struct IsEmitterDead
: std::unary_function<CParticleEmitter*, bool>
{
bool operator()(const CParticleEmitter* Em) const
{
return Em->IsDead();
}
};

template<typename T> struct scalar_delete
: std::unary_function<T*, void>
{
void operator()(T* ptr) const { delete ptr; }
};

template<typename T> struct array_delete
: std::unary_function<T*, void>
{
void operator()(T* ptr) const { delete[] ptr; }
};

vector<CParticleEmitter*>::iterator iDeadStart;

// Elements for which the predicate is true go before those
// for which the predicate is false. We use the std::not1()
// adaptor to negate IsEmitterDead and put the live emitters
// in front. It would probably be easier to directly have a
// IsEmitterALive functor though.

iDeadStart=partition(m_Emitters.begin(), m_Emitters.end(),
not1(IsEmitterDead()))

// Do a scalar delete on the dead elements
for_each(iDeadStart, m_Emitters.end(), scalar_delete<CParticleEmitter>());

m_Emitters.erase(iDeadStart, m_Emitters.end());

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
Quote:
Original post by snk_kid
Are you sure it overwrites? on GCC 3.4.2 & VC++ 7.1 the elements that are to be removed get destroyed (there destructors called) but the memory not deallocated, maybe its implementation dependent?


Well, it obviously is implementation-dependent, but I believe they could just assign over elements to be removed. That would be consistent with the Assignable requirement for elements, and the erase-remove idiom. It also seems to me that it's about the most efficient way to do it.


I think your wright i'm just losing my mind.

Quote:
Original post by Fruny
Pointers don't have a destructor anyway, and it would be a grievous mistake for the library to call delete on them, as it would assume some very specific ownership semantics, severly limiting the generality of the library (scalar vs. array delete, pointers to static or stack variables, multiple containers holding the same pointers).


Again i'm losing my mind, i was using a test version with a user defined type thats why i was going on about destructors, i think your wright thou its overwritten but if it did try to call the destructor then there is probably some type traits to handle built-in primitive types.

[Edited by - snk_kid on September 26, 2004 6:26:39 PM]

Share this post


Link to post
Share on other sites
If your particles have a short life, its probable best not to be constantly using new and delete on them anyways. Its better to just allocate a chunk of memory as big as the maximum number of particles and leave it intact as long as the particle system is alive. You just have to track how many particles are alive and keep the living particles at the front of your memory chunk and the dead ones at the back. To add a particle you add at position count and increase count by one. To delete a particle, swap the particle with the particle at count-1 and decrease count by one. No memory ever gets allocated or deallocated except when the entire system is created or destroyed, and all particles are contiguous in memory, so heap fragmentation is minimized. If your particles are large swap pointers into your particle array instead of moving the particles themselves.

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.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!