Skip to content

Commit 3aac9eb

Browse files
author
Gabriel Schulhof
committed
Fix TODOs to fix memory leaks
Fixes: #333
1 parent b6e2d92 commit 3aac9eb

File tree

7 files changed

+402
-106
lines changed

7 files changed

+402
-106
lines changed

‎napi-inl.h

Lines changed: 225 additions & 83 deletions
Large diffs are not rendered by default.

‎napi.h

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,66 +1287,86 @@ namespace Napi {
12871287
class PropertyDescriptor {
12881288
public:
12891289
template <typename Getter>
1290-
static PropertyDescriptor Accessor(const char* utf8name,
1290+
static PropertyDescriptor Accessor(Napi::Env env,
1291+
Napi::Object destination,
1292+
const char* utf8name,
12911293
Getter getter,
12921294
napi_property_attributes attributes = napi_default,
12931295
void* data = nullptr);
12941296
template <typename Getter>
1295-
static PropertyDescriptor Accessor(const std::string& utf8name,
1297+
static PropertyDescriptor Accessor(Napi::Env env,
1298+
Napi::Object destination,
1299+
const std::string& utf8name,
12961300
Getter getter,
12971301
napi_property_attributes attributes = napi_default,
12981302
void* data = nullptr);
12991303
template <typename Getter>
1300-
static PropertyDescriptor Accessor(napi_value name,
1304+
static PropertyDescriptor Accessor(Napi::Env env,
1305+
Napi::Object destination,
1306+
napi_value name,
13011307
Getter getter,
13021308
napi_property_attributes attributes = napi_default,
13031309
void* data = nullptr);
13041310
template <typename Getter>
1305-
static PropertyDescriptor Accessor(Name name,
1311+
static PropertyDescriptor Accessor(Napi::Env env,
1312+
Napi::Object destination,
1313+
Name name,
13061314
Getter getter,
13071315
napi_property_attributes attributes = napi_default,
13081316
void* data = nullptr);
13091317
template <typename Getter, typename Setter>
1310-
static PropertyDescriptor Accessor(const char* utf8name,
1318+
static PropertyDescriptor Accessor(Napi::Env env,
1319+
Napi::Object destination,
1320+
const char* utf8name,
13111321
Getter getter,
13121322
Setter setter,
13131323
napi_property_attributes attributes = napi_default,
13141324
void* data = nullptr);
13151325
template <typename Getter, typename Setter>
1316-
static PropertyDescriptor Accessor(const std::string& utf8name,
1326+
static PropertyDescriptor Accessor(Napi::Env env,
1327+
Napi::Object destination,
1328+
const std::string& utf8name,
13171329
Getter getter,
13181330
Setter setter,
13191331
napi_property_attributes attributes = napi_default,
13201332
void* data = nullptr);
13211333
template <typename Getter, typename Setter>
1322-
static PropertyDescriptor Accessor(napi_value name,
1334+
static PropertyDescriptor Accessor(Napi::Env env,
1335+
Napi::Object destination,
1336+
napi_value name,
13231337
Getter getter,
13241338
Setter setter,
13251339
napi_property_attributes attributes = napi_default,
13261340
void* data = nullptr);
13271341
template <typename Getter, typename Setter>
1328-
static PropertyDescriptor Accessor(Name name,
1342+
static PropertyDescriptor Accessor(Napi::Env env,
1343+
Napi::Object destination,
1344+
Name name,
13291345
Getter getter,
13301346
Setter setter,
13311347
napi_property_attributes attributes = napi_default,
13321348
void* data = nullptr);
13331349
template <typename Callable>
1334-
static PropertyDescriptor Function(const char* utf8name,
1350+
static PropertyDescriptor Function(Napi::Env env,
1351+
const char* utf8name,
13351352
Callable cb,
13361353
napi_property_attributes attributes = napi_default,
13371354
void* data = nullptr);
13381355
template <typename Callable>
1339-
static PropertyDescriptor Function(const std::string& utf8name,
1356+
static PropertyDescriptor Function(Napi::Env env,
1357+
const std::string& utf8name,
13401358
Callable cb,
13411359
napi_property_attributes attributes = napi_default,
13421360
void* data = nullptr);
13431361
template <typename Callable>
1344-
static PropertyDescriptor Function(napi_value name,
1362+
static PropertyDescriptor Function(Napi::Env env,
1363+
napi_value name,
13451364
Callable cb,
13461365
napi_property_attributes attributes = napi_default,
13471366
void* data = nullptr);
13481367
template <typename Callable>
1349-
static PropertyDescriptor Function(Name name,
1368+
static PropertyDescriptor Function(Napi::Env env,
1369+
Name name,
13501370
Callable cb,
13511371
napi_property_attributes attributes = napi_default,
13521372
void* data = nullptr);
@@ -1441,11 +1461,13 @@ namespace Napi {
14411461
const char* utf8name,
14421462
const std::vector<PropertyDescriptor>& properties,
14431463
void* data = nullptr);
1444-
static PropertyDescriptor StaticMethod(const char* utf8name,
1464+
static PropertyDescriptor StaticMethod(Napi::Env env,
1465+
const char* utf8name,
14451466
StaticVoidMethodCallback method,
14461467
napi_property_attributes attributes = napi_default,
14471468
void* data = nullptr);
1448-
static PropertyDescriptor StaticMethod(const char* utf8name,
1469+
static PropertyDescriptor StaticMethod(Napi::Env env,
1470+
const char* utf8name,
14491471
StaticMethodCallback method,
14501472
napi_property_attributes attributes = napi_default,
14511473
void* data = nullptr);
@@ -1517,6 +1539,11 @@ namespace Napi {
15171539
static napi_value InstanceGetterCallbackWrapper(napi_env env, napi_callback_info info);
15181540
static napi_value InstanceSetterCallbackWrapper(napi_env env, napi_callback_info info);
15191541
static void FinalizeCallback(napi_env env, void* data, void* hint);
1542+
static Function DefineClass(napi_env env,
1543+
const char* utf8name,
1544+
size_t count,
1545+
const napi_property_descriptor* desc,
1546+
void* data);
15201547

15211548
template <typename TCallback>
15221549
struct MethodCallbackData {

‎test/binding.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Object InitTypedArray(Env env);
2323
Object InitObjectWrap(Env env);
2424
Object InitObjectReference(Env env);
2525
Object InitVersionManagement(Env env);
26+
Object InitThunkingManual(Env env);
2627

2728
Object Init(Env env, Object exports) {
2829
exports.Set("arraybuffer", InitArrayBuffer(env));
@@ -47,6 +48,7 @@ Object Init(Env env, Object exports) {
4748
exports.Set("objectwrap", InitObjectWrap(env));
4849
exports.Set("objectreference", InitObjectReference(env));
4950
exports.Set("version_management", InitVersionManagement(env));
51+
exports.Set("thunking_manual", InitThunkingManual(env));
5052
return exports;
5153
}
5254

‎test/binding.gyp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
'objectwrap.cc',
2929
'objectreference.cc',
3030
'version_management.cc'
31+
'thunking_manual.cc',
3132
],
3233
'include_dirs': ["<!@(node -p \"require('../').include\")"],
3334
'dependencies': ["<!(node -p \"require('../').gyp\")"],

‎test/object/object.cc

Lines changed: 9 additions & 9 deletions
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

Lines changed: 106 additions & 0 deletions
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

Lines changed: 18 additions & 0 deletions
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)