Skip to content

Commit acbdad9

Browse files
author
Gabriel Schulhof
committed
Fix TODOs to fix memory leaks
Fixes: nodejs#333
1 parent 622ffae commit acbdad9

File tree

7 files changed

+399
-106
lines changed

7 files changed

+399
-106
lines changed

‎napi-inl.h

+222-83
Large diffs are not rendered by default.

‎napi.h

+41-14
Original file line numberDiff line numberDiff line change
@@ -1236,66 +1236,86 @@ namespace Napi {
12361236
class PropertyDescriptor {
12371237
public:
12381238
template <typename Getter>
1239-
static PropertyDescriptor Accessor(const char* utf8name,
1239+
static PropertyDescriptor Accessor(Napi::Env env,
1240+
Napi::Object destination,
1241+
const char* utf8name,
12401242
Getter getter,
12411243
napi_property_attributes attributes = napi_default,
12421244
void* data = nullptr);
12431245
template <typename Getter>
1244-
static PropertyDescriptor Accessor(const std::string& utf8name,
1246+
static PropertyDescriptor Accessor(Napi::Env env,
1247+
Napi::Object destination,
1248+
const std::string& utf8name,
12451249
Getter getter,
12461250
napi_property_attributes attributes = napi_default,
12471251
void* data = nullptr);
12481252
template <typename Getter>
1249-
static PropertyDescriptor Accessor(napi_value name,
1253+
static PropertyDescriptor Accessor(Napi::Env env,
1254+
Napi::Object destination,
1255+
napi_value name,
12501256
Getter getter,
12511257
napi_property_attributes attributes = napi_default,
12521258
void* data = nullptr);
12531259
template <typename Getter>
1254-
static PropertyDescriptor Accessor(Name name,
1260+
static PropertyDescriptor Accessor(Napi::Env env,
1261+
Napi::Object destination,
1262+
Name name,
12551263
Getter getter,
12561264
napi_property_attributes attributes = napi_default,
12571265
void* data = nullptr);
12581266
template <typename Getter, typename Setter>
1259-
static PropertyDescriptor Accessor(const char* utf8name,
1267+
static PropertyDescriptor Accessor(Napi::Env env,
1268+
Napi::Object destination,
1269+
const char* utf8name,
12601270
Getter getter,
12611271
Setter setter,
12621272
napi_property_attributes attributes = napi_default,
12631273
void* data = nullptr);
12641274
template <typename Getter, typename Setter>
1265-
static PropertyDescriptor Accessor(const std::string& utf8name,
1275+
static PropertyDescriptor Accessor(Napi::Env env,
1276+
Napi::Object destination,
1277+
const std::string& utf8name,
12661278
Getter getter,
12671279
Setter setter,
12681280
napi_property_attributes attributes = napi_default,
12691281
void* data = nullptr);
12701282
template <typename Getter, typename Setter>
1271-
static PropertyDescriptor Accessor(napi_value name,
1283+
static PropertyDescriptor Accessor(Napi::Env env,
1284+
Napi::Object destination,
1285+
napi_value name,
12721286
Getter getter,
12731287
Setter setter,
12741288
napi_property_attributes attributes = napi_default,
12751289
void* data = nullptr);
12761290
template <typename Getter, typename Setter>
1277-
static PropertyDescriptor Accessor(Name name,
1291+
static PropertyDescriptor Accessor(Napi::Env env,
1292+
Napi::Object destination,
1293+
Name name,
12781294
Getter getter,
12791295
Setter setter,
12801296
napi_property_attributes attributes = napi_default,
12811297
void* data = nullptr);
12821298
template <typename Callable>
1283-
static PropertyDescriptor Function(const char* utf8name,
1299+
static PropertyDescriptor Function(Napi::Env env,
1300+
const char* utf8name,
12841301
Callable cb,
12851302
napi_property_attributes attributes = napi_default,
12861303
void* data = nullptr);
12871304
template <typename Callable>
1288-
static PropertyDescriptor Function(const std::string& utf8name,
1305+
static PropertyDescriptor Function(Napi::Env env,
1306+
const std::string& utf8name,
12891307
Callable cb,
12901308
napi_property_attributes attributes = napi_default,
12911309
void* data = nullptr);
12921310
template <typename Callable>
1293-
static PropertyDescriptor Function(napi_value name,
1311+
static PropertyDescriptor Function(Napi::Env env,
1312+
napi_value name,
12941313
Callable cb,
12951314
napi_property_attributes attributes = napi_default,
12961315
void* data = nullptr);
12971316
template <typename Callable>
1298-
static PropertyDescriptor Function(Name name,
1317+
static PropertyDescriptor Function(Napi::Env env,
1318+
Name name,
12991319
Callable cb,
13001320
napi_property_attributes attributes = napi_default,
13011321
void* data = nullptr);
@@ -1390,11 +1410,13 @@ namespace Napi {
13901410
const char* utf8name,
13911411
const std::vector<PropertyDescriptor>& properties,
13921412
void* data = nullptr);
1393-
static PropertyDescriptor StaticMethod(const char* utf8name,
1413+
static PropertyDescriptor StaticMethod(Napi::Env env,
1414+
const char* utf8name,
13941415
StaticVoidMethodCallback method,
13951416
napi_property_attributes attributes = napi_default,
13961417
void* data = nullptr);
1397-
static PropertyDescriptor StaticMethod(const char* utf8name,
1418+
static PropertyDescriptor StaticMethod(Napi::Env env,
1419+
const char* utf8name,
13981420
StaticMethodCallback method,
13991421
napi_property_attributes attributes = napi_default,
14001422
void* data = nullptr);
@@ -1442,6 +1464,11 @@ namespace Napi {
14421464
static napi_value InstanceGetterCallbackWrapper(napi_env env, napi_callback_info info);
14431465
static napi_value InstanceSetterCallbackWrapper(napi_env env, napi_callback_info info);
14441466
static void FinalizeCallback(napi_env env, void* data, void* hint);
1467+
static Function DefineClass(napi_env env,
1468+
const char* utf8name,
1469+
size_t count,
1470+
const napi_property_descriptor* desc,
1471+
void* data);
14451472

14461473
template <typename TCallback>
14471474
struct MethodCallbackData {

‎test/binding.cc

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ Object InitPromise(Env env);
2020
Object InitTypedArray(Env env);
2121
Object InitObjectWrap(Env env);
2222
Object InitObjectReference(Env env);
23+
Object InitThunkingManual(Env env);
2324

2425
Object Init(Env env, Object exports) {
2526
exports.Set("arraybuffer", InitArrayBuffer(env));
@@ -41,6 +42,7 @@ Object Init(Env env, Object exports) {
4142
exports.Set("typedarray", InitTypedArray(env));
4243
exports.Set("objectwrap", InitObjectWrap(env));
4344
exports.Set("objectreference", InitObjectReference(env));
45+
exports.Set("thunking_manual", InitThunkingManual(env));
4446
return exports;
4547
}
4648

‎test/binding.gyp

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
'typedarray.cc',
2626
'objectwrap.cc',
2727
'objectreference.cc',
28+
'thunking_manual.cc',
2829
],
2930
'include_dirs': ["<!@(node -p \"require('../').include\")"],
3031
'dependencies': ["<!(node -p \"require('../').gyp\")"],

‎test/object/object.cc

+9-9
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@ void DefineProperties(const CallbackInfo& info) {
6060

6161
if (nameType.Utf8Value() == "literal") {
6262
obj.DefineProperties({
63-
PropertyDescriptor::Accessor("readonlyAccessor", TestGetter),
64-
PropertyDescriptor::Accessor("readwriteAccessor", TestGetter, TestSetter),
63+
PropertyDescriptor::Accessor(info.Env(), obj, "readonlyAccessor", TestGetter),
64+
PropertyDescriptor::Accessor(info.Env(), obj, "readwriteAccessor", TestGetter, TestSetter),
6565
PropertyDescriptor::Value("readonlyValue", trueValue),
6666
PropertyDescriptor::Value("readwriteValue", trueValue, napi_writable),
6767
PropertyDescriptor::Value("enumerableValue", trueValue, napi_enumerable),
6868
PropertyDescriptor::Value("configurableValue", trueValue, napi_configurable),
69-
PropertyDescriptor::Function("function", TestFunction),
69+
PropertyDescriptor::Function(info.Env(), "function", TestFunction),
7070
});
7171
} else if (nameType.Utf8Value() == "string") {
7272
// VS2013 has lifetime issues when passing temporary objects into the constructor of another
@@ -82,19 +82,19 @@ void DefineProperties(const CallbackInfo& info) {
8282
std::string str7("function");
8383

8484
obj.DefineProperties({
85-
PropertyDescriptor::Accessor(str1, TestGetter),
86-
PropertyDescriptor::Accessor(str2, TestGetter, TestSetter),
85+
PropertyDescriptor::Accessor(info.Env(), obj, str1, TestGetter),
86+
PropertyDescriptor::Accessor(info.Env(), obj, str2, TestGetter, TestSetter),
8787
PropertyDescriptor::Value(str3, trueValue),
8888
PropertyDescriptor::Value(str4, trueValue, napi_writable),
8989
PropertyDescriptor::Value(str5, trueValue, napi_enumerable),
9090
PropertyDescriptor::Value(str6, trueValue, napi_configurable),
91-
PropertyDescriptor::Function(str7, TestFunction),
91+
PropertyDescriptor::Function(info.Env(), str7, TestFunction),
9292
});
9393
} else if (nameType.Utf8Value() == "value") {
9494
obj.DefineProperties({
95-
PropertyDescriptor::Accessor(
95+
PropertyDescriptor::Accessor(info.Env(), obj,
9696
Napi::String::New(info.Env(), "readonlyAccessor"), TestGetter),
97-
PropertyDescriptor::Accessor(
97+
PropertyDescriptor::Accessor(info.Env(), obj,
9898
Napi::String::New(info.Env(), "readwriteAccessor"), TestGetter, TestSetter),
9999
PropertyDescriptor::Value(
100100
Napi::String::New(info.Env(), "readonlyValue"), trueValue),
@@ -104,7 +104,7 @@ void DefineProperties(const CallbackInfo& info) {
104104
Napi::String::New(info.Env(), "enumerableValue"), trueValue, napi_enumerable),
105105
PropertyDescriptor::Value(
106106
Napi::String::New(info.Env(), "configurableValue"), trueValue, napi_configurable),
107-
PropertyDescriptor::Function(
107+
PropertyDescriptor::Function(info.Env(),
108108
Napi::String::New(info.Env(), "function"), TestFunction),
109109
});
110110
}

‎test/thunking_manual.cc

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#include <napi.h>
2+
3+
static Napi::Value TestMethod(const Napi::CallbackInfo& /*info*/) {
4+
return Napi::Value();
5+
}
6+
7+
static Napi::Value TestGetter(const Napi::CallbackInfo& /*info*/) {
8+
return Napi::Value();
9+
}
10+
11+
static void TestSetter(const Napi::CallbackInfo& /*info*/) {
12+
}
13+
14+
class TestClass : public Napi::ObjectWrap<TestClass> {
15+
public:
16+
TestClass(const Napi::CallbackInfo& info):
17+
ObjectWrap<TestClass>(info) {
18+
}
19+
static Napi::Value TestClassStaticMethod(const Napi::CallbackInfo& info) {
20+
return Napi::Number::New(info.Env(), 42);
21+
}
22+
23+
static void TestClassStaticVoidMethod(const Napi::CallbackInfo& /*info*/) {
24+
}
25+
26+
Napi::Value TestClassInstanceMethod(const Napi::CallbackInfo& info) {
27+
return Napi::Number::New(info.Env(), 42);
28+
}
29+
30+
void TestClassInstanceVoidMethod(const Napi::CallbackInfo& /*info*/) {
31+
}
32+
33+
Napi::Value TestClassInstanceGetter(const Napi::CallbackInfo& info) {
34+
return Napi::Number::New(info.Env(), 42);
35+
}
36+
37+
void TestClassInstanceSetter(const Napi::CallbackInfo& /*info*/,
38+
const Napi::Value& /*new_value*/) {
39+
}
40+
41+
static Napi::Function NewClass(Napi::Env env) {
42+
return DefineClass(env, "TestClass", {
43+
StaticMethod(env, "staticMethod", TestClassStaticMethod),
44+
StaticMethod(env, "staticVoidMethod", TestClassStaticVoidMethod),
45+
InstanceMethod("instanceMethod", &TestClass::TestClassInstanceMethod),
46+
InstanceMethod("instanceVoidMethod",
47+
&TestClass::TestClassInstanceVoidMethod),
48+
InstanceMethod(Napi::Symbol::New(env, "instanceMethod"),
49+
&TestClass::TestClassInstanceMethod),
50+
InstanceMethod(Napi::Symbol::New(env, "instanceVoidMethod"),
51+
&TestClass::TestClassInstanceVoidMethod),
52+
InstanceAccessor("instanceAccessor",
53+
&TestClass::TestClassInstanceGetter,
54+
&TestClass::TestClassInstanceSetter)
55+
});
56+
}
57+
};
58+
59+
static Napi::Value CreateTestObject(const Napi::CallbackInfo& info) {
60+
Napi::Object item = Napi::Object::New(info.Env());
61+
item["testMethod"] =
62+
Napi::Function::New(info.Env(), TestMethod, "testMethod");
63+
64+
item.DefineProperties({
65+
Napi::PropertyDescriptor::Accessor(info.Env(),
66+
item,
67+
"accessor_1",
68+
TestGetter),
69+
Napi::PropertyDescriptor::Accessor(info.Env(),
70+
item,
71+
std::string("accessor_1_std_string"),
72+
TestGetter),
73+
Napi::PropertyDescriptor::Accessor(info.Env(),
74+
item,
75+
Napi::String::New(info.Env(),
76+
"accessor_1_js_string"),
77+
TestGetter),
78+
Napi::PropertyDescriptor::Accessor(info.Env(),
79+
item,
80+
"accessor_2",
81+
TestGetter,
82+
TestSetter),
83+
Napi::PropertyDescriptor::Accessor(info.Env(),
84+
item,
85+
std::string("accessor_2_std_string"),
86+
TestGetter,
87+
TestSetter),
88+
Napi::PropertyDescriptor::Accessor(info.Env(),
89+
item,
90+
Napi::String::New(info.Env(),
91+
"accessor_2_js_string"),
92+
TestGetter,
93+
TestSetter),
94+
Napi::PropertyDescriptor::Value("TestClass",
95+
TestClass::NewClass(info.Env())),
96+
});
97+
98+
return item;
99+
}
100+
101+
Napi::Object InitThunkingManual(Napi::Env env) {
102+
Napi::Object exports = Napi::Object::New(env);
103+
exports["createTestObject"] =
104+
Napi::Function::New(env, CreateTestObject, "createTestObject");
105+
return exports;
106+
}

‎test/thunking_manual.js

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Flags: --expose-gc
2+
'use strict';
3+
const buildType = process.config.target_defaults.default_configuration;
4+
const assert = require('assert');
5+
6+
test(require(`./build/${buildType}/binding.node`));
7+
test(require(`./build/${buildType}/binding_noexcept.node`));
8+
9+
function test(binding) {
10+
console.log("Thunking: Performing initial GC");
11+
global.gc();
12+
console.log("Thunking: Creating test object");
13+
let object = binding.thunking_manual.createTestObject();
14+
object = null;
15+
console.log("Thunking: About to GC\n--------");
16+
global.gc();
17+
console.log("--------\nThunking: GC complete");
18+
}

0 commit comments

Comments
 (0)