Skip to content

Commit

Permalink
Fix TODOs to fix memory leaks
Browse files Browse the repository at this point in the history 
Fixes: #333
  • Loading branch information
Gabriel Schulhof committed Sep 18, 2018
1 parent b6e2d92 commit 3aac9eb
Show file tree
Hide file tree
Showing 7 changed files with 402 additions and 106 deletions.
308 changes: 225 additions & 83 deletions napi-inl.h
View file

Large diffs are not rendered by default.

55 changes: 41 additions & 14 deletions napi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1287,66 +1287,86 @@ namespace Napi {
class PropertyDescriptor {
public:
template <typename Getter>
static PropertyDescriptor Accessor(const char* utf8name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
const char* utf8name,
Getter getter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter>
static PropertyDescriptor Accessor(const std::string& utf8name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
const std::string& utf8name,
Getter getter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter>
static PropertyDescriptor Accessor(napi_value name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
napi_value name,
Getter getter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter>
static PropertyDescriptor Accessor(Name name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
Name name,
Getter getter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter, typename Setter>
static PropertyDescriptor Accessor(const char* utf8name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
const char* utf8name,
Getter getter,
Setter setter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter, typename Setter>
static PropertyDescriptor Accessor(const std::string& utf8name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
const std::string& utf8name,
Getter getter,
Setter setter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter, typename Setter>
static PropertyDescriptor Accessor(napi_value name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
napi_value name,
Getter getter,
Setter setter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Getter, typename Setter>
static PropertyDescriptor Accessor(Name name,
static PropertyDescriptor Accessor(Napi::Env env,
Napi::Object destination,
Name name,
Getter getter,
Setter setter,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Callable>
static PropertyDescriptor Function(const char* utf8name,
static PropertyDescriptor Function(Napi::Env env,
const char* utf8name,
Callable cb,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Callable>
static PropertyDescriptor Function(const std::string& utf8name,
static PropertyDescriptor Function(Napi::Env env,
const std::string& utf8name,
Callable cb,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Callable>
static PropertyDescriptor Function(napi_value name,
static PropertyDescriptor Function(Napi::Env env,
napi_value name,
Callable cb,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
template <typename Callable>
static PropertyDescriptor Function(Name name,
static PropertyDescriptor Function(Napi::Env env,
Name name,
Callable cb,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
Expand Down Expand Up @@ -1441,11 +1461,13 @@ namespace Napi {
const char* utf8name,
const std::vector<PropertyDescriptor>& properties,
void* data = nullptr);
static PropertyDescriptor StaticMethod(const char* utf8name,
static PropertyDescriptor StaticMethod(Napi::Env env,
const char* utf8name,
StaticVoidMethodCallback method,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
static PropertyDescriptor StaticMethod(const char* utf8name,
static PropertyDescriptor StaticMethod(Napi::Env env,
const char* utf8name,
StaticMethodCallback method,
napi_property_attributes attributes = napi_default,
void* data = nullptr);
Expand Down Expand Up @@ -1517,6 +1539,11 @@ namespace Napi {
static napi_value InstanceGetterCallbackWrapper(napi_env env, napi_callback_info info);
static napi_value InstanceSetterCallbackWrapper(napi_env env, napi_callback_info info);
static void FinalizeCallback(napi_env env, void* data, void* hint);
static Function DefineClass(napi_env env,
const char* utf8name,
size_t count,
const napi_property_descriptor* desc,
void* data);

template <typename TCallback>
struct MethodCallbackData {
Expand Down
2 changes: 2 additions & 0 deletions test/binding.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Object InitTypedArray(Env env);
Object InitObjectWrap(Env env);
Object InitObjectReference(Env env);
Object InitVersionManagement(Env env);
Object InitThunkingManual(Env env);

Object Init(Env env, Object exports) {
exports.Set("arraybuffer", InitArrayBuffer(env));
Expand All @@ -47,6 +48,7 @@ Object Init(Env env, Object exports) {
exports.Set("objectwrap", InitObjectWrap(env));
exports.Set("objectreference", InitObjectReference(env));
exports.Set("version_management", InitVersionManagement(env));
exports.Set("thunking_manual", InitThunkingManual(env));
return exports;
}

Expand Down
1 change: 1 addition & 0 deletions test/binding.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
'objectwrap.cc',
'objectreference.cc',
'version_management.cc'
'thunking_manual.cc',
],
'include_dirs': ["<!@(node -p \"require('../').include\")"],
'dependencies': ["<!(node -p \"require('../').gyp\")"],
Expand Down
18 changes: 9 additions & 9 deletions test/object/object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ void DefineProperties(const CallbackInfo& info) {

if (nameType.Utf8Value() == "literal") {
obj.DefineProperties({
PropertyDescriptor::Accessor("readonlyAccessor", TestGetter),
PropertyDescriptor::Accessor("readwriteAccessor", TestGetter, TestSetter),
PropertyDescriptor::Accessor(info.Env(), obj, "readonlyAccessor", TestGetter),
PropertyDescriptor::Accessor(info.Env(), obj, "readwriteAccessor", TestGetter, TestSetter),
PropertyDescriptor::Value("readonlyValue", trueValue),
PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable),
PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable),
PropertyDescriptor::Value("configurableValue", trueValue, napi_configurable),
PropertyDescriptor::Function("function", TestFunction),
PropertyDescriptor::Function(info.Env(), "function", TestFunction),
});
} else if (nameType.Utf8Value() == "string") {
// VS2013 has lifetime issues when passing temporary objects into the constructor of another
Expand All @@ -82,19 +82,19 @@ void DefineProperties(const CallbackInfo& info) {
std::string str7("function");

obj.DefineProperties({
PropertyDescriptor::Accessor(str1, TestGetter),
PropertyDescriptor::Accessor(str2, TestGetter, TestSetter),
PropertyDescriptor::Accessor(info.Env(), obj, str1, TestGetter),
PropertyDescriptor::Accessor(info.Env(), obj, str2, TestGetter, TestSetter),
PropertyDescriptor::Value(str3, trueValue),
PropertyDescriptor::Value(str4, trueValue, napi_writable),
PropertyDescriptor::Value(str5, trueValue, napi_enumerable),
PropertyDescriptor::Value(str6, trueValue, napi_configurable),
PropertyDescriptor::Function(str7, TestFunction),
PropertyDescriptor::Function(info.Env(), str7, TestFunction),
});
} else if (nameType.Utf8Value() == "value") {
obj.DefineProperties({
PropertyDescriptor::Accessor(
PropertyDescriptor::Accessor(info.Env(), obj,
Napi::String::New(info.Env(), "readonlyAccessor"), TestGetter),
PropertyDescriptor::Accessor(
PropertyDescriptor::Accessor(info.Env(), obj,
Napi::String::New(info.Env(), "readwriteAccessor"), TestGetter, TestSetter),
PropertyDescriptor::Value(
Napi::String::New(info.Env(), "readonlyValue"), trueValue),
Expand All @@ -104,7 +104,7 @@ void DefineProperties(const CallbackInfo& info) {
Napi::String::New(info.Env(), "enumerableValue"), trueValue, napi_enumerable),
PropertyDescriptor::Value(
Napi::String::New(info.Env(), "configurableValue"), trueValue, napi_configurable),
PropertyDescriptor::Function(
PropertyDescriptor::Function(info.Env(),
Napi::String::New(info.Env(), "function"), TestFunction),
});
}
Expand Down
106 changes: 106 additions & 0 deletions test/thunking_manual.cc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
#include <napi.h>

static Napi::Value TestMethod(const Napi::CallbackInfo& /*info*/) {
return Napi::Value();
}

static Napi::Value TestGetter(const Napi::CallbackInfo& /*info*/) {
return Napi::Value();
}

static void TestSetter(const Napi::CallbackInfo& /*info*/) {
}

class TestClass : public Napi::ObjectWrap<TestClass> {
public:
TestClass(const Napi::CallbackInfo& info):
ObjectWrap<TestClass>(info) {
}
static Napi::Value TestClassStaticMethod(const Napi::CallbackInfo& info) {
return Napi::Number::New(info.Env(), 42);
}

static void TestClassStaticVoidMethod(const Napi::CallbackInfo& /*info*/) {
}

Napi::Value TestClassInstanceMethod(const Napi::CallbackInfo& info) {
return Napi::Number::New(info.Env(), 42);
}

void TestClassInstanceVoidMethod(const Napi::CallbackInfo& /*info*/) {
}

Napi::Value TestClassInstanceGetter(const Napi::CallbackInfo& info) {
return Napi::Number::New(info.Env(), 42);
}

void TestClassInstanceSetter(const Napi::CallbackInfo& /*info*/,
const Napi::Value& /*new_value*/) {
}

static Napi::Function NewClass(Napi::Env env) {
return DefineClass(env, "TestClass", {
StaticMethod(env, "staticMethod", TestClassStaticMethod),
StaticMethod(env, "staticVoidMethod", TestClassStaticVoidMethod),
InstanceMethod("instanceMethod", &TestClass::TestClassInstanceMethod),
InstanceMethod("instanceVoidMethod",
&TestClass::TestClassInstanceVoidMethod),
InstanceMethod(Napi::Symbol::New(env, "instanceMethod"),
&TestClass::TestClassInstanceMethod),
InstanceMethod(Napi::Symbol::New(env, "instanceVoidMethod"),
&TestClass::TestClassInstanceVoidMethod),
InstanceAccessor("instanceAccessor",
&TestClass::TestClassInstanceGetter,
&TestClass::TestClassInstanceSetter)
});
}
};

static Napi::Value CreateTestObject(const Napi::CallbackInfo& info) {
Napi::Object item = Napi::Object::New(info.Env());
item["testMethod"] =
Napi::Function::New(info.Env(), TestMethod, "testMethod");

item.DefineProperties({
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
"accessor_1",
TestGetter),
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
std::string("accessor_1_std_string"),
TestGetter),
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
Napi::String::New(info.Env(),
"accessor_1_js_string"),
TestGetter),
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
"accessor_2",
TestGetter,
TestSetter),
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
std::string("accessor_2_std_string"),
TestGetter,
TestSetter),
Napi::PropertyDescriptor::Accessor(info.Env(),
item,
Napi::String::New(info.Env(),
"accessor_2_js_string"),
TestGetter,
TestSetter),
Napi::PropertyDescriptor::Value("TestClass",
TestClass::NewClass(info.Env())),
});

return item;
}

Napi::Object InitThunkingManual(Napi::Env env) {
Napi::Object exports = Napi::Object::New(env);
exports["createTestObject"] =
Napi::Function::New(env, CreateTestObject, "createTestObject");
return exports;
}
18 changes: 18 additions & 0 deletions test/thunking_manual.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Flags: --expose-gc
'use strict';
const buildType = process.config.target_defaults.default_configuration;
const assert = require('assert');

test(require(`./build/${buildType}/binding.node`));
test(require(`./build/${buildType}/binding_noexcept.node`));

function test(binding) {
console.log("Thunking: Performing initial GC");
global.gc();
console.log("Thunking: Creating test object");
let object = binding.thunking_manual.createTestObject();
object = null;
console.log("Thunking: About to GC\n--------");
global.gc();
console.log("--------\nThunking: GC complete");
}

0 comments on commit 3aac9eb

Please sign in to comment.