0

I have some code here that implements a dynamic memory pool. The pool starts off at size 0 and grows with each successive allocation. It is used to try and minimise the overhead of tons of allocations and de-allocations.

The call to malloc is NOT matched by a call to free. It seems to be relying on the application that uses it to not call enough new's in succession for the application to leak a significant amount of memory.

I did not write it, so this is my best guess.

My question are:

  1. Is the absence of a call to free a bug or am I missing something to do with overloading the delete operator?
  2. Is this implementation relying on the OS to clean up the small amount of memory that does leak at exit?

Thanks.

//Obj.h
class Obj
{
public:
    Obj(){};
    void* operator new(std::size_t size);
    void operator delete(void* p);
private:
    static std::vector<void*> pool_;
    static std::size_t checked_in_;
    static std::size_t checked_out_;
};

//Obj.cpp
std::vector<void*> Obj::pool_;
std::size_t Obj::checked_out_ = 0;
std::size_t Obj::checked_in_  = 0;

void* Obj::operator new(std::size_t size)
{
    if (pool_.empty())
    {
        ++checked_out_;
        return malloc(size);
    }
    else
    {
        --checked_in_;
        ++checked_out_;
        void* p = pool_.back();
        pool_.pop_back();
        return p;
    }
}

void Obj::operator delete(void* p)
{
    pool_.push_back(p);
    if (pool_.size() % 10000 == 0)
    {
        std::cout<<"mem leak\n";
    }
    --checked_out_;
    ++checked_in_;
}
Carl
  • 43,122
  • 10
  • 80
  • 104
  • 1
    To be precise, it relies on the application that uses it not to dramatically reduce the number of allocations it requires from the pool at the same time. This is a pretty lousy implementation, because it would be trivial to solve the problem with a miniscule code adjustment. (Check the size before calling `push_back`. If the pool is full, instead of calling `push_back` and incrementing `checked_in`, call `free` and don't increment `checked_in`.) And what's the deal with `checked_in`? -- it's perfectly duplicates `pool_.size()`. Why track the same thing twice? – David Schwartz Apr 16 '13 at 03:52
  • Thanks for that. Its always nice to find out that its not an obscure corner of the C++ standard tripping you up. – Carl Apr 16 '13 at 04:05

2 Answers2

0

You're creating a memory pool. This pool implementation will grow as needed, but will never return memory to the OS. That's normally a bug, but not when creating your own method of allocating memory- as long as this pool exists for the life of your program. You'll leak when the program exits, but you can likely live with that. Basically you're overriding how new/malloc usually work and doing memory completely on your own.

Gabe Sechan
  • 90,003
  • 9
  • 87
  • 127
0

The missing 'free' means that you cannot embed this in some larger application, start it up, shut it down, and end up back where you started. This is fine if you control the entire application, and not fine if this code has to, in fact, be embeddable. To make that work, you would need some entrypoint that walks the vector calling free.

It never leaks in the conventional sense, since each malloc'ed chunk is stored in the vector by operator delete for re-use, and the operator delete complains if it sees too many items in the vector.

bmargulies
  • 97,814
  • 39
  • 186
  • 310
  • Yeah, it was unit testing some legacy code that used this internally that led me to uncover this bug. – Carl Apr 16 '13 at 03:57