From 1ce5e63987358a7166bdba0dc82d6d03c5fc341a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 15 Feb 2019 11:48:56 +0100 Subject: [PATCH] n-api: do not call into JS when that is not allowed Check whether calling into JS is allowed before doing so. PR-URL: https://github.com/nodejs/node/pull/26127 Reviewed-By: Gus Caplan Reviewed-By: Michael Dawson --- src/js_native_api_v8.h | 14 ++++--- src/node_api.cc | 5 +++ .../test_worker_terminate/binding.gyp | 8 ++++ test/node-api/test_worker_terminate/test.js | 23 +++++++++++ .../test_worker_terminate.c | 39 +++++++++++++++++++ 5 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 test/node-api/test_worker_terminate/binding.gyp create mode 100644 test/node-api/test_worker_terminate/test.js create mode 100644 test/node-api/test_worker_terminate/test_worker_terminate.c diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 81b00f2aa59b8f..094fc4afcc044c 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -11,6 +11,7 @@ struct napi_env__ { context_persistent(isolate, context) { CHECK_EQ(isolate, context->GetIsolate()); } + virtual ~napi_env__() {} v8::Isolate* const isolate; // Shortcut for context()->GetIsolate() v8impl::Persistent context_persistent; @@ -21,6 +22,8 @@ struct napi_env__ { inline void Ref() { refs++; } inline void Unref() { if ( --refs == 0) delete this; } + virtual bool can_call_into_js() const { return true; } + v8impl::Persistent last_exception; napi_extended_error_info last_error; int open_handle_scopes = 0; @@ -68,11 +71,12 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code, RETURN_STATUS_IF_FALSE((env), !((maybe).IsEmpty()), (status)) // NAPI_PREAMBLE is not wrapped in do..while: try_catch must have function scope -#define NAPI_PREAMBLE(env) \ - CHECK_ENV((env)); \ - RETURN_STATUS_IF_FALSE((env), (env)->last_exception.IsEmpty(), \ - napi_pending_exception); \ - napi_clear_last_error((env)); \ +#define NAPI_PREAMBLE(env) \ + CHECK_ENV((env)); \ + RETURN_STATUS_IF_FALSE((env), \ + (env)->last_exception.IsEmpty() && (env)->can_call_into_js(), \ + napi_pending_exception); \ + napi_clear_last_error((env)); \ v8impl::TryCatch try_catch((env)) #define CHECK_TO_TYPE(env, type, context, result, src, status) \ diff --git a/src/node_api.cc b/src/node_api.cc index ff2e12f571a0f3..010fb3fe0ff840 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -12,9 +12,14 @@ struct node_napi_env__ : public napi_env__ { napi_env__(context) { CHECK_NOT_NULL(node_env()); } + inline node::Environment* node_env() const { return node::Environment::GetCurrent(context()); } + + bool can_call_into_js() const override { + return node_env()->can_call_into_js(); + } }; typedef node_napi_env__* node_napi_env; diff --git a/test/node-api/test_worker_terminate/binding.gyp b/test/node-api/test_worker_terminate/binding.gyp new file mode 100644 index 00000000000000..3a9465c7454148 --- /dev/null +++ b/test/node-api/test_worker_terminate/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_worker_terminate", + "sources": [ "test_worker_terminate.c" ] + } + ] +} diff --git a/test/node-api/test_worker_terminate/test.js b/test/node-api/test_worker_terminate/test.js new file mode 100644 index 00000000000000..2bfaab8e8eb0a9 --- /dev/null +++ b/test/node-api/test_worker_terminate/test.js @@ -0,0 +1,23 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const { Worker, isMainThread, workerData } = require('worker_threads'); + +if (isMainThread) { + const counter = new Int32Array(new SharedArrayBuffer(4)); + const worker = new Worker(__filename, { workerData: { counter } }); + worker.on('exit', common.mustCall(() => { + assert.strictEqual(counter[0], 1); + })); + worker.on('error', common.mustNotCall()); +} else { + const { Test } = require(`./build/${common.buildType}/test_worker_terminate`); + + const { counter } = workerData; + // Test() tries to call a function twice and asserts that the second call does + // not work because of a pending exception. + Test(() => { + Atomics.add(counter, 0, 1); + process.exit(); + }); +} diff --git a/test/node-api/test_worker_terminate/test_worker_terminate.c b/test/node-api/test_worker_terminate/test_worker_terminate.c new file mode 100644 index 00000000000000..a88d936293e2a8 --- /dev/null +++ b/test/node-api/test_worker_terminate/test_worker_terminate.c @@ -0,0 +1,39 @@ +#include +#include +#include +#include "../../js-native-api/common.h" + +napi_value Test(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value recv; + napi_value argv[1]; + napi_status status; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, &recv, NULL)); + NAPI_ASSERT(env, argc >= 1, "Not enough arguments, expected 1."); + + napi_valuetype t; + NAPI_CALL(env, napi_typeof(env, argv[0], &t)); + NAPI_ASSERT(env, t == napi_function, + "Wrong first argument, function expected."); + + status = napi_call_function(env, recv, argv[0], 0, NULL, NULL); + assert(status == napi_ok); + status = napi_call_function(env, recv, argv[0], 0, NULL, NULL); + assert(status == napi_pending_exception); + + 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)