shared_ptr not releasing custom malloc in a thread

Question

In my code I'm creating a shared_ptr inside a lambda in order to save a PNG file as a background task. Unfortunately, even though I have a custom deleter for the shared_ptr, it seems the bytes are not deallocated correctly.

The code I use to create the shared_ptr:

std::shared_ptr<GLubyte> buffer = std::shared_ptr<GLubyte>((GLubyte*) malloc(*dataLength), [](GLubyte* buffer) {
        free(buffer);
    });

And in order to save the file and finally deallocate it:

std::thread t([=, buffer = std::move(buffer)]() mutable {
        bool done = writePNGFileFromBuffer(path, buffer.get(), width, height);
        return done;
    });
t.detach();

I have tried to put buffer.reset() inside the lambda, but although the buffer is null memory is not deallocated. I also have tried to change the creator function to something like:

std::shared_ptr<GLubyte> buffer = std::shared_ptr<GLubyte>((GLubyte*) malloc(*dataLength), std::free);

But it also doesn't work. Now I have been using the lambda deleter because then I can try to put a breakpoint inside and check free is called, but memory is still not released.

Furthermore, I have verified that the release works if I put free(buffer.get()) inside the lambda, but to put it makes no sense to me, because I'm using shared_ptr in order to avoid stuff like this.

Can you help me to release this buffer? Thank you so much.

Answer 1

I wrote this little test harness to prove that the new/delete was being performed correctly.

Note the use of new / delete[] in the buffer constructor. Your use of malloc/free give the code a bad smell. Resorting to free(ptr.get()) is hiding some other logic problem that you haven't solved. If you leave this in the program it will bite you later.

proxy is acting as a replacement for a GLubyte that counts the number allocated and destroyed, so I can confirm with assert that every construction has a corresponding destruction.

#include <iostream>
#include <memory>
#include <thread>
#include <cassert>
#include <atomic>

using namespace std;

#define USE_PROXY 1

struct proxy
{
    proxy() {
        ++_count;
    }
    ~proxy() {
        --_count;
    }

    static std::atomic<size_t> _count;
};
std::atomic<size_t> proxy::_count = { 0 };

#if USE_PROXY
using GLubyte = proxy;
#else
//using GLubyte = uint8_t;
#endif

bool writePNGFileFromBuffer(const char* path, const GLubyte* bytes, int width, int height)
{

    return true;
}


auto main() -> int
{
    {
        int dataLength = 10000;

        auto buffer = std::shared_ptr<GLubyte>(new GLubyte[dataLength], 
                                               [](GLubyte* p) { delete[] p; });

        const char* path = "/tmp/foo";
        int width = 100, height = 100;

        std::thread t([=, buffer = std::move(buffer)]() mutable {
            bool done = writePNGFileFromBuffer(path, buffer.get(), width, height);
            return done;
        });
        t.join();
        assert(!buffer);
    }
    assert(proxy::_count == 0);
    return 0;
}

for further thought, you may have wondered how a failure case in the write.... function could be handled. At the moment the return value is being thrown away. There are a few ways we could deal with this - one is to supply a lambda that can be called when the write operation is complete. Another is to wrap the write operation into a std::async.

bool writeSharedPNGFileFromBuffer(const char* path, shared_ptr<const GLubyte> bytes, int width, int height)
{
    return writePNGFileFromBuffer(path, bytes.get(), width, height);
}


auto main() -> int
{
    {
        int dataLength = 100;

        auto buffer = std::shared_ptr<GLubyte>(new GLubyte[10000], [](GLubyte* p) { delete[] p; });

        const char* path = "/tmp/foo";
        int width = 100, height = 100;

        auto f = std::async(launch::async, writeSharedPNGFileFromBuffer, path, move(buffer), width, height);
        // f is a std::future - a handle to somewhere the result will eventually land.
        // perform main thread work here...
        // ... and get the return value when we're ready to deal with it
        auto written = f.get();
        cout << "written: " << written << endl;
        assert(!buffer);
    }
    assert(proxy::_count == 0);
    return 0;
}