From e304c42a0f7a80d5a3e7944b8b2f39f78323cc08 Mon Sep 17 00:00:00 2001 From: babygoat Date: Thu, 23 Nov 2017 19:44:20 +0800 Subject: [PATCH 01/45] test: add unhandled rejection guard Add an unhandled rejection function in addons-napi/test_promise/test.js. Also, add a rejection handler to catch the unhandled rejection after introducing the guard and test the reason code. PR-URL: https://github.com/nodejs/node/pull/17275 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- test/addons-napi/test_promise/test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js index b43ecd87363e44..6dc51b3fa558a2 100644 --- a/test/addons-napi/test_promise/test.js +++ b/test/addons-napi/test_promise/test.js @@ -7,6 +7,8 @@ const common = require('../../common'); const assert = require('assert'); const test_promise = require(`./build/${common.buildType}/test_promise`); +common.crashOnUnhandledRejection(); + // A resolution { const expected_result = 42; @@ -44,7 +46,14 @@ const test_promise = require(`./build/${common.buildType}/test_promise`); } assert.strictEqual(test_promise.isPromise(test_promise.createPromise()), true); -assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true); + +const rejectPromise = Promise.reject(-1); +const expected_reason = -1; +assert.strictEqual(test_promise.isPromise(rejectPromise), true); +rejectPromise.catch((reason) => { + assert.strictEqual(reason, expected_reason); +}); + assert.strictEqual(test_promise.isPromise(2.4), false); assert.strictEqual(test_promise.isPromise('I promise!'), false); assert.strictEqual(test_promise.isPromise(undefined), false); From 887dcb9a1150788a9c7472878d6c1ba530476352 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 12 Dec 2017 21:20:47 -0800 Subject: [PATCH 02/45] test: remove literals that obscure assert messages Remove string literals as messages to `assert.strictEqual()`. They can be misleading here (where perhaps the reason an assertino failed isn't that the deleter wasn't called but rather was called too many times. PR-URL: https://github.com/nodejs/node/pull/17642 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Jon Moss Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski Reviewed-By: Michael Dawson --- test/addons-napi/test_buffer/test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/addons-napi/test_buffer/test.js b/test/addons-napi/test_buffer/test.js index 713966775df18b..740b0474a79c60 100644 --- a/test/addons-napi/test_buffer/test.js +++ b/test/addons-napi/test_buffer/test.js @@ -9,14 +9,13 @@ assert.strictEqual(binding.newBuffer().toString(), binding.theText); assert.strictEqual(binding.newExternalBuffer().toString(), binding.theText); console.log('gc1'); global.gc(); -assert.strictEqual(binding.getDeleterCallCount(), 1, 'deleter was not called'); +assert.strictEqual(binding.getDeleterCallCount(), 1); assert.strictEqual(binding.copyBuffer().toString(), binding.theText); let buffer = binding.staticBuffer(); -assert.strictEqual(binding.bufferHasInstance(buffer), true, - 'buffer type checking fails'); +assert.strictEqual(binding.bufferHasInstance(buffer), true); assert.strictEqual(binding.bufferInfo(buffer), true); buffer = null; global.gc(); console.log('gc2'); -assert.strictEqual(binding.getDeleterCallCount(), 2, 'deleter was not called'); +assert.strictEqual(binding.getDeleterCallCount(), 2); From bba1442ffd1c9aaabc5cf910547dd8c61640a154 Mon Sep 17 00:00:00 2001 From: alnyan Date: Sun, 17 Dec 2017 12:22:46 +0200 Subject: [PATCH 03/45] n-api: fix memory leak in napi_async_destroy() PR-URL: https://github.com/nodejs/node/pull/17714 Reviewed-By: Alexey Orlenko Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Timothy Gu Reviewed-By: James M Snell --- src/node_api.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node_api.cc b/src/node_api.cc index f16ceefd5eb5b9..ac0b0959b599d6 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -2770,6 +2770,8 @@ napi_status napi_async_destroy(napi_env env, reinterpret_cast(async_context); node::EmitAsyncDestroy(isolate, *node_async_context); + delete node_async_context; + return napi_clear_last_error(env); } From bcbb7af663c42a58de58234c69b1fa8291e27d5e Mon Sep 17 00:00:00 2001 From: Nicholas Drane Date: Thu, 21 Dec 2017 16:48:41 -0600 Subject: [PATCH 04/45] test: remove ambiguous error messages from test_error assert.strictEqual accepts 3 arguments, the last of which allows for user-specified error message to be thrown when the assertion fails. Unfortunately, this error message is less helpful than the default when it is vague. This commit removes vague, user-specified error messages, instead relying on clearer, default error messages. PR-URL: https://github.com/nodejs/node/pull/17812 Reviewed-By: Colin Ihrig Reviewed-By: Jon Moss Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- test/addons-napi/test_error/test.js | 46 ++++++++--------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/test/addons-napi/test_error/test.js b/test/addons-napi/test_error/test.js index bc2b7a8db75d3e..d5c92cb4c3cd2e 100644 --- a/test/addons-napi/test_error/test.js +++ b/test/addons-napi/test_error/test.js @@ -83,56 +83,34 @@ common.expectsError( let error = test_error.createError(); assert.ok(error instanceof Error, 'expected error to be an instance of Error'); -assert.strictEqual(error.message, 'error', 'expected message to be "error"'); +assert.strictEqual(error.message, 'error'); error = test_error.createRangeError(); assert.ok(error instanceof RangeError, 'expected error to be an instance of RangeError'); -assert.strictEqual(error.message, - 'range error', - 'expected message to be "range error"'); +assert.strictEqual(error.message, 'range error'); error = test_error.createTypeError(); assert.ok(error instanceof TypeError, 'expected error to be an instance of TypeError'); -assert.strictEqual(error.message, - 'type error', - 'expected message to be "type error"'); +assert.strictEqual(error.message, 'type error'); error = test_error.createErrorCode(); assert.ok(error instanceof Error, 'expected error to be an instance of Error'); -assert.strictEqual(error.code, - 'ERR_TEST_CODE', - 'expected code to be "ERR_TEST_CODE"'); -assert.strictEqual(error.message, - 'Error [error]', - 'expected message to be "Error [error]"'); -assert.strictEqual(error.name, - 'Error [ERR_TEST_CODE]', - 'expected name to be "Error [ERR_TEST_CODE]"'); +assert.strictEqual(error.code, 'ERR_TEST_CODE'); +assert.strictEqual(error.message, 'Error [error]'); +assert.strictEqual(error.name, 'Error [ERR_TEST_CODE]'); error = test_error.createRangeErrorCode(); assert.ok(error instanceof RangeError, 'expected error to be an instance of RangeError'); -assert.strictEqual(error.message, - 'RangeError [range error]', - 'expected message to be "RangeError [range error]"'); -assert.strictEqual(error.code, - 'ERR_TEST_CODE', - 'expected code to be "ERR_TEST_CODE"'); -assert.strictEqual(error.name, - 'RangeError [ERR_TEST_CODE]', - 'expected name to be "RangeError[ERR_TEST_CODE]"'); +assert.strictEqual(error.message, 'RangeError [range error]'); +assert.strictEqual(error.code, 'ERR_TEST_CODE'); +assert.strictEqual(error.name, 'RangeError [ERR_TEST_CODE]'); error = test_error.createTypeErrorCode(); assert.ok(error instanceof TypeError, 'expected error to be an instance of TypeError'); -assert.strictEqual(error.message, - 'TypeError [type error]', - 'expected message to be "TypeError [type error]"'); -assert.strictEqual(error.code, - 'ERR_TEST_CODE', - 'expected code to be "ERR_TEST_CODE"'); -assert.strictEqual(error.name, - 'TypeError [ERR_TEST_CODE]', - 'expected name to be "TypeError[ERR_TEST_CODE]"'); +assert.strictEqual(error.message, 'TypeError [type error]'); +assert.strictEqual(error.code, 'ERR_TEST_CODE'); +assert.strictEqual(error.name, 'TypeError [ERR_TEST_CODE]'); From 6bf376935e52ae5ca2c2088c936336ffc35ef9ab Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 5 Jan 2018 12:19:20 -0500 Subject: [PATCH 05/45] doc: updates examples to use NULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Examples in the N-API doc used a mix of nullptr and NULL. We should be consistent and because N-API is a 'C' API I believe using NULL is better. This will avoid any potential confusion as to whether N-API can be used with plain C. PR-URL: https://github.com/nodejs/node/pull/18008 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen Reviewed-By: Franziska Hinkelmann Reviewed-By: Gireesh Punathil --- doc/api/n-api.md | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index f9c24fe4a0e2b1..52eb6dbe983cf6 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -905,9 +905,9 @@ napi_value Init(napi_env env, napi_value exports) { napi_status status; napi_property_descriptor desc = {"hello", Method, 0, 0, 0, napi_default, 0}; - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; status = napi_define_properties(env, exports, 1, &desc); - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; return exports; } ``` @@ -919,7 +919,7 @@ napi_value Init(napi_env env, napi_value exports) { napi_value method; napi_status status; status = napi_create_function(env, "exports", Method, NULL, &method)); - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; return method; } ``` @@ -932,21 +932,21 @@ For example, to define a class so that new instances can be created napi_value Init(napi_env env, napi_value exports) { napi_status status; napi_property_descriptor properties[] = { - { "value", nullptr, GetValue, SetValue, 0, napi_default, 0 }, + { "value", NULL, GetValue, SetValue, 0, napi_default, 0 }, DECLARE_NAPI_METHOD("plusOne", PlusOne), DECLARE_NAPI_METHOD("multiply", Multiply), }; napi_value cons; status = - napi_define_class(env, "MyObject", New, nullptr, 3, properties, &cons); - if (status != napi_ok) return nullptr; + napi_define_class(env, "MyObject", New, NULL, 3, properties, &cons); + if (status != napi_ok) return NULL; status = napi_create_reference(env, cons, 1, &constructor); - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; status = napi_set_named_property(env, exports, "MyObject", cons); - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; return exports; } @@ -2364,8 +2364,8 @@ if (status != napi_ok) return status; // Set the properties napi_property_descriptor descriptors[] = { - { "foo", nullptr, 0, 0, 0, fooValue, napi_default, 0 }, - { "bar", nullptr, 0, 0, 0, barValue, napi_default, 0 } + { "foo", NULL, 0, 0, 0, fooValue, napi_default, 0 }, + { "bar", NULL, 0, 0, 0, barValue, napi_default, 0 } } status = napi_define_properties(env, obj, @@ -2876,18 +2876,18 @@ object. A sample module might look as follows: ```C napi_value SayHello(napi_env env, napi_callback_info info) { printf("Hello\n"); - return nullptr; + return NULL; } napi_value Init(napi_env env, napi_value exports) { napi_status status; napi_value fn; - status = napi_create_function(env, nullptr, 0, SayHello, nullptr, &fn); - if (status != napi_ok) return nullptr; + status = napi_create_function(env, NULL, 0, SayHello, nullptr, &fn); + if (status != napi_ok) return NULL; status = napi_set_named_property(env, exports, "sayHello", fn); - if (status != napi_ok) return nullptr; + if (status != napi_ok) return NULL; return exports; } @@ -2952,7 +2952,7 @@ napi_status napi_get_new_target(napi_env env, Returns `napi_ok` if the API succeeded. This API returns the `new.target` of the constructor call. If the current -callback is not a constructor call, the result is `nullptr`. +callback is not a constructor call, the result is `NULL`. ### *napi_new_instance* @@ -3389,7 +3389,7 @@ napi_status napi_async_init(napi_env env, Returns `napi_ok` if the API succeeded. -### *napi_async_destroy** +### napi_async_destroy @@ -3403,7 +3403,7 @@ napi_status napi_async_destroy(napi_env env, Returns `napi_ok` if the API succeeded. -### *napi_make_callback* +### napi_make_callback @@ -1038,7 +1038,7 @@ JavaScript arrays are described in [Section 22.1](https://tc39.github.io/ecma262/#sec-array-objects) of the ECMAScript Language Specification. -#### *napi_create_array_with_length* +#### napi_create_array_with_length @@ -1067,7 +1067,7 @@ JavaScript arrays are described in [Section 22.1](https://tc39.github.io/ecma262/#sec-array-objects) of the ECMAScript Language Specification. -#### *napi_create_arraybuffer* +#### napi_create_arraybuffer @@ -1099,7 +1099,7 @@ JavaScript ArrayBuffer objects are described in [Section 24.1](https://tc39.github.io/ecma262/#sec-arraybuffer-objects) of the ECMAScript Language Specification. -#### *napi_create_buffer* +#### napi_create_buffer @@ -1120,7 +1120,7 @@ Returns `napi_ok` if the API succeeded. This API allocates a `node::Buffer` object. While this is still a fully-supported data structure, in most cases using a TypedArray will suffice. -#### *napi_create_buffer_copy* +#### napi_create_buffer_copy @@ -1145,7 +1145,7 @@ This API allocates a `node::Buffer` object and initializes it with data copied from the passed-in buffer. While this is still a fully-supported data structure, in most cases using a TypedArray will suffice. -#### *napi_create_external* +#### napi_create_external @@ -1212,7 +1212,7 @@ JavaScript ArrayBuffers are described in [Section 24.1](https://tc39.github.io/ecma262/#sec-arraybuffer-objects) of the ECMAScript Language Specification. -#### *napi_create_external_buffer* +#### napi_create_external_buffer @@ -1243,7 +1243,7 @@ structure, in most cases using a TypedArray will suffice. *Note*: For Node.js >=4 `Buffers` are Uint8Arrays. -#### *napi_create_function* +#### napi_create_function @@ -1276,7 +1276,7 @@ JavaScript Functions are described in [Section 19.2](https://tc39.github.io/ecma262/#sec-function-objects) of the ECMAScript Language Specification. -#### *napi_create_object* +#### napi_create_object @@ -1296,7 +1296,7 @@ The JavaScript Object type is described in [Section 6.1.7](https://tc39.github.io/ecma262/#sec-object-type) of the ECMAScript Language Specification. -#### *napi_create_symbol* +#### napi_create_symbol @@ -1319,7 +1319,7 @@ The JavaScript Symbol type is described in [Section 19.4](https://tc39.github.io/ecma262/#sec-symbol-objects) of the ECMAScript Language Specification. -#### *napi_create_typedarray* +#### napi_create_typedarray @@ -1355,7 +1355,7 @@ JavaScript TypedArray Objects are described in of the ECMAScript Language Specification. -#### *napi_create_dataview* +#### napi_create_dataview @@ -1389,7 +1389,7 @@ JavaScript DataView Objects are described in [Section 24.3][] of the ECMAScript Language Specification. ### Functions to convert from C types to N-API -#### *napi_create_int32* +#### napi_create_int32 @@ -1410,7 +1410,7 @@ The JavaScript Number type is described in [Section 6.1.6](https://tc39.github.io/ecma262/#sec-ecmascript-language-types-number-type) of the ECMAScript Language Specification. -#### *napi_create_uint32* +#### napi_create_uint32 @@ -1431,7 +1431,7 @@ The JavaScript Number type is described in [Section 6.1.6](https://tc39.github.io/ecma262/#sec-ecmascript-language-types-number-type) of the ECMAScript Language Specification. -#### *napi_create_int64* +#### napi_create_int64 @@ -1458,7 +1458,7 @@ outside the range of [`Number.MAX_SAFE_INTEGER`](https://tc39.github.io/ecma262/#sec-number.max_safe_integer) (2^53 - 1) will lose precision. -#### *napi_create_double* +#### napi_create_double @@ -1479,7 +1479,7 @@ The JavaScript Number type is described in [Section 6.1.6](https://tc39.github.io/ecma262/#sec-ecmascript-language-types-number-type) of the ECMAScript Language Specification. -#### *napi_create_string_latin1* +#### napi_create_string_latin1 @@ -1504,7 +1504,7 @@ The JavaScript String type is described in [Section 6.1.4](https://tc39.github.io/ecma262/#sec-ecmascript-language-types-string-type) of the ECMAScript Language Specification. -#### *napi_create_string_utf16* +#### napi_create_string_utf16 @@ -1529,7 +1529,7 @@ The JavaScript String type is described in [Section 6.1.4](https://tc39.github.io/ecma262/#sec-ecmascript-language-types-string-type) of the ECMAScript Language Specification. -#### *napi_create_string_utf8* +#### napi_create_string_utf8 @@ -1555,7 +1555,7 @@ The JavaScript String type is described in of the ECMAScript Language Specification. ### Functions to convert from N-API to C types -#### *napi_get_array_length* +#### napi_get_array_length @@ -1578,7 +1578,7 @@ Array length is described in [Section 22.1.4.1](https://tc39.github.io/ecma262/#sec-properties-of-array-instances-length) of the ECMAScript Language Specification. -#### *napi_get_arraybuffer_info* +#### napi_get_arraybuffer_info @@ -1606,7 +1606,7 @@ which can be used to guarantee control over the lifetime of the ArrayBuffer. It's also safe to use the returned data buffer within the same callback as long as there are no calls to other APIs that might trigger a GC. -#### *napi_get_buffer_info* +#### napi_get_buffer_info @@ -1630,7 +1630,7 @@ and it's length. *Warning*: Use caution while using this API since the underlying data buffer's lifetime is not guaranteed if it's managed by the VM. -#### *napi_get_prototype* +#### napi_get_prototype @@ -1648,7 +1648,7 @@ not the same as the function's `prototype` property). Returns `napi_ok` if the API succeeded. -#### *napi_get_typedarray_info* +#### napi_get_typedarray_info @@ -1680,7 +1680,7 @@ is managed by the VM -#### *napi_get_dataview_info* +#### napi_get_dataview_info @@ -1708,7 +1708,7 @@ Returns `napi_ok` if the API succeeded. This API returns various properties of a DataView. -#### *napi_get_value_bool* +#### napi_get_value_bool @@ -1727,7 +1727,7 @@ passed in it returns `napi_boolean_expected`. This API returns the C boolean primitive equivalent of the given JavaScript Boolean. -#### *napi_get_value_double* +#### napi_get_value_double @@ -1749,7 +1749,7 @@ This API returns the C double primitive equivalent of the given JavaScript Number. -#### *napi_get_value_external* +#### napi_get_value_external @@ -1769,7 +1769,7 @@ passed in it returns `napi_invalid_arg`. This API retrieves the external data pointer that was previously passed to `napi_create_external()`. -#### *napi_get_value_int32* +#### napi_get_value_int32 @@ -1792,7 +1792,7 @@ of the given JavaScript Number. If the number exceeds the range of the bottom 32 bits. This can result in a large positive number becoming a negative number if the value is > 2^31 -1. -#### *napi_get_value_int64* +#### napi_get_value_int64 @@ -1812,7 +1812,7 @@ is passed in it returns `napi_number_expected`. This API returns the C int64 primitive equivalent of the given JavaScript Number -#### *napi_get_value_string_latin1* +#### napi_get_value_string_latin1 @@ -1839,7 +1839,7 @@ is passed in it returns `napi_string_expected`. This API returns the ISO-8859-1-encoded string corresponding the value passed in. -#### *napi_get_value_string_utf8* +#### napi_get_value_string_utf8 @@ -1865,7 +1865,7 @@ is passed in it returns `napi_string_expected`. This API returns the UTF8-encoded string corresponding the value passed in. -#### *napi_get_value_string_utf16* +#### napi_get_value_string_utf16 @@ -1891,7 +1891,7 @@ is passed in it returns `napi_string_expected`. This API returns the UTF16-encoded string corresponding the value passed in. -#### *napi_get_value_uint32* +#### napi_get_value_uint32 @@ -1913,7 +1913,7 @@ This API returns the C primitive equivalent of the given `napi_value` as a `uint32_t`. ### Functions to get global instances -#### *napi_get_boolean* +#### napi_get_boolean @@ -1931,7 +1931,7 @@ Returns `napi_ok` if the API succeeded. This API is used to return the JavaScript singleton object that is used to represent the given boolean value -#### *napi_get_global* +#### napi_get_global @@ -1946,7 +1946,7 @@ Returns `napi_ok` if the API succeeded. This API returns the global Object. -#### *napi_get_null* +#### napi_get_null @@ -1961,7 +1961,7 @@ Returns `napi_ok` if the API succeeded. This API returns the null Object. -#### *napi_get_undefined* +#### napi_get_undefined @@ -1989,7 +1989,7 @@ These APIs support doing one of the following: 2. Check the type of a JavaScript value 3. Check for equality between two JavaScript values -### *napi_coerce_to_bool* +### napi_coerce_to_bool @@ -2010,7 +2010,7 @@ This API implements the abstract operation ToBoolean as defined in of the ECMAScript Language Specification. This API can be re-entrant if getters are defined on the passed-in Object. -### *napi_coerce_to_number* +### napi_coerce_to_number @@ -2031,7 +2031,7 @@ This API implements the abstract operation ToNumber as defined in of the ECMAScript Language Specification. This API can be re-entrant if getters are defined on the passed-in Object. -### *napi_coerce_to_object* +### napi_coerce_to_object @@ -2052,7 +2052,7 @@ This API implements the abstract operation ToObject as defined in of the ECMAScript Language Specification. This API can be re-entrant if getters are defined on the passed-in Object. -### *napi_coerce_to_string* +### napi_coerce_to_string @@ -2073,7 +2073,7 @@ This API implements the abstract operation ToString as defined in of the ECMAScript Language Specification. This API can be re-entrant if getters are defined on the passed-in Object. -### *napi_typeof* +### napi_typeof @@ -2094,7 +2094,7 @@ the object as defined in [Section 12.5.5][] of the ECMAScript Language Specification. However, it has support for detecting an External value. If `value` has a type that is invalid, an error is returned. -### *napi_instanceof* +### napi_instanceof @@ -2119,7 +2119,7 @@ defined in [Section 12.10.4](https://tc39.github.io/ecma262/#sec-instanceofoperator) of the ECMAScript Language Specification. -### *napi_is_array* +### napi_is_array @@ -2137,7 +2137,7 @@ This API represents invoking the `IsArray` operation on the object as defined in [Section 7.2.2](https://tc39.github.io/ecma262/#sec-isarray) of the ECMAScript Language Specification. -### *napi_is_arraybuffer* +### napi_is_arraybuffer @@ -2153,7 +2153,7 @@ Returns `napi_ok` if the API succeeded. This API checks if the Object passsed in is an array buffer. -### *napi_is_buffer* +### napi_is_buffer @@ -2170,7 +2170,7 @@ Returns `napi_ok` if the API succeeded. This API checks if the Object passsed in is a buffer. -### *napi_is_error* +### napi_is_error @@ -2186,7 +2186,7 @@ Returns `napi_ok` if the API succeeded. This API checks if the Object passsed in is an Error. -### *napi_is_typedarray* +### napi_is_typedarray @@ -2204,7 +2204,7 @@ This API checks if the Object passsed in is a typed array. -### *napi_is_dataview* +### napi_is_dataview @@ -2221,7 +2221,7 @@ Returns `napi_ok` if the API succeeded. This API checks if the Object passed in is a DataView. -### *napi_strict_equals* +### napi_strict_equals @@ -2375,7 +2375,7 @@ if (status != napi_ok) return status; ``` ### Structures -#### *napi_property_attributes* +#### napi_property_attributes ```C typedef enum { napi_default = 0, @@ -2409,7 +2409,7 @@ a static property on a class as opposed to an instance property, which is the default. This is used only by [`napi_define_class`][]. It is ignored by `napi_define_properties`. -#### *napi_property_descriptor* +#### napi_property_descriptor ```C typedef struct { // One of utf8name or name should be NULL. @@ -2455,7 +2455,7 @@ this function is invoked. See [`napi_property_attributes`](#n_api_napi_property_attributes). ### Functions -#### *napi_get_property_names* +#### napi_get_property_names @@ -2476,7 +2476,7 @@ Returns `napi_ok` if the API succeeded. This API returns the array of propertys for the Object passed in -#### *napi_set_property* +#### napi_set_property @@ -2496,7 +2496,7 @@ Returns `napi_ok` if the API succeeded. This API set a property on the Object passed in. -#### *napi_get_property* +#### napi_get_property @@ -2517,7 +2517,7 @@ Returns `napi_ok` if the API succeeded. This API gets the requested property from the Object passed in. -#### *napi_has_property* +#### napi_has_property @@ -2538,7 +2538,7 @@ Returns `napi_ok` if the API succeeded. This API checks if the Object passed in has the named property. -#### *napi_delete_property* +#### napi_delete_property @@ -2560,7 +2560,7 @@ Returns `napi_ok` if the API succeeded. This API attempts to delete the `key` own property from `object`. -#### *napi_has_own_property* +#### napi_has_own_property @@ -2583,7 +2583,7 @@ be a string or a Symbol, or an error will be thrown. N-API will not perform any conversion between data types. -#### *napi_set_named_property* +#### napi_set_named_property @@ -2604,7 +2604,7 @@ Returns `napi_ok` if the API succeeded. This method is equivalent to calling [`napi_set_property`][] with a `napi_value` created from the string passed in as `utf8Name` -#### *napi_get_named_property* +#### napi_get_named_property @@ -2625,7 +2625,7 @@ Returns `napi_ok` if the API succeeded. This method is equivalent to calling [`napi_get_property`][] with a `napi_value` created from the string passed in as `utf8Name` -#### *napi_has_named_property* +#### napi_has_named_property @@ -2646,7 +2646,7 @@ Returns `napi_ok` if the API succeeded. This method is equivalent to calling [`napi_has_property`][] with a `napi_value` created from the string passed in as `utf8Name` -#### *napi_set_element* +#### napi_set_element @@ -2666,7 +2666,7 @@ Returns `napi_ok` if the API succeeded. This API sets and element on the Object passed in. -#### *napi_get_element* +#### napi_get_element @@ -2686,7 +2686,7 @@ Returns `napi_ok` if the API succeeded. This API gets the element at the requested index. -#### *napi_has_element* +#### napi_has_element @@ -2707,7 +2707,7 @@ Returns `napi_ok` if the API succeeded. This API returns if the Object passed in has an element at the requested index. -#### *napi_delete_element* +#### napi_delete_element @@ -2728,7 +2728,7 @@ Returns `napi_ok` if the API succeeded. This API attempts to delete the specified `index` from `object`. -#### *napi_define_properties* +#### napi_define_properties @@ -2771,7 +2771,7 @@ like a regular JavaScript function call, or as a constructor function. -### *napi_call_function* +### napi_call_function @@ -2837,7 +2837,7 @@ status = napi_get_value_int32(env, return_val, &result); if (status != napi_ok) return; ``` -### *napi_create_function* +### napi_create_function @@ -2905,7 +2905,7 @@ myaddon.sayHello(); `NAPI_MODULE` in the earlier snippet but the name of the target in `binding.gyp` responsible for creating the `.node` file. -### *napi_get_cb_info* +### napi_get_cb_info @@ -2935,7 +2935,7 @@ Returns `napi_ok` if the API succeeded. This method is used within a callback function to retrieve details about the call like the arguments and the `this` pointer from a given callback info. -### *napi_get_new_target* +### napi_get_new_target @@ -2954,7 +2954,7 @@ Returns `napi_ok` if the API succeeded. This API returns the `new.target` of the constructor call. If the current callback is not a constructor call, the result is `NULL`. -### *napi_new_instance* +### napi_new_instance @@ -3049,7 +3049,7 @@ if (is_instance) { The reference must be freed once it is no longer needed. -### *napi_define_class* +### napi_define_class @@ -3105,7 +3105,7 @@ case, to prevent the function value from being garbage-collected, create a persistent reference to it using [`napi_create_reference`][] and ensure the reference count is kept >= 1. -### *napi_wrap* +### napi_wrap @@ -3167,7 +3167,7 @@ native instance associated with it by virtue of a previous call to another native instance with the given object, call `napi_remove_wrap()` on it first. -### *napi_unwrap* +### napi_unwrap @@ -3192,7 +3192,7 @@ method or accessor, then the `this` argument to the callback is the wrapper object; the wrapped C++ instance that is the target of the call can be obtained then by calling `napi_unwrap()` on the wrapper object. -### *napi_remove_wrap* +### napi_remove_wrap From 111678bd0fc3261bf43f6f2c95ffe71b5744ca5f Mon Sep 17 00:00:00 2001 From: Jinho Bang Date: Mon, 8 Jan 2018 23:37:27 +0900 Subject: [PATCH 10/45] n-api: throw RangeError napi_create_typedarray() According to the ECMA spec, we should throw a RangeError in the following cases: - `(length * elementSize) + offset` > the size of the array passed in - `offset % elementSize` != `0` In the current implementation, this check was omitted. So, the following code will cause a crash. ``` napi_create_typedarray(env, napi_uint16_array, 2 /* length */, buffer, 1 /* byte_offset */, &output_array); ``` This change fixes the problem and write some related tests. Refs: https://tc39.github.io/ecma262/#sec-typedarray-buffer-byteoffset-length PR-URL: https://github.com/nodejs/node/pull/18037 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- doc/api/errors.md | 12 +++++ lib/internal/errors.js | 3 ++ src/node_api.cc | 51 +++++++++++++++---- test/addons-napi/test_typedarray/test.js | 18 +++++++ .../test_typedarray/test_typedarray.c | 28 ++++++++-- 5 files changed, 100 insertions(+), 12 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 973d523258be2a..c7264b083f2d98 100755 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -997,6 +997,18 @@ While using `N-API`, `Constructor.prototype` was not an object. While calling `napi_create_dataview()`, a given `offset` was outside the bounds of the dataview or `offset + length` was larger than a length of given `buffer`. + +### ERR_NAPI_INVALID_TYPEDARRAY_ALIGNMENT + +While calling `napi_create_typedarray()`, the provided `offset` was not a +multiple of the element size. + + +### ERR_NAPI_INVALID_TYPEDARRAY_LENGTH + +While calling `napi_create_typedarray()`, `(length * size_of_element) + +byte_offset` was larger than the length of given `buffer`. + ### ERR_NO_ICU diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4c8acbc5446f44..7dbcce28ce6d00 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -265,6 +265,9 @@ E('ERR_NAPI_CONS_PROTOTYPE_OBJECT', 'Constructor.prototype must be an object'); E('ERR_NAPI_INVALID_DATAVIEW_ARGS', 'byte_offset + byte_length should be less than or eqaul to the size in ' + 'bytes of the array passed in'); +E('ERR_NAPI_INVALID_TYPEDARRAY_ALIGNMENT', 'start offset of %s should be a ' + + 'multiple of %s'); +E('ERR_NAPI_INVALID_TYPEDARRAY_LENGTH', 'Invalid typed array length'); E('ERR_NO_CRYPTO', 'Node.js is not compiled with OpenSSL crypto support'); E('ERR_NO_ICU', '%s is not supported on Node.js compiled without ICU'); E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s'); diff --git a/src/node_api.cc b/src/node_api.cc index 2101fcba020d95..27ab6707de7a6f 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -142,6 +142,30 @@ struct napi_env__ { (!try_catch.HasCaught() ? napi_ok \ : napi_set_last_error((env), napi_pending_exception)) +#define THROW_RANGE_ERROR_IF_FALSE(env, condition, error, message) \ + do { \ + if (!(condition)) { \ + napi_throw_range_error((env), (error), (message)); \ + return napi_set_last_error((env), napi_generic_failure); \ + } \ + } while (0) + +#define CREATE_TYPED_ARRAY( \ + env, type, size_of_element, buffer, byte_offset, length, out) \ + do { \ + if ((size_of_element) > 1) { \ + THROW_RANGE_ERROR_IF_FALSE( \ + (env), (byte_offset) % (size_of_element) == 0, \ + "ERR_NAPI_INVALID_TYPEDARRAY_ALIGNMENT", \ + "start offset of "#type" should be a multiple of "#size_of_element); \ + } \ + THROW_RANGE_ERROR_IF_FALSE((env), (length) * (size_of_element) + \ + (byte_offset) <= buffer->ByteLength(), \ + "ERR_NAPI_INVALID_TYPEDARRAY_LENGTH", \ + "Invalid typed array length"); \ + (out) = v8::type::New((buffer), (byte_offset), (length)); \ + } while (0) + namespace { namespace v8impl { @@ -3063,31 +3087,40 @@ napi_status napi_create_typedarray(napi_env env, switch (type) { case napi_int8_array: - typedArray = v8::Int8Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Int8Array, 1, buffer, byte_offset, length, typedArray); break; case napi_uint8_array: - typedArray = v8::Uint8Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Uint8Array, 1, buffer, byte_offset, length, typedArray); break; case napi_uint8_clamped_array: - typedArray = v8::Uint8ClampedArray::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Uint8ClampedArray, 1, buffer, byte_offset, length, typedArray); break; case napi_int16_array: - typedArray = v8::Int16Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Int16Array, 2, buffer, byte_offset, length, typedArray); break; case napi_uint16_array: - typedArray = v8::Uint16Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Uint16Array, 2, buffer, byte_offset, length, typedArray); break; case napi_int32_array: - typedArray = v8::Int32Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Int32Array, 4, buffer, byte_offset, length, typedArray); break; case napi_uint32_array: - typedArray = v8::Uint32Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Uint32Array, 4, buffer, byte_offset, length, typedArray); break; case napi_float32_array: - typedArray = v8::Float32Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Float32Array, 4, buffer, byte_offset, length, typedArray); break; case napi_float64_array: - typedArray = v8::Float64Array::New(buffer, byte_offset, length); + CREATE_TYPED_ARRAY( + env, Float64Array, 8, buffer, byte_offset, length, typedArray); break; default: return napi_set_last_error(env, napi_invalid_arg); diff --git a/test/addons-napi/test_typedarray/test.js b/test/addons-napi/test_typedarray/test.js index 27ef054fe4635e..4a4e79ebe7bcdb 100644 --- a/test/addons-napi/test_typedarray/test.js +++ b/test/addons-napi/test_typedarray/test.js @@ -55,3 +55,21 @@ arrayTypes.forEach((currentType) => { assert.notStrictEqual(theArray, template); assert.strictEqual(theArray.buffer, buffer); }); + +arrayTypes.forEach((currentType) => { + const template = Reflect.construct(currentType, buffer); + assert.throws(() => { + test_typedarray.CreateTypedArray(template, buffer, 0, 136); + }, /Invalid typed array length/); +}); + +const nonByteArrayTypes = [ Int16Array, Uint16Array, Int32Array, Uint32Array, + Float32Array, Float64Array ]; +nonByteArrayTypes.forEach((currentType) => { + const template = Reflect.construct(currentType, buffer); + assert.throws(() => { + test_typedarray.CreateTypedArray(template, buffer, + currentType.BYTES_PER_ELEMENT + 1, 1); + console.log(`start of offset ${currentType}`); + }, /start offset of/); +}); diff --git a/test/addons-napi/test_typedarray/test_typedarray.c b/test/addons-napi/test_typedarray/test_typedarray.c index 0325faedd09f8e..82c7a5bf361d30 100644 --- a/test/addons-napi/test_typedarray/test_typedarray.c +++ b/test/addons-napi/test_typedarray/test_typedarray.c @@ -97,11 +97,11 @@ napi_value External(napi_env env, napi_callback_info info) { } napi_value CreateTypedArray(napi_env env, napi_callback_info info) { - size_t argc = 2; - napi_value args[2]; + size_t argc = 4; + napi_value args[4]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); - NAPI_ASSERT(env, argc == 2, "Wrong number of arguments"); + NAPI_ASSERT(env, argc == 2 || argc == 4, "Wrong number of arguments"); napi_value input_array = args[0]; napi_valuetype valuetype0; @@ -136,6 +136,28 @@ napi_value CreateTypedArray(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_get_typedarray_info( env, input_array, &type, &length, NULL, &in_array_buffer, &byte_offset)); + if (argc == 4) { + napi_valuetype valuetype2; + NAPI_CALL(env, napi_typeof(env, args[2], &valuetype2)); + + NAPI_ASSERT(env, valuetype2 == napi_number, + "Wrong type of arguments. Expects a number as third argument."); + + uint32_t uint32_length; + NAPI_CALL(env, napi_get_value_uint32(env, args[2], &uint32_length)); + length = uint32_length; + + napi_valuetype valuetype3; + NAPI_CALL(env, napi_typeof(env, args[3], &valuetype3)); + + NAPI_ASSERT(env, valuetype3 == napi_number, + "Wrong type of arguments. Expects a number as third argument."); + + uint32_t uint32_byte_offset; + NAPI_CALL(env, napi_get_value_uint32(env, args[3], &uint32_byte_offset)); + byte_offset = uint32_byte_offset; + } + napi_value output_array; NAPI_CALL(env, napi_create_typedarray( env, type, length, input_buffer, byte_offset, &output_array)); From 94618a5f66007c893a32d77a8251b2fbdcac26b3 Mon Sep 17 00:00:00 2001 From: furstenheim Date: Sun, 14 Jan 2018 19:01:51 +0100 Subject: [PATCH 11/45] test: fixed typos in napi test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18148 Reviewed-By: James M Snell Reviewed-By: Jon Moss Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Anna Henningsen Reviewed-By: Tobias Nießen Reviewed-By: Luigi Pinca --- test/addons-napi/test_symbol/test_symbol.c | 2 +- test/addons-napi/test_typedarray/test_typedarray.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/addons-napi/test_symbol/test_symbol.c b/test/addons-napi/test_symbol/test_symbol.c index 6acc4c55db0b2b..c91b6ae54f4932 100644 --- a/test/addons-napi/test_symbol/test_symbol.c +++ b/test/addons-napi/test_symbol/test_symbol.c @@ -12,7 +12,7 @@ napi_value Test(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_typeof(env, args[0], &valuetype)); NAPI_ASSERT(env, valuetype == napi_symbol, - "Wrong type of argments. Expects a symbol."); + "Wrong type of arguments. Expects a symbol."); char buffer[128]; size_t buffer_size = 128; diff --git a/test/addons-napi/test_typedarray/test_typedarray.c b/test/addons-napi/test_typedarray/test_typedarray.c index 82c7a5bf361d30..2758a6f53298fe 100644 --- a/test/addons-napi/test_typedarray/test_typedarray.c +++ b/test/addons-napi/test_typedarray/test_typedarray.c @@ -13,20 +13,20 @@ napi_value Multiply(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_typeof(env, args[0], &valuetype0)); NAPI_ASSERT(env, valuetype0 == napi_object, - "Wrong type of argments. Expects a typed array as first argument."); + "Wrong type of arguments. Expects a typed array as first argument."); napi_value input_array = args[0]; bool is_typedarray; NAPI_CALL(env, napi_is_typedarray(env, input_array, &is_typedarray)); NAPI_ASSERT(env, is_typedarray, - "Wrong type of argments. Expects a typed array as first argument."); + "Wrong type of arguments. Expects a typed array as first argument."); napi_valuetype valuetype1; NAPI_CALL(env, napi_typeof(env, args[1], &valuetype1)); NAPI_ASSERT(env, valuetype1 == napi_number, - "Wrong type of argments. Expects a number as second argument."); + "Wrong type of arguments. Expects a number as second argument."); double multiplier; NAPI_CALL(env, napi_get_value_double(env, args[1], &multiplier)); @@ -108,26 +108,26 @@ napi_value CreateTypedArray(napi_env env, napi_callback_info info) { NAPI_CALL(env, napi_typeof(env, input_array, &valuetype0)); NAPI_ASSERT(env, valuetype0 == napi_object, - "Wrong type of argments. Expects a typed array as first argument."); + "Wrong type of arguments. Expects a typed array as first argument."); bool is_typedarray; NAPI_CALL(env, napi_is_typedarray(env, input_array, &is_typedarray)); NAPI_ASSERT(env, is_typedarray, - "Wrong type of argments. Expects a typed array as first argument."); + "Wrong type of arguments. Expects a typed array as first argument."); napi_valuetype valuetype1; napi_value input_buffer = args[1]; NAPI_CALL(env, napi_typeof(env, input_buffer, &valuetype1)); NAPI_ASSERT(env, valuetype1 == napi_object, - "Wrong type of argments. Expects an array buffer as second argument."); + "Wrong type of arguments. Expects an array buffer as second argument."); bool is_arraybuffer; NAPI_CALL(env, napi_is_arraybuffer(env, input_buffer, &is_arraybuffer)); NAPI_ASSERT(env, is_arraybuffer, - "Wrong type of argments. Expects an array buffer as second argument."); + "Wrong type of arguments. Expects an array buffer as second argument."); napi_typedarray_type type; napi_value in_array_buffer; From 375d04394f46599096a54520db11990ecae91217 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 16 Jan 2018 14:05:26 -0500 Subject: [PATCH 12/45] doc: remove uannecessary Require This was the only instance were we said a parameter was required. It is assumed parameters are required unless the doc says they are option. Remove `Required` to make consistent with the rest of the doc PR-URL: https://github.com/nodejs/node/pull/18184 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Gireesh Punathil Reviewed-By: Daniel Bevenius --- doc/api/n-api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index d884853ec41e52..9b780803e27688 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3283,7 +3283,7 @@ napi_status napi_create_async_work(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] async_resource`: An optional object associated with the async work that will be passed to possible async_hooks [`init` hooks][]. -- `[in] async_resource_name`: An identifier for the kind of resource that is +- `[in] async_resource_name`: Identifier for the kind of resource that is being provided for diagnostic information exposed by the `async_hooks` API. - `[in] execute`: The native function which should be called to excute the logic asynchronously. @@ -3382,7 +3382,7 @@ napi_status napi_async_init(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] async_resource`: An optional object associated with the async work that will be passed to possible `async_hooks` [`init` hooks][]. -- `[in] async_resource_name`: Required identifier for the kind of resource +- `[in] async_resource_name`: Identifier for the kind of resource that is being provided for diagnostic information exposed by the `async_hooks` API. - `[out] result`: The initialized async context. From f7fb0c3c7304a146787a99da1945ff8e2dbefc20 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 13 Jan 2018 16:51:28 -0500 Subject: [PATCH 13/45] timers: allow Immediates to be unrefed Refactor Immediates handling to allow for them to be unrefed, similar to setTimeout, but without extra handles. Document the new `immediate.ref()` and `immediate.unref()` methods. Add SetImmediateUnref on the C++ side. PR-URL: https://github.com/nodejs/node/pull/18139 Reviewed-By: Anna Henningsen Reviewed-By: Franziska Hinkelmann Reviewed-By: James M Snell --- test/addons-napi/test_uv_loop/test_uv_loop.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/addons-napi/test_uv_loop/test_uv_loop.cc b/test/addons-napi/test_uv_loop/test_uv_loop.cc index 44819f72bb6b9d..048e25af9ddfb3 100644 --- a/test/addons-napi/test_uv_loop/test_uv_loop.cc +++ b/test/addons-napi/test_uv_loop/test_uv_loop.cc @@ -24,6 +24,15 @@ void* SetImmediate(napi_env env, T&& cb) { assert(cb() != nullptr); }); + // Idle handle is needed only to stop the event loop from blocking in poll. + uv_idle_t* idle = new uv_idle_t; + uv_idle_init(loop, idle); + uv_idle_start(idle, [](uv_idle_t* idle) { + uv_close(reinterpret_cast(idle), [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }); + }); + return nullptr; } From 23fd46040d2c194b69497d5aea49a5bf1448b43a Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 23 Jan 2018 22:18:38 -0800 Subject: [PATCH 14/45] test: refactor addons-napi/test_exception/test.js * provide block scoping to prevent unintended side effects * remove confusing and unnecessary assertion message * use consisitent `actual, expected` argument order for assertions PR-URL: https://github.com/nodejs/node/pull/18340 Reviewed-By: Colin Ihrig Reviewed-By: Jon Moss Reviewed-By: James M Snell --- test/addons-napi/test_exception/test.js | 86 ++++++++++++------------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/test/addons-napi/test_exception/test.js b/test/addons-napi/test_exception/test.js index 8bd2f50b12b15f..83961411df7574 100644 --- a/test/addons-napi/test_exception/test.js +++ b/test/addons-napi/test_exception/test.js @@ -4,49 +4,47 @@ const common = require('../../common'); const test_exception = require(`./build/${common.buildType}/test_exception`); const assert = require('assert'); const theError = new Error('Some error'); -function throwTheError() { - throw theError; + +{ + const throwTheError = () => { throw theError; }; + + // Test that the native side successfully captures the exception + let returnedError = test_exception.returnException(throwTheError); + assert.strictEqual(theError, returnedError); + + // Test that the native side passes the exception through + assert.throws( + () => { test_exception.allowException(throwTheError); }, + (err) => err === theError + ); + + // Test that the exception thrown above was marked as pending + // before it was handled on the JS side + assert.strictEqual(test_exception.wasPending(), true, + 'VM was marked as having an exception pending' + + ' when it was allowed through'); + + // Test that the native side does not capture a non-existing exception + returnedError = test_exception.returnException(common.mustCall()); + assert.strictEqual(returnedError, undefined, + 'Returned error should be undefined when no exception is' + + ` thrown, but ${returnedError} was passed`); } -let caughtError; - -// Test that the native side successfully captures the exception -let returnedError = test_exception.returnException(throwTheError); -assert.strictEqual(theError, returnedError); - -// Test that the native side passes the exception through -assert.throws( - () => { - test_exception.allowException(throwTheError); - }, - function(err) { - return err === theError; - }, - 'Thrown exception was allowed to pass through unhindered' -); - -// Test that the exception thrown above was marked as pending -// before it was handled on the JS side -assert.strictEqual(test_exception.wasPending(), true, - 'VM was marked as having an exception pending' + - ' when it was allowed through'); - -// Test that the native side does not capture a non-existing exception -returnedError = test_exception.returnException(common.mustCall()); -assert.strictEqual(undefined, returnedError, - 'Returned error should be undefined when no exception is' + - ` thrown, but ${returnedError} was passed`); - -// Test that no exception appears that was not thrown by us -try { - test_exception.allowException(common.mustCall()); -} catch (anError) { - caughtError = anError; + +{ + // Test that no exception appears that was not thrown by us + let caughtError; + try { + test_exception.allowException(common.mustCall()); + } catch (anError) { + caughtError = anError; + } + assert.strictEqual(caughtError, undefined, + 'No exception originated on the native side, but' + + ` ${caughtError} was passed`); + + // Test that the exception state remains clear when no exception is thrown + assert.strictEqual(test_exception.wasPending(), false, + 'VM was not marked as having an exception pending' + + ' when none was allowed through'); } -assert.strictEqual(undefined, caughtError, - 'No exception originated on the native side, but' + - ` ${caughtError} was passed`); - -// Test that the exception state remains clear when no exception is thrown -assert.strictEqual(test_exception.wasPending(), false, - 'VM was not marked as having an exception pending' + - ' when none was allowed through'); From 021e4a4a37638b9be62a7c8bdc90a3240bdf15de Mon Sep 17 00:00:00 2001 From: Aaron Kau Date: Sat, 27 Jan 2018 14:20:13 -0500 Subject: [PATCH 15/45] n-api: change assert ok check to notStrictEqual. PR-URL: https://github.com/nodejs/node/pull/18414 Reviewed-By: Gireesh Punathil Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- test/addons-napi/test_general/test.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index ee6618c8121289..c89d718ca18575 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -28,9 +28,9 @@ assert.strictEqual(test_general.testGetPrototype(baseObject), Object.getPrototypeOf(baseObject)); assert.strictEqual(test_general.testGetPrototype(extendedObject), Object.getPrototypeOf(extendedObject)); -assert.ok(test_general.testGetPrototype(baseObject) !== - test_general.testGetPrototype(extendedObject), - 'Prototypes for base and extended should be different'); +// Prototypes for base and extended should be different. +assert.notStrictEqual(test_general.testGetPrototype(baseObject), + test_general.testGetPrototype(extendedObject)); // test version management functions // expected version is currently 1 @@ -70,17 +70,15 @@ assert.strictEqual(test_general.derefItemWasCalled(), true, // Assert that wrapping twice fails. const x = {}; test_general.wrap(x); -assert.throws(function() { - test_general.wrap(x); -}, Error); +assert.throws(() => test_general.wrap(x), Error); // Ensure that wrapping, removing the wrap, and then wrapping again works. const y = {}; test_general.wrap(y); test_general.removeWrap(y); -assert.doesNotThrow(function() { - test_general.wrap(y); -}, Error, 'Wrapping twice succeeds if a remove_wrap() separates the instances'); +assert.doesNotThrow(() => test_general.wrap(y), Error, + 'Wrapping twice succeeds if a remove_wrap()' + + ' separates the instances'); // Ensure that removing a wrap and garbage collecting does not fire the // finalize callback. From bf96235a6ff1a116c5034cd391811f457f7e3c57 Mon Sep 17 00:00:00 2001 From: Ben Wilcox Date: Sat, 27 Jan 2018 14:39:55 -0500 Subject: [PATCH 16/45] test: show pending exception error in napi tests Shows the result of the wasPending in the error message if the assertion fails. PR-URL: https://github.com/nodejs/node/pull/18413 Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater --- test/addons-napi/test_exception/test.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/addons-napi/test_exception/test.js b/test/addons-napi/test_exception/test.js index 83961411df7574..787b7d78b1c72b 100644 --- a/test/addons-napi/test_exception/test.js +++ b/test/addons-napi/test_exception/test.js @@ -20,9 +20,10 @@ const theError = new Error('Some error'); // Test that the exception thrown above was marked as pending // before it was handled on the JS side - assert.strictEqual(test_exception.wasPending(), true, - 'VM was marked as having an exception pending' + - ' when it was allowed through'); + const exception_pending = test_exception.wasPending(); + assert.strictEqual(exception_pending, true, + 'Exception not pending as expected,' + + ` .wasPending() returned ${exception_pending}`); // Test that the native side does not capture a non-existing exception returnedError = test_exception.returnException(common.mustCall()); @@ -44,7 +45,8 @@ const theError = new Error('Some error'); ` ${caughtError} was passed`); // Test that the exception state remains clear when no exception is thrown - assert.strictEqual(test_exception.wasPending(), false, - 'VM was not marked as having an exception pending' + - ' when none was allowed through'); + const exception_pending = test_exception.wasPending(); + assert.strictEqual(exception_pending, false, + 'Exception state did not remain clear as expected,' + + ` .wasPending() returned ${exception_pending}`); } From 17862cdf683dc089903e9ae836543210a728af7e Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 22 Jan 2018 23:13:02 -0500 Subject: [PATCH 17/45] n-api: implement wrapping using private properties PR-URL: https://github.com/nodejs/node/pull/18311 Reviewed-By: Anna Henningsen Reviewed-By: Michael Dawson Fixes: https://github.com/nodejs/node/issues/14367 --- src/env.h | 2 + src/node_api.cc | 160 +++++++++++++++--------------------------------- 2 files changed, 52 insertions(+), 110 deletions(-) diff --git a/src/env.h b/src/env.h index 6113e6d2de26ea..0dc180bf7489e1 100644 --- a/src/env.h +++ b/src/env.h @@ -93,6 +93,8 @@ class ModuleWrap; V(processed_private_symbol, "node:processed") \ V(selected_npn_buffer_private_symbol, "node:selectedNpnBuffer") \ V(domain_private_symbol, "node:domain") \ + V(napi_env, "node:napi:env") \ + V(napi_wrapper, "node:napi:wrapper") \ // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. diff --git a/src/node_api.cc b/src/node_api.cc index 27ab6707de7a6f..78841d14882eeb 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -17,6 +17,7 @@ #include #include "node_api.h" #include "node_internals.h" +#include "env.h" static napi_status napi_set_last_error(napi_env env, napi_status error_code, @@ -46,6 +47,9 @@ struct napi_env__ { uv_loop_t* loop = nullptr; }; +#define NAPI_PRIVATE_KEY(context, suffix) \ + (node::Environment::GetCurrent((context))->napi_ ## suffix()) + #define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \ do { \ if ((env)->prefix ## _template.IsEmpty()) { \ @@ -373,6 +377,10 @@ class Reference : private Finalizer { } public: + void* Data() { + return _finalize_data; + } + static Reference* New(napi_env env, v8::Local value, uint32_t initial_refcount, @@ -732,45 +740,6 @@ v8::Local CreateAccessorCallbackData(napi_env env, return cbdata; } -int kWrapperFields = 3; - -// Pointer used to identify items wrapped by N-API. Used by FindWrapper and -// napi_wrap(). -const char napi_wrap_name[] = "N-API Wrapper"; - -// Search the object's prototype chain for the wrapper object. Usually the -// wrapper would be the first in the chain, but it is OK for other objects to -// be inserted in the prototype chain. -static -bool FindWrapper(v8::Local obj, - v8::Local* result = nullptr, - v8::Local* parent = nullptr) { - v8::Local wrapper = obj; - - do { - v8::Local proto = wrapper->GetPrototype(); - if (proto.IsEmpty() || !proto->IsObject()) { - return false; - } - if (parent != nullptr) { - *parent = wrapper; - } - wrapper = proto.As(); - if (wrapper->InternalFieldCount() == kWrapperFields) { - v8::Local external = wrapper->GetInternalField(1); - if (external->IsExternal() && - external.As()->Value() == v8impl::napi_wrap_name) { - break; - } - } - } while (true); - - if (result != nullptr) { - *result = wrapper; - } - return true; -} - static void DeleteEnv(napi_env env, void* data, void* hint) { delete env; } @@ -787,11 +756,8 @@ napi_env GetEnv(v8::Local context) { // because we need to stop hard if either of them is empty. // // Re https://github.com/nodejs/node/pull/14217#discussion_r128775149 - auto key = v8::Private::ForApi(isolate, - v8::String::NewFromOneByte(isolate, - reinterpret_cast("N-API Environment"), - v8::NewStringType::kInternalized).ToLocalChecked()); - auto value = global->GetPrivate(context, key).ToLocalChecked(); + auto value = global->GetPrivate(context, NAPI_PRIVATE_KEY(context, env)) + .ToLocalChecked(); if (value->IsExternal()) { result = static_cast(value.As()->Value()); @@ -801,7 +767,8 @@ napi_env GetEnv(v8::Local context) { // We must also stop hard if the result of assigning the env to the global // is either nothing or false. - CHECK(global->SetPrivate(context, key, external).FromJust()); + CHECK(global->SetPrivate(context, NAPI_PRIVATE_KEY(context, env), external) + .FromJust()); // Create a self-destructing reference to external that will get rid of the // napi_env when external goes out of scope. @@ -811,28 +778,46 @@ napi_env GetEnv(v8::Local context) { return result; } +enum UnwrapAction { + KeepWrap, + RemoveWrap +}; + static napi_status Unwrap(napi_env env, napi_value js_object, void** result, - v8::Local* wrapper, - v8::Local* parent = nullptr) { + UnwrapAction action) { + NAPI_PREAMBLE(env); CHECK_ARG(env, js_object); - CHECK_ARG(env, result); + if (action == KeepWrap) { + CHECK_ARG(env, result); + } + + v8::Isolate* isolate = env->isolate; + v8::Local context = isolate->GetCurrentContext(); v8::Local value = v8impl::V8LocalValueFromJsValue(js_object); RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg); v8::Local obj = value.As(); - RETURN_STATUS_IF_FALSE( - env, v8impl::FindWrapper(obj, wrapper, parent), napi_invalid_arg); + auto val = obj->GetPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)) + .ToLocalChecked(); + RETURN_STATUS_IF_FALSE(env, val->IsExternal(), napi_invalid_arg); + Reference* reference = + static_cast(val.As()->Value()); - v8::Local unwrappedValue = (*wrapper)->GetInternalField(0); - RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg); + if (result) { + *result = reference->Data(); + } - *result = unwrappedValue.As()->Value(); + if (action == RemoveWrap) { + CHECK(obj->DeletePrivate(context, NAPI_PRIVATE_KEY(context, wrapper)) + .FromJust()); + Reference::Delete(reference); + } - return napi_ok; + return GET_RETURN_STATUS(env); } static @@ -2391,26 +2376,9 @@ napi_status napi_wrap(napi_env env, v8::Local obj = value.As(); // If we've already wrapped this object, we error out. - RETURN_STATUS_IF_FALSE(env, !v8impl::FindWrapper(obj), napi_invalid_arg); - - // Create a wrapper object with an internal field to hold the wrapped pointer - // and a second internal field to identify the owner as N-API. - v8::Local wrapper_template; - ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, v8impl::kWrapperFields); - - auto maybe_object = wrapper_template->NewInstance(context); - CHECK_MAYBE_EMPTY(env, maybe_object, napi_generic_failure); - v8::Local wrapper = maybe_object.ToLocalChecked(); - - // Store the pointer as an external in the wrapper. - wrapper->SetInternalField(0, v8::External::New(isolate, native_object)); - wrapper->SetInternalField(1, v8::External::New(isolate, - reinterpret_cast(const_cast(v8impl::napi_wrap_name)))); - - // Insert the wrapper into the object's prototype chain. - v8::Local proto = obj->GetPrototype(); - CHECK(wrapper->SetPrototype(context, proto).FromJust()); - CHECK(obj->SetPrototype(context, wrapper).FromJust()); + RETURN_STATUS_IF_FALSE(env, + !obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).FromJust(), + napi_invalid_arg); v8impl::Reference* reference = nullptr; if (result != nullptr) { @@ -2422,52 +2390,24 @@ napi_status napi_wrap(napi_env env, reference = v8impl::Reference::New( env, obj, 0, false, finalize_cb, native_object, finalize_hint); *result = reinterpret_cast(reference); - } else if (finalize_cb != nullptr) { - // Create a self-deleting reference just for the finalize callback. - reference = v8impl::Reference::New( - env, obj, 0, true, finalize_cb, native_object, finalize_hint); + } else { + // Create a self-deleting reference. + reference = v8impl::Reference::New(env, obj, 0, true, finalize_cb, + native_object, finalize_cb == nullptr ? nullptr : finalize_hint); } - if (reference != nullptr) { - wrapper->SetInternalField(2, v8::External::New(isolate, reference)); - } + CHECK(obj->SetPrivate(context, NAPI_PRIVATE_KEY(context, wrapper), + v8::External::New(isolate, reference)).FromJust()); return GET_RETURN_STATUS(env); } napi_status napi_unwrap(napi_env env, napi_value obj, void** result) { - // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw - // JS exceptions. - CHECK_ENV(env); - v8::Local wrapper; - return napi_set_last_error(env, v8impl::Unwrap(env, obj, result, &wrapper)); + return v8impl::Unwrap(env, obj, result, v8impl::KeepWrap); } napi_status napi_remove_wrap(napi_env env, napi_value obj, void** result) { - NAPI_PREAMBLE(env); - v8::Local wrapper; - v8::Local parent; - napi_status status = v8impl::Unwrap(env, obj, result, &wrapper, &parent); - if (status != napi_ok) { - return napi_set_last_error(env, status); - } - - v8::Local external = wrapper->GetInternalField(2); - if (external->IsExternal()) { - v8impl::Reference::Delete( - static_cast(external.As()->Value())); - } - - if (!parent.IsEmpty()) { - v8::Maybe maybe = parent->SetPrototype( - env->isolate->GetCurrentContext(), wrapper->GetPrototype()); - CHECK_MAYBE_NOTHING(env, maybe, napi_generic_failure); - if (!maybe.FromMaybe(false)) { - return napi_set_last_error(env, napi_generic_failure); - } - } - - return GET_RETURN_STATUS(env); + return v8impl::Unwrap(env, obj, result, v8impl::RemoveWrap); } napi_status napi_create_external(napi_env env, From 29bbf0c419402c090f548a30f80173e0158494fb Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 2 Feb 2018 17:27:01 +0100 Subject: [PATCH 18/45] n-api: wrap control flow macro in do/while Make CHECK_ENV() safe to use in the following context: if (condition) CHECK_ENV(env); else something_else(); PR-URL: https://github.com/nodejs/node/pull/18532 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Ali Ijaz Sheikh Reviewed-By: Ruben Bridgewater Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Daniel Bevenius --- src/node_api.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 78841d14882eeb..611f6c8085e384 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -70,10 +70,12 @@ struct napi_env__ { } \ } while (0) -#define CHECK_ENV(env) \ - if ((env) == nullptr) { \ - return napi_invalid_arg; \ - } +#define CHECK_ENV(env) \ + do { \ + if ((env) == nullptr) { \ + return napi_invalid_arg; \ + } \ + } while (0) #define CHECK_ARG(env, arg) \ RETURN_STATUS_IF_FALSE((env), ((arg) != nullptr), napi_invalid_arg) From 6619a1b840e5a57b11e2ddb7f86dbd54b3cbff91 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 2 Feb 2018 10:33:01 -0500 Subject: [PATCH 19/45] doc: remove usage of you in n-api doc We avoid using 'you' in the documentation based on our guidelines. Remove usage in the n-api doc. PR-URL: https://github.com/nodejs/node/pull/18528 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater Reviewed-By: Gireesh Punathil Reviewed-By: Daniel Bevenius --- doc/api/n-api.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 9b780803e27688..d10b2f1aa63452 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3161,11 +3161,8 @@ required in order to enable correct proper of the reference. Afterward, additional manipulation of the wrapper's prototype chain may cause `napi_unwrap()` to fail. -*Note*: Calling `napi_wrap()` a second time on an object that already has a -native instance associated with it by virtue of a previous call to -`napi_wrap()` will cause an error to be returned. If you wish to associate -another native instance with the given object, call `napi_remove_wrap()` on it -first. +Calling napi_wrap() a second time on an object will return an error. To associate +another native instance with the object, use napi_remove_wrap() first. ### napi_unwrap +```C +NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env, + napi_value resource_object, + napi_async_context context, + napi_callback_scope* result) +``` +- `[in] env`: The environment that the API is invoked under. +- `[in] resource_object`: An optional object associated with the async work + that will be passed to possible async_hooks [`init` hooks][]. +- `[in] context`: Context for the async operation that is +invoking the callback. This should be a value previously obtained +from [`napi_async_init`][]. +- `[out] result`: The newly created scope. + +There are cases(for example resolving promises) where it is +necessary to have the equivalent of the scope associated with a callback +in place when making certain N-API calls. If there is no other script on +the stack the [`napi_open_callback_scope`][] and +[`napi_close_callback_scope`][] functions can be used to open/close +the required scope. + +### *napi_close_callback_scope* + +```C +NAPI_EXTERN napi_status napi_close_callback_scope(napi_env env, + napi_callback_scope scope) +``` +- `[in] env`: The environment that the API is invoked under. +- `[in] scope`: The scope to be closed. + ## Version Management ### napi_get_node_version @@ -3725,6 +3761,7 @@ NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env, [`napi_async_init`]: #n_api_napi_async_init [`napi_cancel_async_work`]: #n_api_napi_cancel_async_work [`napi_close_escapable_handle_scope`]: #n_api_napi_close_escapable_handle_scope +[`napi_close_callback_scope`]: #n_api_napi_close_callback_scope [`napi_close_handle_scope`]: #n_api_napi_close_handle_scope [`napi_create_async_work`]: #n_api_napi_create_async_work [`napi_create_error`]: #n_api_napi_create_error @@ -3750,6 +3787,7 @@ NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env, [`napi_get_last_error_info`]: #n_api_napi_get_last_error_info [`napi_get_and_clear_last_exception`]: #n_api_napi_get_and_clear_last_exception [`napi_make_callback`]: #n_api_napi_make_callback +[`napi_open_callback_scope`]: #n_api_napi_open_callback_scope [`napi_open_escapable_handle_scope`]: #n_api_napi_open_escapable_handle_scope [`napi_open_handle_scope`]: #n_api_napi_open_handle_scope [`napi_property_descriptor`]: #n_api_napi_property_descriptor diff --git a/src/node_api.cc b/src/node_api.cc index 611f6c8085e384..2c5f3066f728b1 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -44,6 +44,7 @@ struct napi_env__ { v8::Persistent accessor_data_template; napi_extended_error_info last_error; int open_handle_scopes = 0; + int open_callback_scopes = 0; uv_loop_t* loop = nullptr; }; @@ -253,6 +254,18 @@ V8EscapableHandleScopeFromJsEscapableHandleScope( return reinterpret_cast(s); } +static +napi_callback_scope JsCallbackScopeFromV8CallbackScope( + node::CallbackScope* s) { + return reinterpret_cast(s); +} + +static +node::CallbackScope* V8CallbackScopeFromJsCallbackScope( + napi_callback_scope s) { + return reinterpret_cast(s); +} + //=== Conversion between V8 Handles and napi_value ======================== // This asserts v8::Local<> will always be implemented with a single @@ -544,6 +557,7 @@ class CallbackWrapperBase : public CallbackWrapper { napi_clear_last_error(env); int open_handle_scopes = env->open_handle_scopes; + int open_callback_scopes = env->open_callback_scopes; napi_value result = cb(env, cbinfo_wrapper); @@ -552,6 +566,7 @@ class CallbackWrapperBase : public CallbackWrapper { } CHECK_EQ(env->open_handle_scopes, open_handle_scopes); + CHECK_EQ(env->open_callback_scopes, open_callback_scopes); if (!env->last_exception.IsEmpty()) { isolate->ThrowException( @@ -911,7 +926,8 @@ const char* error_messages[] = {nullptr, "An exception is pending", "The async work item was cancelled", "napi_escape_handle already called on scope", - "Invalid handle scope usage"}; + "Invalid handle scope usage", + "Invalid callback scope usage"}; static inline napi_status napi_clear_last_error(napi_env env) { env->last_error.error_code = napi_ok; @@ -942,9 +958,9 @@ napi_status napi_get_last_error_info(napi_env env, // We don't have a napi_status_last as this would result in an ABI // change each time a message was added. static_assert( - node::arraysize(error_messages) == napi_handle_scope_mismatch + 1, + node::arraysize(error_messages) == napi_callback_scope_mismatch + 1, "Count of error messages must match count of error values"); - CHECK_LE(env->last_error.error_code, napi_handle_scope_mismatch); + CHECK_LE(env->last_error.error_code, napi_callback_scope_mismatch); // Wait until someone requests the last error information to fetch the error // message string @@ -2633,6 +2649,46 @@ napi_status napi_escape_handle(napi_env env, return napi_set_last_error(env, napi_escape_called_twice); } +napi_status napi_open_callback_scope(napi_env env, + napi_value resource_object, + napi_async_context async_context_handle, + napi_callback_scope* result) { + // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw + // JS exceptions. + CHECK_ENV(env); + CHECK_ARG(env, result); + + v8::Local context = env->isolate->GetCurrentContext(); + + node::async_context* node_async_context = + reinterpret_cast(async_context_handle); + + v8::Local resource; + CHECK_TO_OBJECT(env, context, resource, resource_object); + + *result = v8impl::JsCallbackScopeFromV8CallbackScope( + new node::CallbackScope(env->isolate, + resource, + *node_async_context)); + + env->open_callback_scopes++; + return napi_clear_last_error(env); +} + +napi_status napi_close_callback_scope(napi_env env, napi_callback_scope scope) { + // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw + // JS exceptions. + CHECK_ENV(env); + CHECK_ARG(env, scope); + if (env->open_callback_scopes == 0) { + return napi_callback_scope_mismatch; + } + + env->open_callback_scopes--; + delete v8impl::V8CallbackScopeFromJsCallbackScope(scope); + return napi_clear_last_error(env); +} + napi_status napi_new_instance(napi_env env, napi_value constructor, size_t argc, diff --git a/src/node_api.h b/src/node_api.h index ee0ad3518e13aa..5940c2b4a57ac2 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -424,6 +424,14 @@ NAPI_EXTERN napi_status napi_escape_handle(napi_env env, napi_value escapee, napi_value* result); +NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env, + napi_value resource_object, + napi_async_context context, + napi_callback_scope* result); + +NAPI_EXTERN napi_status napi_close_callback_scope(napi_env env, + napi_callback_scope scope); + // Methods to support error handling NAPI_EXTERN napi_status napi_throw(napi_env env, napi_value error); NAPI_EXTERN napi_status napi_throw_error(napi_env env, diff --git a/src/node_api_types.h b/src/node_api_types.h index 230c1f4ae3446f..76f38802e83e2e 100644 --- a/src/node_api_types.h +++ b/src/node_api_types.h @@ -15,6 +15,7 @@ typedef struct napi_value__ *napi_value; typedef struct napi_ref__ *napi_ref; typedef struct napi_handle_scope__ *napi_handle_scope; typedef struct napi_escapable_handle_scope__ *napi_escapable_handle_scope; +typedef struct napi_callback_scope__ *napi_callback_scope; typedef struct napi_callback_info__ *napi_callback_info; typedef struct napi_async_context__ *napi_async_context; typedef struct napi_async_work__ *napi_async_work; @@ -70,7 +71,8 @@ typedef enum { napi_pending_exception, napi_cancelled, napi_escape_called_twice, - napi_handle_scope_mismatch + napi_handle_scope_mismatch, + napi_callback_scope_mismatch } napi_status; typedef napi_value (*napi_callback)(napi_env env, diff --git a/test/addons-napi/test_callback_scope/binding.cc b/test/addons-napi/test_callback_scope/binding.cc new file mode 100644 index 00000000000000..e6631b6ac7bb52 --- /dev/null +++ b/test/addons-napi/test_callback_scope/binding.cc @@ -0,0 +1,138 @@ +#include "node_api.h" +#include "uv.h" +#include "../common.h" + +namespace { + +// the test needs to fake out the async structure, so we need to use +// the raw structure here and then cast as done behind the scenes +// in napi calls. +struct async_context { + double async_id; + double trigger_async_id; +}; + + +napi_value RunInCallbackScope(napi_env env, napi_callback_info info) { + size_t argc; + napi_value args[4]; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, nullptr, nullptr, nullptr)); + NAPI_ASSERT(env, argc == 4 , "Wrong number of arguments"); + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); + + napi_valuetype valuetype; + NAPI_CALL(env, napi_typeof(env, args[0], &valuetype)); + NAPI_ASSERT(env, valuetype == napi_object, + "Wrong type of arguments. Expects an object as first argument."); + + NAPI_CALL(env, napi_typeof(env, args[1], &valuetype)); + NAPI_ASSERT(env, valuetype == napi_number, + "Wrong type of arguments. Expects a number as second argument."); + + NAPI_CALL(env, napi_typeof(env, args[2], &valuetype)); + NAPI_ASSERT(env, valuetype == napi_number, + "Wrong type of arguments. Expects a number as third argument."); + + NAPI_CALL(env, napi_typeof(env, args[3], &valuetype)); + NAPI_ASSERT(env, valuetype == napi_function, + "Wrong type of arguments. Expects a function as third argument."); + + struct async_context context; + NAPI_CALL(env, napi_get_value_double(env, args[1], &context.async_id)); + NAPI_CALL(env, + napi_get_value_double(env, args[2], &context.trigger_async_id)); + + napi_callback_scope scope = nullptr; + NAPI_CALL( + env, + napi_open_callback_scope(env, + args[0], + reinterpret_cast(&context), + &scope)); + + // if the function has an exception pending after the call that is ok + // so we don't use NAPI_CALL as we must close the callback scope regardless + napi_value result = nullptr; + napi_status function_call_result = + napi_call_function(env, args[0], args[3], 0, nullptr, &result); + if (function_call_result != napi_ok) { + GET_AND_THROW_LAST_ERROR((env)); + } + + NAPI_CALL(env, napi_close_callback_scope(env, scope)); + + return result; +} + +static napi_env shared_env = nullptr; +static napi_deferred deferred = nullptr; + +static void Callback(uv_work_t* req, int ignored) { + napi_env env = shared_env; + + napi_handle_scope handle_scope = nullptr; + NAPI_CALL_RETURN_VOID(env, napi_open_handle_scope(env, &handle_scope)); + + napi_value resource_name; + NAPI_CALL_RETURN_VOID(env, napi_create_string_utf8( + env, "test", NAPI_AUTO_LENGTH, &resource_name)); + napi_async_context context; + NAPI_CALL_RETURN_VOID(env, + napi_async_init(env, nullptr, resource_name, &context)); + + napi_value resource_object; + NAPI_CALL_RETURN_VOID(env, napi_create_object(env, &resource_object)); + + napi_value undefined_value; + NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined_value)); + + napi_callback_scope scope = nullptr; + NAPI_CALL_RETURN_VOID(env, napi_open_callback_scope(env, + resource_object, + context, + &scope)); + + NAPI_CALL_RETURN_VOID(env, + napi_resolve_deferred(env, deferred, undefined_value)); + + NAPI_CALL_RETURN_VOID(env, napi_close_callback_scope(env, scope)); + + NAPI_CALL_RETURN_VOID(env, napi_close_handle_scope(env, handle_scope)); + delete req; +} + +napi_value TestResolveAsync(napi_env env, napi_callback_info info) { + napi_value promise = nullptr; + if (deferred == nullptr) { + shared_env = env; + NAPI_CALL(env, napi_create_promise(env, &deferred, &promise)); + + uv_loop_t* loop = nullptr; + NAPI_CALL(env, napi_get_uv_event_loop(env, &loop)); + + uv_work_t* req = new uv_work_t(); + uv_queue_work(loop, + req, + [](uv_work_t*) {}, + Callback); + } + return promise; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor descriptors[] = { + DECLARE_NAPI_PROPERTY("runInCallbackScope", RunInCallbackScope), + DECLARE_NAPI_PROPERTY("testResolveAsync", TestResolveAsync) + }; + + NAPI_CALL(env, napi_define_properties( + env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors)); + + return exports; +} + +} // anonymous namespace + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/addons-napi/test_callback_scope/binding.gyp b/test/addons-napi/test_callback_scope/binding.gyp new file mode 100644 index 00000000000000..7ede63d94a0d77 --- /dev/null +++ b/test/addons-napi/test_callback_scope/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ], + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/test/addons-napi/test_callback_scope/test-async-hooks.js b/test/addons-napi/test_callback_scope/test-async-hooks.js new file mode 100644 index 00000000000000..1a11bf60398f9b --- /dev/null +++ b/test/addons-napi/test_callback_scope/test-async-hooks.js @@ -0,0 +1,29 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// The async_hook that we enable would register the process.emitWarning() +// call from loading the N-API addon as asynchronous activity because +// it contains a process.nextTick() call. Monkey patch it to be a no-op +// before we load the addon in order to avoid this. +process.emitWarning = () => {}; + +const { runInCallbackScope } = require(`./build/${common.buildType}/binding`); + +let insideHook = false; +async_hooks.createHook({ + before: common.mustCall((id) => { + assert.strictEqual(id, 1000); + insideHook = true; + }), + after: common.mustCall((id) => { + assert.strictEqual(id, 1000); + insideHook = false; + }) +}).enable(); + +runInCallbackScope({}, 1000, 1000, () => { + assert(insideHook); +}); diff --git a/test/addons-napi/test_callback_scope/test-resolve-async.js b/test/addons-napi/test_callback_scope/test-resolve-async.js new file mode 100644 index 00000000000000..e9f4b9044c0154 --- /dev/null +++ b/test/addons-napi/test_callback_scope/test-resolve-async.js @@ -0,0 +1,13 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const { testResolveAsync } = require(`./build/${common.buildType}/binding`); + +let called = false; +testResolveAsync().then(common.mustCall(() => { + called = true; +})); + +setTimeout(common.mustCall(() => { assert(called); }), + common.platformTimeout(20)); diff --git a/test/addons-napi/test_callback_scope/test.js b/test/addons-napi/test_callback_scope/test.js new file mode 100644 index 00000000000000..2f2efe5f47b98a --- /dev/null +++ b/test/addons-napi/test_callback_scope/test.js @@ -0,0 +1,17 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const { runInCallbackScope } = require(`./build/${common.buildType}/binding`); + +assert.strictEqual(runInCallbackScope({}, 0, 0, () => 42), 42); + +{ + process.once('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err.message, 'foo'); + })); + + runInCallbackScope({}, 0, 0, () => { + throw new Error('foo'); + }); +} From 01b28faefd992cb00dbf84cd8e5d93f74fd423c6 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 2 Feb 2018 20:34:19 -0500 Subject: [PATCH 22/45] n-api: remove extra reference from test PR-URL: https://github.com/nodejs/node/pull/18542 Reviewed-By: Michael Dawson Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Colin Ihrig --- test/addons-napi/test_constructor/test_constructor.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/addons-napi/test_constructor/test_constructor.c b/test/addons-napi/test_constructor/test_constructor.c index 70f53ec5a92f55..4ee8323dd6ed40 100644 --- a/test/addons-napi/test_constructor/test_constructor.c +++ b/test/addons-napi/test_constructor/test_constructor.c @@ -3,7 +3,6 @@ static double value_ = 1; static double static_value_ = 10; -napi_ref constructor_; napi_value GetValue(napi_env env, napi_callback_info info) { size_t argc = 0; @@ -80,8 +79,6 @@ napi_value Init(napi_env env, napi_value exports) { NAPI_CALL(env, napi_define_class(env, "MyObject", NAPI_AUTO_LENGTH, New, NULL, sizeof(properties)/sizeof(*properties), properties, &cons)); - NAPI_CALL(env, napi_create_reference(env, cons, 1, &constructor_)); - return cons; } From d2f0672902108943c3f5d3edf5cc845f9f9da9bd Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Tue, 6 Feb 2018 02:00:12 +0200 Subject: [PATCH 23/45] doc: fix typo in n-api.md PR-URL: https://github.com/nodejs/node/pull/18590 Reviewed-By: Jon Moss Reviewed-By: Weijia Wang Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Daniel Bevenius Reviewed-By: Yuta Hiroto --- doc/api/n-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 8ed758b0dfe460..4b2bb5d44b69e9 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3464,7 +3464,7 @@ invoking the callback. This should be a value previously obtained from [`napi_async_init`][]. - `[out] result`: The newly created scope. -There are cases(for example resolving promises) where it is +There are cases (for example resolving promises) where it is necessary to have the equivalent of the scope associated with a callback in place when making certain N-API calls. If there is no other script on the stack the [`napi_open_callback_scope`][] and From 9c20b87fd926be4cae6fb0bda1acc96b7584d70a Mon Sep 17 00:00:00 2001 From: Bhavani Shankar Date: Thu, 1 Feb 2018 15:32:41 +0530 Subject: [PATCH 24/45] test: improve error message output PR-URL: https://github.com/nodejs/node/pull/18498 Reviewed-By: Ruben Bridgewater Reviewed-By: Anatoli Papirovski Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil Reviewed-By: Michael Dawson --- test/addons-napi/test_general/test.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index c89d718ca18575..bcaa13d894bedc 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -63,22 +63,25 @@ let w = {}; test_general.wrap(w); w = null; global.gc(); -assert.strictEqual(test_general.derefItemWasCalled(), true, +const derefItemWasCalled = test_general.derefItemWasCalled(); +assert.strictEqual(derefItemWasCalled, true, 'deref_item() was called upon garbage collecting a ' + - 'wrapped object'); + 'wrapped object. test_general.derefItemWasCalled() ' + + `returned ${derefItemWasCalled}`); + // Assert that wrapping twice fails. const x = {}; test_general.wrap(x); -assert.throws(() => test_general.wrap(x), Error); +common.expectsError(() => test_general.wrap(x), + { type: Error, message: 'Invalid argument' }); // Ensure that wrapping, removing the wrap, and then wrapping again works. const y = {}; test_general.wrap(y); test_general.removeWrap(y); -assert.doesNotThrow(() => test_general.wrap(y), Error, - 'Wrapping twice succeeds if a remove_wrap()' + - ' separates the instances'); +// Wrapping twice succeeds if a remove_wrap() separates the instances +assert.doesNotThrow(() => test_general.wrap(y)); // Ensure that removing a wrap and garbage collecting does not fire the // finalize callback. @@ -87,8 +90,11 @@ test_general.testFinalizeWrap(z); test_general.removeWrap(z); z = null; global.gc(); -assert.strictEqual(test_general.finalizeWasCalled(), false, - 'finalize callback was not called upon garbage collection'); +const finalizeWasCalled = test_general.finalizeWasCalled(); +assert.strictEqual(finalizeWasCalled, false, + 'finalize callback was not called upon garbage collection.' + + ' test_general.finalizeWasCalled() ' + + `returned ${finalizeWasCalled}`); // test napi_adjust_external_memory const adjustedValue = test_general.testAdjustExternalMemory(); From ef50323be7235299e32d3beef7919ea714f868f7 Mon Sep 17 00:00:00 2001 From: Jack Horton Date: Mon, 5 Feb 2018 11:00:33 -0800 Subject: [PATCH 25/45] test: convert new tests to use error types PR-URL: https://github.com/nodejs/node/pull/18581 Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca --- test/addons-napi/test_typedarray/test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/addons-napi/test_typedarray/test.js b/test/addons-napi/test_typedarray/test.js index 4a4e79ebe7bcdb..4c139d92200fe3 100644 --- a/test/addons-napi/test_typedarray/test.js +++ b/test/addons-napi/test_typedarray/test.js @@ -60,7 +60,7 @@ arrayTypes.forEach((currentType) => { const template = Reflect.construct(currentType, buffer); assert.throws(() => { test_typedarray.CreateTypedArray(template, buffer, 0, 136); - }, /Invalid typed array length/); + }, RangeError); }); const nonByteArrayTypes = [ Int16Array, Uint16Array, Int32Array, Uint32Array, @@ -71,5 +71,5 @@ nonByteArrayTypes.forEach((currentType) => { test_typedarray.CreateTypedArray(template, buffer, currentType.BYTES_PER_ELEMENT + 1, 1); console.log(`start of offset ${currentType}`); - }, /start offset of/); + }, RangeError); }); From c4592e6ac997d2cd94d071197c7aa1a776f220cf Mon Sep 17 00:00:00 2001 From: Aonghus O Nia Date: Thu, 8 Feb 2018 17:01:23 -0500 Subject: [PATCH 26/45] doc: fix exporting a function example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Missing the length argument in napi_create_function. PR-URL: https://github.com/nodejs/node/pull/18661 Reviewed-By: Michael Dawson Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- doc/api/n-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4b2bb5d44b69e9..4e9ba918ee5b0d 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -918,7 +918,7 @@ For example, to set a function to be returned by the `require()` for the addon: napi_value Init(napi_env env, napi_value exports) { napi_value method; napi_status status; - status = napi_create_function(env, "exports", Method, NULL, &method); + status = napi_create_function(env, "exports", NAPI_AUTO_LENGTH, Method, NULL, &method); if (status != napi_ok) return NULL; return method; } From 5ab4d319ef854d81b4eb98c77f0c6ed8bfe88e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 10 Feb 2018 16:32:01 +0100 Subject: [PATCH 27/45] doc: mark NAPI_AUTO_LENGTH as code PR-URL: https://github.com/nodejs/node/pull/18697 Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Jeremiah Senkpiel Reviewed-By: Luigi Pinca --- doc/api/n-api.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4e9ba918ee5b0d..aad4cadd948ad2 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -554,10 +554,10 @@ NAPI_NO_RETURN void napi_fatal_error(const char* location, - `[in] location`: Optional location at which the error occurred. - `[in] location_len`: The length of the location in bytes, or -NAPI_AUTO_LENGTH if it is null-terminated. +`NAPI_AUTO_LENGTH` if it is null-terminated. - `[in] message`: The message associated with the error. - `[in] message_len`: The length of the message in bytes, or -NAPI_AUTO_LENGTH if it is +`NAPI_AUTO_LENGTH` if it is null-terminated. The function call does not return, the process will be terminated. @@ -1260,7 +1260,7 @@ napi_status napi_create_function(napi_env env, - `[in] utf8name`: A string representing the name of the function encoded as UTF8. - `[in] length`: The length of the utf8name in bytes, or -NAPI_AUTO_LENGTH if it is null-terminated. +`NAPI_AUTO_LENGTH` if it is null-terminated. - `[in] cb`: A function pointer to the native function to be invoked when the created function is invoked from JavaScript. - `[in] data`: Optional arbitrary context data to be passed into the native @@ -1493,7 +1493,7 @@ napi_status napi_create_string_latin1(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] str`: Character buffer representing a ISO-8859-1-encoded string. - `[in] length`: The length of the string in bytes, or -NAPI_AUTO_LENGTH if it is null-terminated. +`NAPI_AUTO_LENGTH` if it is null-terminated. - `[out] result`: A `napi_value` representing a JavaScript String. Returns `napi_ok` if the API succeeded. @@ -1518,7 +1518,7 @@ napi_status napi_create_string_utf16(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] str`: Character buffer representing a UTF16-LE-encoded string. - `[in] length`: The length of the string in two-byte code units, or -NAPI_AUTO_LENGTH if it is null-terminated. +`NAPI_AUTO_LENGTH` if it is null-terminated. - `[out] result`: A `napi_value` representing a JavaScript String. Returns `napi_ok` if the API succeeded. @@ -1542,7 +1542,7 @@ napi_status napi_create_string_utf8(napi_env env, - `[in] env`: The environment that the API is invoked under. - `[in] str`: Character buffer representing a UTF8-encoded string. -- `[in] length`: The length of the string in bytes, or NAPI_AUTO_LENGTH +- `[in] length`: The length of the string in bytes, or `NAPI_AUTO_LENGTH` if it is null-terminated. - `[out] result`: A `napi_value` representing a JavaScript String. @@ -3068,7 +3068,7 @@ napi_status napi_define_class(napi_env env, - `[in] utf8name`: Name of the JavaScript constructor function; this is not required to be the same as the C++ class name, though it is recommended for clarity. - - `[in] length`: The length of the utf8name in bytes, or NAPI_AUTO_LENGTH + - `[in] length`: The length of the utf8name in bytes, or `NAPI_AUTO_LENGTH` if it is null-terminated. - `[in] constructor`: Callback function that handles constructing instances of the class. (This should be a static method on the class, not an actual From a1df46637190a5a4f4ea119534645215116f37b6 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 11 Feb 2018 17:07:39 -0500 Subject: [PATCH 28/45] test: remove unnecessary timer The timer in NAPI's test_callback_scope/test-resolve-async.js can be removed. If the test fails, it will timeout on its own. The extra timer increases the chances of the test being flaky. PR-URL: https://github.com/nodejs/node/pull/18719 Fixes: https://github.com/nodejs/node/issues/18702 Reviewed-By: Ruben Bridgewater Reviewed-By: Santiago Gimeno Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- .../test_callback_scope/test-resolve-async.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/addons-napi/test_callback_scope/test-resolve-async.js b/test/addons-napi/test_callback_scope/test-resolve-async.js index e9f4b9044c0154..77f25c9dde533f 100644 --- a/test/addons-napi/test_callback_scope/test-resolve-async.js +++ b/test/addons-napi/test_callback_scope/test-resolve-async.js @@ -1,13 +1,6 @@ 'use strict'; const common = require('../../common'); -const assert = require('assert'); const { testResolveAsync } = require(`./build/${common.buildType}/binding`); -let called = false; -testResolveAsync().then(common.mustCall(() => { - called = true; -})); - -setTimeout(common.mustCall(() => { assert(called); }), - common.platformTimeout(20)); +testResolveAsync().then(common.mustCall()); From eb85cd8e7e1c2755568ecee92fb0879edaff1439 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 27 Feb 2018 13:02:24 -0500 Subject: [PATCH 29/45] n-api: fix object test Passing a pointer to a static integer is sufficient for the test. PR-URL: https://github.com/nodejs/node/pull/19039 Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: Hitesh Kanwathirtha --- test/addons-napi/test_object/test_object.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/addons-napi/test_object/test_object.c b/test/addons-napi/test_object/test_object.c index 49a90dd3f99f45..ccf1573114a6f1 100644 --- a/test/addons-napi/test_object/test_object.c +++ b/test/addons-napi/test_object/test_object.c @@ -1,7 +1,6 @@ #include #include "../common.h" #include -#include static int test_value = 3; @@ -199,9 +198,7 @@ napi_value Wrap(napi_env env, napi_callback_info info) { napi_value arg; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &arg, NULL, NULL)); - int32_t* data = malloc(sizeof(int32_t)); - *data = test_value; - NAPI_CALL(env, napi_wrap(env, arg, data, NULL, NULL, NULL)); + NAPI_CALL(env, napi_wrap(env, arg, &test_value, NULL, NULL, NULL)); return NULL; } From 2b3207d83b4fb3ab27e824c37730fe977991195e Mon Sep 17 00:00:00 2001 From: Eric Bickle Date: Thu, 1 Mar 2018 09:27:16 -0800 Subject: [PATCH 30/45] doc: fix n-api asynchronous threading docs Documentation for N-API Custom Asynchronous Operations incorrectly stated that async execution happens on the main event loop. Added details to napi_create_async_work about which threads are used to invoke the execute and complete callbacks. Changed 'async' to 'asynchronous' in the documentation for Custom Asynchronous Operations. Changed "executes in parallel" to "can execute in parallel" for the documentation of napi_create_async_work execute parameter. PR-URL: https://github.com/nodejs/node/pull/19073 Fixes: https://github.com/nodejs/node/issues/19071 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- doc/api/n-api.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index aad4cadd948ad2..9055e44da7b562 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -3283,9 +3283,11 @@ napi_status napi_create_async_work(napi_env env, - `[in] async_resource_name`: Identifier for the kind of resource that is being provided for diagnostic information exposed by the `async_hooks` API. - `[in] execute`: The native function which should be called to excute -the logic asynchronously. +the logic asynchronously. The given function is called from a worker pool +thread and can execute in parallel with the main event loop thread. - `[in] complete`: The native function which will be called when the -asynchronous logic is comple or is cancelled. +asynchronous logic is completed or is cancelled. The given function is called +from the main event loop thread. - `[in] data`: User-provided data context. This will be passed back into the execute and complete functions. - `[out] result`: `napi_async_work*` which is the handle to the newly created @@ -3361,9 +3363,9 @@ callback invocation, even if it has been successfully cancelled. ## Custom Asynchronous Operations The simple asynchronous work APIs above may not be appropriate for every -scenario, because with those the async execution still happens on the main -event loop. When using any other async mechanism, the following APIs are -necessary to ensure an async operation is properly tracked by the runtime. +scenario. When using any other asynchronous mechanism, the following APIs +are necessary to ensure an asynchronous operation is properly tracked by +the runtime. ### napi_async_init +```C +napi_status napi_fatal_exception(napi_env env, napi_value err); +``` + +- `[in] env`: The environment that the API is invoked under. +- `[in] err`: The error you want to pass to `uncaughtException`. + +Trigger an `uncaughtException` in JavaScript. Useful if an async +callback throws an exception with no way to recover. + ### Fatal Errors In the event of an unrecoverable error in a native module, a fatal error can be diff --git a/src/node_api.cc b/src/node_api.cc index 9645b5ab30720d..efc08f8c4e570e 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -173,6 +173,7 @@ struct napi_env__ { (out) = v8::type::New((buffer), (byte_offset), (length)); \ } while (0) + namespace { namespace v8impl { @@ -295,6 +296,13 @@ v8::Local V8LocalValueFromJsValue(napi_value v) { return local; } +static inline void trigger_fatal_exception( + napi_env env, v8::Local local_err) { + v8::Local local_msg = + v8::Exception::CreateMessage(env->isolate, local_err); + node::FatalException(env->isolate, local_err, local_msg); +} + static inline napi_status V8NameFromPropertyDescriptor(napi_env env, const napi_property_descriptor* p, v8::Local* result) { @@ -971,6 +979,16 @@ napi_status napi_get_last_error_info(napi_env env, return napi_ok; } +napi_status napi_fatal_exception(napi_env env, napi_value err) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, err); + + v8::Local local_err = v8impl::V8LocalValueFromJsValue(err); + v8impl::trigger_fatal_exception(env, local_err); + + return napi_clear_last_error(env); +} + NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, const char* message, @@ -3375,10 +3393,9 @@ class Work : public node::AsyncResource { // report it as a fatal exception. (There is no JavaScript on the // callstack that can possibly handle it.) if (!env->last_exception.IsEmpty()) { - v8::TryCatch try_catch(env->isolate); - env->isolate->ThrowException( - v8::Local::New(env->isolate, env->last_exception)); - node::FatalException(env->isolate, try_catch); + v8::Local local_err = v8::Local::New( + env->isolate, env->last_exception); + v8impl::trigger_fatal_exception(env, local_err); } } } diff --git a/src/node_api.h b/src/node_api.h index 5940c2b4a57ac2..627e56118011e6 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -112,6 +112,8 @@ NAPI_EXTERN napi_status napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); +NAPI_EXTERN napi_status napi_fatal_exception(napi_env env, napi_value err); + NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, const char* message, diff --git a/src/node_internals.h b/src/node_internals.h index 8f3fb4fb9aa27d..7f73f4b8deaa9a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -218,6 +218,11 @@ void GetSockOrPeerName(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().Set(err); } +void FatalException(v8::Isolate* isolate, + v8::Local error, + v8::Local message); + + void SignalExit(int signo); #ifdef __POSIX__ void RegisterSignalHandler(int signal, diff --git a/test/addons-napi/test_async/test-uncaught.js b/test/addons-napi/test_async/test-uncaught.js new file mode 100644 index 00000000000000..fdcb3203f54410 --- /dev/null +++ b/test/addons-napi/test_async/test-uncaught.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const test_async = require(`./build/${common.buildType}/test_async`); + +process.on('uncaughtException', common.mustCall(function(err) { + try { + throw new Error('should not fail'); + } catch (err) { + assert.strictEqual(err.message, 'should not fail'); + } + assert.strictEqual(err.message, 'uncaught'); +})); + +// Successful async execution and completion callback. +test_async.Test(5, {}, common.mustCall(function() { + throw new Error('uncaught'); +})); diff --git a/test/addons-napi/test_fatal_exception/binding.gyp b/test/addons-napi/test_fatal_exception/binding.gyp new file mode 100644 index 00000000000000..f4dc0a71ea2817 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_fatal_exception", + "sources": [ "test_fatal_exception.c" ] + } + ] +} diff --git a/test/addons-napi/test_fatal_exception/test.js b/test/addons-napi/test_fatal_exception/test.js new file mode 100644 index 00000000000000..f02b9bce1e8169 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/test.js @@ -0,0 +1,11 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const test_fatal = require(`./build/${common.buildType}/test_fatal_exception`); + +process.on('uncaughtException', common.mustCall(function(err) { + assert.strictEqual(err.message, 'fatal error'); +})); + +const err = new Error('fatal error'); +test_fatal.Test(err); diff --git a/test/addons-napi/test_fatal_exception/test_fatal_exception.c b/test/addons-napi/test_fatal_exception/test_fatal_exception.c new file mode 100644 index 00000000000000..fd81c56d856db8 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/test_fatal_exception.c @@ -0,0 +1,26 @@ +#include +#include "../common.h" + +napi_value Test(napi_env env, napi_callback_info info) { + napi_value err; + size_t argc = 1; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &err, NULL, NULL)); + + NAPI_CALL(env, napi_fatal_exception(env, err)); + + return NULL; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NAPI_PROPERTY("Test", Test), + }; + + NAPI_CALL(env, napi_define_properties( + env, exports, sizeof(properties) / sizeof(*properties), properties)); + + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) From 3a50835447157653f3eb4b68db3ffe222a25ce30 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 19 Mar 2018 11:59:41 -0400 Subject: [PATCH 40/45] n-api: re-write test_make_callback This re-writes the test in C by dropping std::vector in favour of a C99 variable length array, and by dropping the anonymous namespace in favour of static function declarations. PR-URL: https://github.com/nodejs/node/pull/19448 Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Michael Dawson --- .../{binding.cc => binding.c} | 21 +++++++++---------- .../test_make_callback/binding.gyp | 2 +- 2 files changed, 11 insertions(+), 12 deletions(-) rename test/addons-napi/test_make_callback/{binding.cc => binding.c} (80%) diff --git a/test/addons-napi/test_make_callback/binding.cc b/test/addons-napi/test_make_callback/binding.c similarity index 80% rename from test/addons-napi/test_make_callback/binding.cc rename to test/addons-napi/test_make_callback/binding.c index 952dfcc1cb5bec..23750f56b838fc 100644 --- a/test/addons-napi/test_make_callback/binding.cc +++ b/test/addons-napi/test_make_callback/binding.c @@ -1,13 +1,13 @@ #include #include "../common.h" -#include -namespace { +#define MAX_ARGUMENTS 10 +static napi_value MakeCallback(napi_env env, napi_callback_info info) { - const int kMaxArgs = 10; - size_t argc = kMaxArgs; - napi_value args[kMaxArgs]; + size_t argc = MAX_ARGUMENTS; + size_t n; + napi_value args[MAX_ARGUMENTS]; // NOLINTNEXTLINE (readability/null_usage) NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); @@ -16,9 +16,9 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value recv = args[0]; napi_value func = args[1]; - std::vector argv; - for (size_t n = 2; n < argc; n += 1) { - argv.push_back(args[n]); + napi_value argv[MAX_ARGUMENTS - 2]; + for (n = 2; n < argc; n += 1) { + argv[n - 2] = args[n]; } napi_valuetype func_type; @@ -35,7 +35,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { napi_value result; if (func_type == napi_function) { NAPI_CALL(env, napi_make_callback( - env, context, recv, func, argv.size(), argv.data(), &result)); + env, context, recv, func, argc - 2, argv, &result)); } else { NAPI_ASSERT(env, false, "Unexpected argument type"); } @@ -45,6 +45,7 @@ napi_value MakeCallback(napi_env env, napi_callback_info info) { return result; } +static napi_value Init(napi_env env, napi_value exports) { napi_value fn; NAPI_CALL(env, napi_create_function( @@ -54,6 +55,4 @@ napi_value Init(napi_env env, napi_value exports) { return exports; } -} // namespace - NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/addons-napi/test_make_callback/binding.gyp b/test/addons-napi/test_make_callback/binding.gyp index 7ede63d94a0d77..23daf507916ff6 100644 --- a/test/addons-napi/test_make_callback/binding.gyp +++ b/test/addons-napi/test_make_callback/binding.gyp @@ -3,7 +3,7 @@ { 'target_name': 'binding', 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ], - 'sources': [ 'binding.cc' ] + 'sources': [ 'binding.c' ] } ] } From b25dec8914d8dc2b91b93996674095461f0b4133 Mon Sep 17 00:00:00 2001 From: jiangq Date: Fri, 23 Mar 2018 22:28:39 +0800 Subject: [PATCH 41/45] doc: Add a missing comma PR-URL: https://github.com/nodejs/node/pull/19555 Reviewed-By: Colin Ihrig Reviewed-By: Trivikram Kamat --- doc/api/n-api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index ed748abe5febd6..4bcbbe1ae14912 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -610,7 +610,7 @@ that has a loop which iterates through the elements in a large array: ```C for (int i = 0; i < 1000000; i++) { napi_value result; - napi_status status = napi_get_element(e object, i, &result); + napi_status status = napi_get_element(e, object, i, &result); if (status != napi_ok) { break; } @@ -647,7 +647,7 @@ for (int i = 0; i < 1000000; i++) { break; } napi_value result; - status = napi_get_element(e object, i, &result); + status = napi_get_element(e, object, i, &result); if (status != napi_ok) { break; } From c33daa407c12055e142e3d145a7e6b465f5dfe07 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 20 Mar 2018 17:14:24 -0400 Subject: [PATCH 42/45] n-api: bump version of n-api supported Bump the version due to additions to the api. PR-URL: https://github.com/nodejs/node/pull/19497 Reviewed-By: Colin Ihrig Reviewed-By: Trivikram Kamat Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- src/node_version.h | 2 +- test/addons-napi/test_general/test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node_version.h b/src/node_version.h index 611cc2a978663f..4d5113766807f1 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -108,6 +108,6 @@ #define NODE_MODULE_VERSION 57 // the NAPI_VERSION provided by this version of the runtime -#define NAPI_VERSION 2 +#define NAPI_VERSION 3 #endif // SRC_NODE_VERSION_H_ diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index bcaa13d894bedc..4faf508d5db145 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -33,8 +33,8 @@ assert.notStrictEqual(test_general.testGetPrototype(baseObject), test_general.testGetPrototype(extendedObject)); // test version management functions -// expected version is currently 1 -assert.strictEqual(test_general.testGetVersion(), 2); +// expected version is currently 3 +assert.strictEqual(test_general.testGetVersion(), 3); const [ major, minor, patch, release ] = test_general.testGetNodeVersion(); assert.strictEqual(process.version.split('-')[0], From 65b06de2899e7e6188dcc65a91070f51f44b8b4d Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Thu, 22 Mar 2018 17:01:37 -0400 Subject: [PATCH 43/45] n-api: ensure in-module exceptions are propagated Whenever we call into an addon, whether it is for a callback, for module init, or for async work-related reasons, we should make sure that * the last error is cleared, * the scopes before the call are the same as after, and * if an exception was thrown and captured inside the module, then it is re-thrown after the call. Therefore we should call into the module in a unified fashion. This change introduces the macro NAPI_CALL_INTO_MODULE() which should be used whenever invoking a callback provided by the module. Fixes: https://github.com/nodejs/node/issues/19437 PR-URL: https://github.com/nodejs/node/pull/19537 Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- src/node_api.cc | 78 ++++++++++--------- test/addons-napi/test_exception/test.js | 30 ++++++- .../test_exception/test_exception.c | 42 ++++++++-- 3 files changed, 107 insertions(+), 43 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index efc08f8c4e570e..f190b09e168052 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -173,6 +173,23 @@ struct napi_env__ { (out) = v8::type::New((buffer), (byte_offset), (length)); \ } while (0) +#define NAPI_CALL_INTO_MODULE(env, call, handle_exception) \ + do { \ + int open_handle_scopes = (env)->open_handle_scopes; \ + int open_callback_scopes = (env)->open_callback_scopes; \ + napi_clear_last_error((env)); \ + call; \ + CHECK_EQ((env)->open_handle_scopes, open_handle_scopes); \ + CHECK_EQ((env)->open_callback_scopes, open_callback_scopes); \ + if (!(env)->last_exception.IsEmpty()) { \ + handle_exception( \ + v8::Local::New((env)->isolate, (env)->last_exception)); \ + (env)->last_exception.Reset(); \ + } \ + } while (0) + +#define NAPI_CALL_INTO_MODULE_THROW(env, call) \ + NAPI_CALL_INTO_MODULE((env), call, (env)->isolate->ThrowException) namespace { namespace v8impl { @@ -352,10 +369,11 @@ class Finalizer { static void FinalizeBufferCallback(char* data, void* hint) { Finalizer* finalizer = static_cast(hint); if (finalizer->_finalize_callback != nullptr) { - finalizer->_finalize_callback( - finalizer->_env, - data, - finalizer->_finalize_hint); + NAPI_CALL_INTO_MODULE_THROW(finalizer->_env, + finalizer->_finalize_callback( + finalizer->_env, + data, + finalizer->_finalize_hint)); } Delete(finalizer); @@ -467,10 +485,11 @@ class Reference : private Finalizer { bool delete_self = reference->_delete_self; if (reference->_finalize_callback != nullptr) { - reference->_finalize_callback( - reference->_env, - reference->_finalize_data, - reference->_finalize_hint); + NAPI_CALL_INTO_MODULE_THROW(reference->_env, + reference->_finalize_callback( + reference->_env, + reference->_finalize_data, + reference->_finalize_hint)); } if (delete_self) { @@ -555,32 +574,17 @@ class CallbackWrapperBase : public CallbackWrapper { napi_callback cb = reinterpret_cast( v8::Local::Cast( _cbdata->GetInternalField(kInternalFieldIndex))->Value()); - v8::Isolate* isolate = _cbinfo.GetIsolate(); napi_env env = static_cast( v8::Local::Cast( _cbdata->GetInternalField(kEnvIndex))->Value()); - // Make sure any errors encountered last time we were in N-API are gone. - napi_clear_last_error(env); - - int open_handle_scopes = env->open_handle_scopes; - int open_callback_scopes = env->open_callback_scopes; - - napi_value result = cb(env, cbinfo_wrapper); + napi_value result; + NAPI_CALL_INTO_MODULE_THROW(env, result = cb(env, cbinfo_wrapper)); if (result != nullptr) { this->SetReturnValue(result); } - - CHECK_EQ(env->open_handle_scopes, open_handle_scopes); - CHECK_EQ(env->open_callback_scopes, open_callback_scopes); - - if (!env->last_exception.IsEmpty()) { - isolate->ThrowException( - v8::Local::New(isolate, env->last_exception)); - env->last_exception.Reset(); - } } const Info& _cbinfo; @@ -888,8 +892,10 @@ void napi_module_register_cb(v8::Local exports, // one is found. napi_env env = v8impl::GetEnv(context); - napi_value _exports = - mod->nm_register_func(env, v8impl::JsValueFromV8LocalValue(exports)); + napi_value _exports; + NAPI_CALL_INTO_MODULE_THROW(env, + _exports = mod->nm_register_func(env, + v8impl::JsValueFromV8LocalValue(exports))); // If register function returned a non-null exports object different from // the exports object we passed it, set that as the "exports" property of @@ -3384,19 +3390,17 @@ class Work : public node::AsyncResource { v8::HandleScope scope(env->isolate); CallbackScope callback_scope(work); - work->_complete(env, ConvertUVErrorCode(status), work->_data); + NAPI_CALL_INTO_MODULE(env, + work->_complete(env, ConvertUVErrorCode(status), work->_data), + [env] (v8::Local local_err) { + // If there was an unhandled exception in the complete callback, + // report it as a fatal exception. (There is no JavaScript on the + // callstack that can possibly handle it.) + v8impl::trigger_fatal_exception(env, local_err); + }); // Note: Don't access `work` after this point because it was // likely deleted by the complete callback. - - // If there was an unhandled exception in the complete callback, - // report it as a fatal exception. (There is no JavaScript on the - // callstack that can possibly handle it.) - if (!env->last_exception.IsEmpty()) { - v8::Local local_err = v8::Local::New( - env->isolate, env->last_exception); - v8impl::trigger_fatal_exception(env, local_err); - } } } diff --git a/test/addons-napi/test_exception/test.js b/test/addons-napi/test_exception/test.js index 787b7d78b1c72b..b9311add6c92d7 100644 --- a/test/addons-napi/test_exception/test.js +++ b/test/addons-napi/test_exception/test.js @@ -1,10 +1,26 @@ 'use strict'; +// Flags: --expose-gc const common = require('../../common'); -const test_exception = require(`./build/${common.buildType}/test_exception`); const assert = require('assert'); const theError = new Error('Some error'); +// The test module throws an error during Init, but in order for its exports to +// not be lost, it attaches them to the error's "bindings" property. This way, +// we can make sure that exceptions thrown during the module initialization +// phase are propagated through require() into JavaScript. +// https://github.com/nodejs/node/issues/19437 +const test_exception = (function() { + let resultingException; + try { + require(`./build/${common.buildType}/test_exception`); + } catch (anException) { + resultingException = anException; + } + assert.strictEqual(resultingException.message, 'Error during Init'); + return resultingException.binding; +})(); + { const throwTheError = () => { throw theError; }; @@ -50,3 +66,15 @@ const theError = new Error('Some error'); 'Exception state did not remain clear as expected,' + ` .wasPending() returned ${exception_pending}`); } + +// Make sure that exceptions that occur during finalization are propagated. +function testFinalize(binding) { + let x = test_exception[binding](); + x = null; + assert.throws(() => { global.gc(); }, /Error during Finalize/); + + // To assuage the linter's concerns. + (function() {})(x); +} +testFinalize('createExternal'); +testFinalize('createExternalBuffer'); diff --git a/test/addons-napi/test_exception/test_exception.c b/test/addons-napi/test_exception/test_exception.c index 8b664210e902a5..61116d0603bfae 100644 --- a/test/addons-napi/test_exception/test_exception.c +++ b/test/addons-napi/test_exception/test_exception.c @@ -3,7 +3,7 @@ static bool exceptionWasPending = false; -napi_value returnException(napi_env env, napi_callback_info info) { +static napi_value returnException(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); @@ -22,7 +22,7 @@ napi_value returnException(napi_env env, napi_callback_info info) { return NULL; } -napi_value allowException(napi_env env, napi_callback_info info) { +static napi_value allowException(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); @@ -38,23 +38,55 @@ napi_value allowException(napi_env env, napi_callback_info info) { return NULL; } -napi_value wasPending(napi_env env, napi_callback_info info) { +static napi_value wasPending(napi_env env, napi_callback_info info) { napi_value result; NAPI_CALL(env, napi_get_boolean(env, exceptionWasPending, &result)); return result; } -napi_value Init(napi_env env, napi_value exports) { +static void finalizer(napi_env env, void *data, void *hint) { + NAPI_CALL_RETURN_VOID(env, + napi_throw_error(env, NULL, "Error during Finalize")); +} + +static napi_value createExternal(napi_env env, napi_callback_info info) { + napi_value external; + + NAPI_CALL(env, + napi_create_external(env, NULL, finalizer, NULL, &external)); + + return external; +} + +static char buffer_data[12]; + +static napi_value createExternalBuffer(napi_env env, napi_callback_info info) { + napi_value buffer; + NAPI_CALL(env, napi_create_external_buffer(env, sizeof(buffer_data), + buffer_data, finalizer, NULL, &buffer)); + return buffer; +} + +static napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { DECLARE_NAPI_PROPERTY("returnException", returnException), DECLARE_NAPI_PROPERTY("allowException", allowException), DECLARE_NAPI_PROPERTY("wasPending", wasPending), + DECLARE_NAPI_PROPERTY("createExternal", createExternal), + DECLARE_NAPI_PROPERTY("createExternalBuffer", createExternalBuffer), }; - NAPI_CALL(env, napi_define_properties( env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors)); + napi_value error, code, message; + NAPI_CALL(env, napi_create_string_utf8(env, "Error during Init", + NAPI_AUTO_LENGTH, &message)); + NAPI_CALL(env, napi_create_string_utf8(env, "", NAPI_AUTO_LENGTH, &code)); + NAPI_CALL(env, napi_create_error(env, code, message, &error)); + NAPI_CALL(env, napi_set_named_property(env, error, "binding", exports)); + NAPI_CALL(env, napi_throw(env, error)); + return exports; } From e3a27373d0353058bd00fa9fb0c6f08d86391f01 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 31 Mar 2018 21:37:17 -0400 Subject: [PATCH 44/45] n-api: back up env before finalize Heed the comment to not use fields of a Reference after calling its finalize callback, because such a call may destroy the Reference. Fixes: https://github.com/nodejs/node/issues/19673 PR-URL: https://github.com/nodejs/node/pull/19718 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/node_api.cc | 3 ++- test/addons-napi/8_passing_wrapped/binding.cc | 15 ++++++++++++--- test/addons-napi/8_passing_wrapped/myobject.cc | 12 +++++++++++- test/addons-napi/8_passing_wrapped/test.js | 9 ++++++++- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index f190b09e168052..c4fadacb5bd224 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -483,9 +483,10 @@ class Reference : private Finalizer { // Check before calling the finalize callback, because the callback might // delete it. bool delete_self = reference->_delete_self; + napi_env env = reference->_env; if (reference->_finalize_callback != nullptr) { - NAPI_CALL_INTO_MODULE_THROW(reference->_env, + NAPI_CALL_INTO_MODULE_THROW(env, reference->_finalize_callback( reference->_env, reference->_finalize_data, diff --git a/test/addons-napi/8_passing_wrapped/binding.cc b/test/addons-napi/8_passing_wrapped/binding.cc index c284c85f9b4936..48e94f10ec4838 100644 --- a/test/addons-napi/8_passing_wrapped/binding.cc +++ b/test/addons-napi/8_passing_wrapped/binding.cc @@ -1,7 +1,9 @@ #include "myobject.h" #include "../common.h" -napi_value CreateObject(napi_env env, napi_callback_info info) { +extern size_t finalize_count; + +static napi_value CreateObject(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); @@ -12,7 +14,7 @@ napi_value CreateObject(napi_env env, napi_callback_info info) { return instance; } -napi_value Add(napi_env env, napi_callback_info info) { +static napi_value Add(napi_env env, napi_callback_info info) { size_t argc = 2; napi_value args[2]; NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); @@ -29,12 +31,19 @@ napi_value Add(napi_env env, napi_callback_info info) { return sum; } -napi_value Init(napi_env env, napi_value exports) { +static napi_value FinalizeCount(napi_env env, napi_callback_info info) { + napi_value return_value; + NAPI_CALL(env, napi_create_uint32(env, finalize_count, &return_value)); + return return_value; +} + +static napi_value Init(napi_env env, napi_value exports) { MyObject::Init(env); napi_property_descriptor desc[] = { DECLARE_NAPI_PROPERTY("createObject", CreateObject), DECLARE_NAPI_PROPERTY("add", Add), + DECLARE_NAPI_PROPERTY("finalizeCount", FinalizeCount), }; NAPI_CALL(env, diff --git a/test/addons-napi/8_passing_wrapped/myobject.cc b/test/addons-napi/8_passing_wrapped/myobject.cc index 19cc7dd2a29493..0c9ca90f52f8f3 100644 --- a/test/addons-napi/8_passing_wrapped/myobject.cc +++ b/test/addons-napi/8_passing_wrapped/myobject.cc @@ -1,9 +1,14 @@ #include "myobject.h" #include "../common.h" +size_t finalize_count = 0; + MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} -MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); } +MyObject::~MyObject() { + finalize_count++; + napi_delete_reference(env_, wrapper_); +} void MyObject::Destructor( napi_env env, void* nativeObject, void* /*finalize_hint*/) { @@ -45,6 +50,11 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) { } obj->env_ = env; + + // It is important that the below call to napi_wrap() be such that we request + // a reference to the wrapped object via the out-parameter, because this + // ensures that we test the code path that deals with a reference that is + // destroyed from its own finalizer. NAPI_CALL(env, napi_wrap(env, _this, obj, diff --git a/test/addons-napi/8_passing_wrapped/test.js b/test/addons-napi/8_passing_wrapped/test.js index 3d24fa5d9fdaa7..7793133f7750ba 100644 --- a/test/addons-napi/8_passing_wrapped/test.js +++ b/test/addons-napi/8_passing_wrapped/test.js @@ -1,9 +1,16 @@ 'use strict'; +// Flags: --expose-gc + const common = require('../../common'); const assert = require('assert'); const addon = require(`./build/${common.buildType}/binding`); -const obj1 = addon.createObject(10); +let obj1 = addon.createObject(10); const obj2 = addon.createObject(20); const result = addon.add(obj1, obj2); assert.strictEqual(result, 30); + +// Make sure the native destructor gets called. +obj1 = null; +global.gc(); +assert.strictEqual(addon.finalizeCount(), 1); From d73e802c52ea0b746d7f7403490ad08ec528addf Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Thu, 15 Mar 2018 17:22:30 -0700 Subject: [PATCH 45/45] n-api: add more `int64_t` tests * Updated tests for `Number` and `int32_t` * Added new tests for `int64_t` * Updated N-API `int64_t` behavior to return zero for all non-finite numbers * Clarified the documentation for these calls. PR-URL: https://github.com/nodejs/node/pull/19402 Refs: https://github.com/nodejs/node-chakracore/pull/500 Reviewed-By: Michael Dawson Reviewed-By: James M Snell --- doc/api/n-api.md | 27 ++++-- src/node_api.cc | 11 ++- test/addons-napi/test_number/test.js | 135 ++++++++++++++++++++------- 3 files changed, 125 insertions(+), 48 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 4bcbbe1ae14912..7ac2ab39448273 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -1808,13 +1808,17 @@ napi_status napi_get_value_int32(napi_env env, - `[out] result`: C int32 primitive equivalent of the given JavaScript Number. Returns `napi_ok` if the API succeeded. If a non-number `napi_value` -is passed in `napi_number_expected . +is passed in `napi_number_expected`. This API returns the C int32 primitive equivalent -of the given JavaScript Number. If the number exceeds the range of the -32 bit integer, then the result is truncated to the equivalent of the -bottom 32 bits. This can result in a large positive number becoming -a negative number if the value is > 2^31 -1. +of the given JavaScript Number. + +If the number exceeds the range of the 32 bit integer, then the result is +truncated to the equivalent of the bottom 32 bits. This can result in a large +positive number becoming a negative number if the value is > 2^31 -1. + +Non-finite number values (NaN, positive infinity, or negative infinity) set the +result to zero. #### napi_get_value_int64