pmed / v8pp

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

Memory leak with properties #140

Closed christophe-f8 closed 3 years ago

christophe-f8 commented 3 years ago

When I compile and run the example under VisualStudio2019/Win10 Visual Leak Detector detects one memory leak with properties. If I comment out the property definition and use only variables and functions, the memory leak disappear.

X_class.set("prop", v8pp::property(&X::get)); // memory leak on exit

I check with both read only and R/W properties and confirmed they both leak (altought, read only properties leak a few bytes less)

The relevant Visual Leak Detector call stack lines are the following:

WARNING: Visual Leak Detector detected memory leaks! ---------- Block 2742 at 0x000000009E41E9E0: 16 bytes ---------- Leak Hash: 0x550C63BA, Count: 1, Total 16 bytes Call Stack (TID 3596): ucrtbased.dll!malloc() d:\A01_work\6\s\src\vctools\crt\vcstartup\src\heap\new_scalar.cpp (35): V8Test.exe!operator new() + 0xA bytes [...]v8pp\function.hpp (51): V8Test.exe!v8pp::detail::externaldata<v8pp::property<int (cdecl X::*)(void)const ,int (cdecl X::)(void)const > >::set() + 0xA bytes [...]v8pp\function.hpp (104): V8Test.exe!v8pp::detail::set_externaldata<v8pp::property<int (__cdecl X::)(void)const ,int (_cdecl X::*)(void)const > >() + 0x1C bytes [...]v8pp\class.hpp (306): V8Test.exe!v8pp::class<X,v8pp::raw_ptr_traits>::set<int (cdecl X::*)(void)const ,int (cdecl X::*)(void)const >() + 0x10F bytes

I tried to checked the code but could not figure out the issue. For now I'll just not use properties but I sure hope this could be fixed, as everything else runs smoothly.

pmed commented 3 years ago

Hi,

There was a similar topic on gitter: https://gitter.im/pmed/v8pp?at=56345b6144f10a06616c870c

properties require to store 2 pointers to member functions. This at least twice more than the v8::External can store. So I use dynamic memory allocation in set_external_data and make such an External a weak reference. You are right and weak references are garbage collected sometimes on full GC.

So you could try a full GC run before the V8 finalisation in your test code, like in test_class.cpp:

    std::string const v8_flags = "--expose_gc";
    v8::V8::SetFlagsFromString(v8_flags.data(), (int)v8_flags.length());
    context.isolate()->RequestGarbageCollectionForTesting(v8::Isolate::GarbageCollectionType::kFullGarbageCollection);
christophe-f8 commented 3 years ago

Thank for the reply. I just tested this (literally copy and paste) and it worked perfectly ! Thank you !