Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add status napi_cannot_run_js #66

Merged
merged 2 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/emnapi/include/js_native_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ typedef enum {
napi_arraybuffer_expected,
napi_detachable_arraybuffer_expected,
napi_would_deadlock, // unused
napi_no_external_buffers_allowed
napi_no_external_buffers_allowed,
napi_cannot_run_js,
} napi_status;
// Note: when adding a new enum value to `napi_status`, please also update
// * `const int last_status` in the definition of `napi_get_last_error_info()'
Expand Down
7 changes: 4 additions & 3 deletions packages/emnapi/src/js_native_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ static const char* emnapi_error_messages[] = {
"An arraybuffer was expected",
"A detachable arraybuffer was expected",
"Main thread would deadlock",
"External buffers are not allowed"
"External buffers are not allowed",
"Cannot run JavaScript",
};

EMNAPI_INTERNAL_EXTERN void _emnapi_get_last_error_info(napi_env env,
Expand All @@ -43,9 +44,9 @@ napi_status napi_get_last_error_info(
CHECK_ENV(env);
CHECK_ARG(env, result);

const int last_status = napi_no_external_buffers_allowed;
const int last_status = napi_cannot_run_js;

static_assert((sizeof(emnapi_error_messages) / sizeof(const char*)) == napi_no_external_buffers_allowed + 1,
static_assert((sizeof(emnapi_error_messages) / sizeof(const char*)) == napi_cannot_run_js + 1,
"Count of error messages must match count of error values");

_emnapi_get_last_error_info(env,
Expand Down
8 changes: 7 additions & 1 deletion packages/emnapi/src/macro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ function $PREAMBLE (env: number, fn: (envObject: Env) => napi_status): napi_stat
$CHECK_ENV!(env)
const envObject = emnapiCtx.envStore.get(env)!
if (envObject.tryCatch.hasCaught()) return envObject.setLastError(napi_status.napi_pending_exception)
// if (!envObject.canCallIntoJs()) return envObject.setLastError(napi_status.napi_cannot_run_js)
if (!envObject.canCallIntoJs()) {
return envObject.setLastError(
envObject.moduleApiVersion === Version.NAPI_VERSION_EXPERIMENTAL
? napi_status.napi_cannot_run_js
: napi_status.napi_pending_exception
)
}
envObject.clearLastError()
try {
return $$escape!(fn as any) as napi_status
Expand Down
23 changes: 18 additions & 5 deletions packages/runtime/src/Context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ class NodejsWaitingRequestCounter {
}

export class Context {
private _isStopping = false
private _canCallIntoJs = true

public envStore = new Store<Env>()
public scopeStore = new ScopeStore()
public refStore = new Store<Reference>()
Expand All @@ -133,7 +136,7 @@ export class Context {
if (typeof process === 'object' && process !== null && typeof process.once === 'function') {
this.refCounter = new NodejsWaitingRequestCounter()
process.once('beforeExit', () => {
this.runCleanup()
this.destroy()
})
}
}
Expand Down Expand Up @@ -246,11 +249,21 @@ export class Context {
this.refCounter?.decrease()
}

// canCallIntoJs (): boolean {
// return true
// }
public setCanCallIntoJs (value: boolean): void {
this._canCallIntoJs = value
}

public setStopping (value: boolean): void {
this._isStopping = value
}

public canCallIntoJs (): boolean {
return this._canCallIntoJs && !this._isStopping
}

dispose (): void {
public destroy (): void {
this.setStopping(true)
this.setCanCallIntoJs(false)
this.runCleanup()
}
}
Expand Down
34 changes: 17 additions & 17 deletions packages/runtime/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ function throwNodeApiVersionError (moduleName: string, moduleApiVersion: number)
throw new Error(errorMessage)
}

function handleThrow (_envObject: Env, value: any): void {
// if (envObject.terminatedOrTerminating()) {
// return
// }
function handleThrow (envObject: Env, value: any): void {
if (envObject.terminatedOrTerminating()) {
return
}
throw value
}

Expand Down Expand Up @@ -64,14 +64,14 @@ export class Env implements IStoreValue {
this.id = 0
}

// /** @virtual */
// public canCallIntoJs (): boolean {
// return true
// }
/** @virtual */
public canCallIntoJs (): boolean {
return true
}

// public terminatedOrTerminating (): boolean {
// return !this.canCallIntoJs()
// }
public terminatedOrTerminating (): boolean {
return !this.canCallIntoJs()
}

public ref (): void {
this.refs++
Expand Down Expand Up @@ -223,9 +223,9 @@ export class NodeEnv extends Env {
super.deleteMe()
}

// public canCallIntoJs (): boolean {
// return this.ctx.canCallIntoJs()
// }
public override canCallIntoJs (): boolean {
return this.ctx.canCallIntoJs()
}

public triggerFatalException (err: any): void {
if (this.nodeBinding) {
Expand All @@ -245,9 +245,9 @@ export class NodeEnv extends Env {

public callbackIntoModule<T> (enforceUncaughtExceptionPolicy: boolean, fn: (env: Env) => T): T {
return this.callIntoModule(fn, (envObject, err) => {
// if (envObject.terminatedOrTerminating()) {
// return
// }
if (envObject.terminatedOrTerminating()) {
return
}
const hasProcess = typeof process === 'object' && process !== null
const hasForceFlag = hasProcess ? Boolean(process.execArgv && (process.execArgv.indexOf('--force-node-api-uncaught-exceptions-policy') !== -1)) : false
if (!hasForceFlag && !enforceUncaughtExceptionPolicy) {
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/src/typings/napi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ declare const enum napi_status {
napi_arraybuffer_expected,
napi_detachable_arraybuffer_expected,
napi_would_deadlock, // unused
napi_no_external_buffers_allowed
napi_no_external_buffers_allowed,
napi_cannot_run_js
}

declare const enum napi_property_attributes {
Expand Down
5 changes: 5 additions & 0 deletions packages/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ target_compile_definitions("reference_obj_only" PRIVATE "NAPI_VERSION=8")
add_test("reference_all_types" "./ref_by_node_api_version/binding.c" OFF OFF "")
target_compile_definitions("reference_all_types" PRIVATE "NAPI_EXPERIMENTAL")

add_test("runjs_pe" "./runjs/binding.c;./runjs/entry_point.c" OFF OFF "")
target_compile_definitions("runjs_pe" PRIVATE "NAPI_VERSION=8")
add_test("runjs_cnrj" "./runjs/binding.c;./runjs/entry_point.c" OFF OFF "")
target_compile_definitions("runjs_cnrj" PRIVATE "NAPI_EXPERIMENTAL")

if(IS_WASM)
if(IS_EMSCRIPTEN)
add_test("emnapitest" "./emnapitest/binding.c" ON OFF "-sEXPORTED_RUNTIME_METHODS=['emnapiSyncMemory']")
Expand Down
4 changes: 3 additions & 1 deletion packages/test/ref/ref_finalizer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ module.exports = new Promise((resolve) => {
throw new Error('finalizer error')
}))
}
global.gc()
setImmediate(() => {
global.gc()
})
}, reject).then(common.mustCall(resolve))
})

Expand Down
5 changes: 5 additions & 0 deletions packages/test/ref_by_node_api_version/binding.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
#include <node_api.h>
#include "../common.h"
#if !defined(__wasm__) || (defined(__EMSCRIPTEN__) || defined(__wasi__))
#include "stdlib.h"
#else
void* malloc(size_t size);
void free(void* p);
#endif

#define NAPI_ASSERT_STATUS(env, assertion, message) \
NAPI_ASSERT_BASE(env, assertion, message, napi_generic_failure)
Expand Down
57 changes: 57 additions & 0 deletions packages/test/runjs/binding.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include <js_native_api.h>
#include "../common.h"
#if !defined(__wasm__) || (defined(__EMSCRIPTEN__) || defined(__wasi__))
#include "stdlib.h"
#else
void* malloc(size_t size);
void free(void* p);
void abort() {
__builtin_trap();
}
#endif

static void Finalize(napi_env env, void* data, void* hint) {
napi_value global, set_timeout;
napi_ref* ref = data;
#ifdef NAPI_EXPERIMENTAL
napi_status expected_status = napi_cannot_run_js;
#else
napi_status expected_status = napi_pending_exception;
#endif // NAPI_EXPERIMENTAL

if (napi_delete_reference(env, *ref) != napi_ok) abort();
if (napi_get_global(env, &global) != napi_ok) abort();
if (napi_get_named_property(env, global, "setTimeout", &set_timeout) !=
expected_status)
abort();
free(ref);
}

static napi_value CreateRef(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value cb;
napi_valuetype value_type;
napi_ref* ref = malloc(sizeof(*ref));
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &cb, NULL, NULL));
NAPI_ASSERT(env, argc == 1, "Function takes only one argument");
NAPI_CALL(env, napi_typeof(env, cb, &value_type));
NAPI_ASSERT(
env, value_type == napi_function, "argument must be function");
NAPI_CALL(env, napi_add_finalizer(env, cb, ref, Finalize, NULL, ref));
return cb;
}

EXTERN_C_START
napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("createRef", CreateRef),
};

NAPI_CALL(
env,
napi_define_properties(
env, exports, sizeof(properties) / sizeof(*properties), properties));

return exports;
}
EXTERN_C_END
7 changes: 7 additions & 0 deletions packages/test/runjs/entry_point.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <node_api.h>

EXTERN_C_START
napi_value Init(napi_env env, napi_value exports);
EXTERN_C_END

NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)
29 changes: 29 additions & 0 deletions packages/test/runjs/runjs.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* eslint-disable camelcase */
'use strict'

// Test that `napi_call_function()` returns `napi_cannot_run_js` in experimental
// mode and `napi_pending_exception` otherwise. This test calls the add-on's
// `createRef()` method, which creates a strong reference to a JS function. When
// the process exits, it calls all reference finalizers. The finalizer for the
// strong reference created herein will attempt to call `napi_get_property()` on
// a property of the global object and will abort the process if the API doesn't
// return the correct status.

const { mustNotCall } = require('../common')
const { load } = require('../util')

function runTests (addon, isVersion8) {
addon.createRef(mustNotCall())
}

async function main () {
const addon_v8 = await load('runjs_pe')
const addon_new = await load('runjs_cnrj')
function runAllTests () {
runTests(addon_v8, /* isVersion8 */ true)
runTests(addon_new, /* isVersion8 */ false)
}
runAllTests()
}

module.exports = main()
3 changes: 3 additions & 0 deletions packages/test/script/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ function test (f) {
if (f.includes('async_context')) {
additionalFlags.push('--gc-interval=100', '--gc-global')
}
if (f.endsWith('ref_finalizer.test.js')) {
additionalFlags.push('--force-node-api-uncaught-exceptions-policy')
}
const r = spawnSync('node', [
'--expose-gc',
...additionalFlags,
Expand Down