0
votes

I have a memory leak with this piece of code and I don't understand why.

Each thread calls the function exec. The function exec simply creates a std::vector and than delete it. This vector has length equal to the number of threads and it is created, and deleted, only once.

You can suppose that this code is thread-safe in the sense that the vector is deleted only after the creation.

class Foo{
  public:
  Foo(const std::size_t& numThreads):size_(numThreads){}
  inline void alloc(){std::call_once(bufferflag_,&Foo::alloc_,this);}
  inline void free(){std::call_once(bufferflag_,&Foo::free_,this);}

  private:
  const std::size_t size_;
  std::vector<double>* bufferptr_;
  std::once_flag bufferflag_;

  inline void alloc_(){bufferptr_ = new std::vector<double>(size_);}
  inline void free_(){delete [] bufferptr_;}
};

void exec(Foo& comm){
  comm.alloc();
  // sync the threads here with some barrier
  comm.free();
}

void main(){
  Foo comm(10);
  std::vector<std::thread> t(10);
  for(std::size_t tid=0;tid!=10;++tid) t[tid]=std::thread(exec,std::ref(comm));
  for(std::size_t tid=0;tid!=10;++tid) t[tid].join();
}

HEAP SUMMARY:

in use at exit: 104 bytes in 2 blocks

total heap usage: 23 allocs, 21 frees, 3,704 bytes allocated

104 (24 direct, 80 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 2

LEAK SUMMARY:

definitely lost: 24 bytes in 1 blocks

indirectly lost: 80 bytes in 1 blocks

possibly lost: 0 bytes in 0 blocks

still reachable: 0 bytes in 0 blocks

suppressed: 0 bytes in 0 blocks

UPDATE

If instead of using call_once I just call the new and delete from the same thread, there are no memory leaks.

1
Maybe you need to call bufferptr.clear() before deleting the pointer in exec function? 80B is sizeof(double) * 10 - VAndrei
If I call bufferptr_->clear() before the delete the heap and leak summary is exactly the same. - Marco Agnese
How are you ensuring that alloc is called before free? - Kerrek SB
This program is one giant race condition. - Kerrek SB
Between the call of the alloc and of the free I use a barrier to synchronize the threads. I haven't posted the full code because it becomes to long and less readable. You can add a lengthy computation in order to be sure that the two function are called in the rigth order or implement a barrier to sync the threads. - Marco Agnese

1 Answers

0
votes

Take a look at this modified code:

class Foo
{
public:
    Foo(const std::size_t& numThreads) :size_(numThreads) {}
    inline void alloc()
    {
        printf("alloc\n");
        std::call_once(alloc_flag, &Foo::alloc_, this);
    }
    inline void foo_free()
    {
        printf("free\n");
        std::call_once(free_flag, &Foo::free_, this); // Changed from bufferflag
    }
private:
    const std::size_t size_;
    std::vector<double>* bufferptr_;
    std::once_flag alloc_flag;
    std::once_flag free_flag;

    inline void alloc_()
    { 
        printf("once_alloc_!\n");
        bufferptr_ = new std::vector<double>(size_);
    }
    inline void free_()
    { 
        printf("once_free_!\n");
        bufferptr_->clear();
        delete bufferptr_; // Not delete[] bufferptr_
        bufferptr_ = NULL;
    }
};

void exec(Foo& comm){
    comm.alloc();
    // Barrier
    comm.foo_free();
}

If you use 2 different once_flag for alloc and free, you run also the free_() method. The fact that you are using only one flag generates a memory leak because the flag is consumed by the first alloc.

To check memory leaks I use _CrtDumpMemoryLeaks(); and I get some extra leaks even if I comment out the thread create/join routines but I think this std::vector thrash. Hope this helps!