CODEWITHSUNDEEP
c++visual-studiolistmemory-leaks
relaxxx
relaxxx

Reputation: 7834

push_backing into list of pointers causes memory leak

I am trying to find memory leak with Visual Leak Detector. It shows me m_neighbors.push_back(ent); causes leak.

(brief callstack = NeighborCalculatorDummy -> foreach -> list -> allocate)

I use it as NeighborCalculatorDummy<Entity *>, so pushback should just insert pointer in list without any allocation. All pointers to entities which come through addEntity are deleted elsewhere in code...

How is it possible for push_back to cause a leak?

template <typename entity_type>
class NeighborCalculatorDummy
{
public:
    inline void addEntity(const entity_type & entity)
    {
        m_entities.push_back(entity);
    }

    void calculateNeighbors(const vector_type & position, flt32 radius)
    {
        flt32 rSq = radius*radius;
        m_neighbors.clear();

        std::for_each(m_entities.begin(), m_entities.end(), [&](entity_type ent){
            if(lengthSq(ent->getPosition() - position) <= rSq)
                m_neighbors.push_back(ent);
        });
    }

private:
    std::vector<entity_type> m_entities;
    std::list<entity_type> m_neighbors;
};

edit

here is the code around NeighborCalculator

//#1 
std::list<Vehicle *> vehicles;
vehicles.push_back(new Vehicle);
vehicles.push_back(new Vehicle);
vehicles.push_back(new Vehicle);

//#2 
NeighborCalculatorDummy<Vehicle *> neighborCalculator = new NeighborCalculatorDummy<Vehicle *>();

std::for_each(vehicles.begin(), vehicles.end(), [&](Vehicle * vehicle){
    neighborCalculator->addEntity(vehicle);
});

//#3 impl of addEntity
template <typename entity_type>
void NeighborCalculatorDummy<entity_type>::addEntity(const entity_type & entity)
{
    ...
    m_entities.push_back(entity);  //m_entities is - std::vector<Vehicle *> 
}

//#4 end of program
delete neighborCalculator;

std::for_each(vehicles.begin(), vehicles.end(), [&](Vehicle * vehicle){
    delete vehicle;
});

Upvotes: 0

Views: 908

Answers (3)

relaxxx
relaxxx

Reputation: 7834

I have omitted virtual destructor in Entity's parent. That is why pushbacking it caused a leak.

Upvotes: 0

Tobias Langner
Tobias Langner

Reputation: 10828

With this definition, depending on entity_type, the code will leak or not.

  • If entity_type is a pointer, the default destructor will just call the destructor of vector. This will free the memory allocated for these pointers, but it will not call delete on them. If this class "owns" the items inside the vector and needs to free them, you need to add a destructor that calls delete for all items inside the vector. In this case, you might think about the template parameter as well as it does not make clear that a pointer is needed here.
  • If entity_type is a value type, the default destructor is sufficient as the copies placed into the vector will be deleted by the destructor of the vector.

Upvotes: 0

sehe
sehe

Reputation: 393457

It seems to me that entity_type is a pointer (judging from the for_each lambda).

You probably wanted to use 

 NeighborCalculatorDummy<SomeEntity>

instead of

 NeighborCalculatorDummy<SomeEntity*>

in some other place of your code (not shown)

Of course the lambda would then be spelled differently:

[&](const entity_type& ent){
        if(lengthSq(ent.getPosition() - position) <= rSq)
            m_neighbors.push_back(ent);
    }

and perhaps more similar spots that assumed the type of entity_type needed dereferencing.

Alternatively, you could use

  • vector<std::shared_ptr<entity_type> > instead
  • Boost Pointer Containers

These might be more appropriate when your entities are polymorphic types or non-copyables/movables. However, it is also likely more work to change you code around

Upvotes: 1

Related Questions