pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
898 stars 120 forks source link

Memory leak in external_data<T>::set #65

Closed mrsharpoblunto closed 6 years ago

mrsharpoblunto commented 6 years ago

When running in visual studio with crt heap debugging enabled, I noticed that theres a small memory leak in external_data::set - specifically the allocation on line 53 never gets cleaned up when binding a property using v8pp::property

static v8::Local<v8::External> set(v8::Isolate* isolate, T&& data)
{
    external_data* value = new external_data; // <-- this never gets cleaned up
    try
    {
        new (value->storage()) T(std::forward<T>(data));
    }
    catch (...)
    {
        delete value;
        throw;
    }
pmed commented 6 years ago

Hi Glenn,

That's actually not a memory leak. This external_data value is stored in a weak v8::External value->pext_ handle below. And this weak handle should be deleted by the garbage collector at some time. But it usually hasn't happen before a program exit.

mrsharpoblunto commented 6 years ago

The memory is still allocated though after I dispose the Isolate - I thought that everything in the js heap should be cleaned up when the Isolate is disposed?

pmed commented 6 years ago

I'm not sure the memory is deallocated on an Isolate destroy. You can try to request garbage collection before the isolate destroy. I use v8Isolate::RequestGarbageCollectionForTesting() for this: https://github.com/pmed/v8pp/blob/master/test/test_class.cpp#L200-L203