From 5705d7bf6089bb8f8a8887658f7c60de6779491c Mon Sep 17 00:00:00 2001 From: legendecas Date: Tue, 21 May 2019 13:48:35 +0800 Subject: [PATCH] n-api: make func argument of napi_create_threadsafe_function optional PR-URL: https://github.com/nodejs/node/pull/27791 Refs: https://github.com/nodejs/node/issues/27592 Reviewed-By: Gabriel Schulhof --- doc/api/n-api.md | 10 ++++- src/node_api.cc | 15 +++++-- .../test_threadsafe_function/binding.c | 43 ++++++++++++++++--- .../node-api/test_threadsafe_function/test.js | 11 +++++ 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 92960d81039601..5b1adfad8466e6 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -478,7 +478,8 @@ typedef void (*napi_threadsafe_function_call_js)(napi_env env, - `[in] env`: The environment to use for API calls, or `NULL` if the thread-safe function is being torn down and `data` may need to be freed. - `[in] js_callback`: The JavaScript function to call, or `NULL` if the -thread-safe function is being torn down and `data` may need to be freed. +thread-safe function is being torn down and `data` may need to be freed. It may +also be `NULL` if the thread-safe function was created without `js_callback`. - `[in] context`: The optional data with which the thread-safe function was created. - `[in] data`: Data created by the secondary thread. It is the responsibility of @@ -4657,6 +4658,10 @@ prevent the event loop from exiting. The APIs `napi_ref_threadsafe_function` and ```C NAPI_EXTERN napi_status @@ -4674,7 +4679,8 @@ napi_create_threadsafe_function(napi_env env, ``` - `[in] env`: The environment that the API is invoked under. -- `[in] func`: The JavaScript function to call from another thread. +- `[in] func`: An optional JavaScript function to call from another thread. +It must be provided if `NULL` is passed to `call_js_cb`. - `[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`: A JavaScript string to provide an identifier for diff --git a/src/node_api.cc b/src/node_api.cc index 3aab7a225777c6..a10664d3e344fe 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -319,10 +319,14 @@ class ThreadSafeFunction : public node::AsyncResource { "ERR_NAPI_TSFN_STOP_IDLE_LOOP", "Failed to stop the idle loop") == napi_ok); } else { - v8::Local js_cb = + napi_value js_callback = nullptr; + if (!ref.IsEmpty()) { + v8::Local js_cb = v8::Local::New(env->isolate, ref); + js_callback = v8impl::JsValueFromV8LocalValue(js_cb); + } call_js_cb(env, - v8impl::JsValueFromV8LocalValue(js_cb), + js_callback, context, data); } @@ -1014,7 +1018,6 @@ napi_create_threadsafe_function(napi_env env, napi_threadsafe_function_call_js call_js_cb, napi_threadsafe_function* result) { CHECK_ENV(env); - CHECK_ARG(env, func); CHECK_ARG(env, async_resource_name); RETURN_STATUS_IF_FALSE(env, initial_thread_count > 0, napi_invalid_arg); CHECK_ARG(env, result); @@ -1022,7 +1025,11 @@ napi_create_threadsafe_function(napi_env env, napi_status status = napi_ok; v8::Local v8_func; - CHECK_TO_FUNCTION(env, v8_func, func); + if (func == nullptr) { + CHECK_ARG(env, call_js_cb); + } else { + CHECK_TO_FUNCTION(env, v8_func, func); + } v8::Local v8_context = env->context(); diff --git a/test/node-api/test_threadsafe_function/binding.c b/test/node-api/test_threadsafe_function/binding.c index c556c9427cc176..c9c526153804c6 100644 --- a/test/node-api/test_threadsafe_function/binding.c +++ b/test/node-api/test_threadsafe_function/binding.c @@ -129,6 +129,19 @@ static void call_js(napi_env env, napi_value cb, void* hint, void* data) { } } +static napi_ref alt_ref; +// Getting the data into JS with the alternative referece +static void call_ref(napi_env env, napi_value _, void* hint, void* data) { + if (!(env == NULL || alt_ref == NULL)) { + napi_value fn, argv, undefined; + NAPI_CALL_RETURN_VOID(env, napi_get_reference_value(env, alt_ref, &fn)); + NAPI_CALL_RETURN_VOID(env, napi_create_int32(env, *(int*)data, &argv)); + NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined)); + NAPI_CALL_RETURN_VOID(env, napi_call_function(env, undefined, fn, 1, &argv, + NULL)); + } +} + // Cleanup static napi_value StopThread(napi_env env, napi_callback_info info) { size_t argc = 2; @@ -168,20 +181,31 @@ static void join_the_threads(napi_env env, void *data, void *hint) { napi_call_function(env, undefined, js_cb, 0, NULL, NULL)); NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, the_hint->js_finalize_cb)); + if (alt_ref != NULL) { + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, alt_ref)); + alt_ref = NULL; + } } static napi_value StartThreadInternal(napi_env env, napi_callback_info info, napi_threadsafe_function_call_js cb, - bool block_on_full) { + bool block_on_full, + bool alt_ref_js_cb) { + size_t argc = 4; napi_value argv[4]; + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + if (alt_ref_js_cb) { + NAPI_CALL(env, napi_create_reference(env, argv[0], 1, &alt_ref)); + argv[0] = NULL; + } + ts_info.block_on_full = (block_on_full ? napi_tsfn_blocking : napi_tsfn_nonblocking); NAPI_ASSERT(env, (ts_fn == NULL), "Existing thread-safe function"); - NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); napi_value async_name; NAPI_CALL(env, napi_create_string_utf8(env, "N-API Thread-safe Function Test", NAPI_AUTO_LENGTH, &async_name)); @@ -223,16 +247,24 @@ static napi_value Release(napi_env env, napi_callback_info info) { // Startup static napi_value StartThread(napi_env env, napi_callback_info info) { - return StartThreadInternal(env, info, call_js, true); + return StartThreadInternal(env, info, call_js, + /** block_on_full */true, /** alt_ref_js_cb */false); } static napi_value StartThreadNonblocking(napi_env env, napi_callback_info info) { - return StartThreadInternal(env, info, call_js, false); + return StartThreadInternal(env, info, call_js, + /** block_on_full */false, /** alt_ref_js_cb */false); } static napi_value StartThreadNoNative(napi_env env, napi_callback_info info) { - return StartThreadInternal(env, info, NULL, true); + return StartThreadInternal(env, info, NULL, + /** block_on_full */true, /** alt_ref_js_cb */false); +} + +static napi_value StartThreadNoJsFunc(napi_env env, napi_callback_info info) { + return StartThreadInternal(env, info, call_ref, + /** block_on_full */true, /** alt_ref_js_cb */true); } // Module init @@ -269,6 +301,7 @@ static napi_value Init(napi_env env, napi_value exports) { DECLARE_NAPI_PROPERTY("StartThread", StartThread), DECLARE_NAPI_PROPERTY("StartThreadNoNative", StartThreadNoNative), DECLARE_NAPI_PROPERTY("StartThreadNonblocking", StartThreadNonblocking), + DECLARE_NAPI_PROPERTY("StartThreadNoJsFunc", StartThreadNoJsFunc), DECLARE_NAPI_PROPERTY("StopThread", StopThread), DECLARE_NAPI_PROPERTY("Unref", Unref), DECLARE_NAPI_PROPERTY("Release", Release), diff --git a/test/node-api/test_threadsafe_function/test.js b/test/node-api/test_threadsafe_function/test.js index cfd00285308f5d..3603d79ee6b5d3 100644 --- a/test/node-api/test_threadsafe_function/test.js +++ b/test/node-api/test_threadsafe_function/test.js @@ -102,6 +102,17 @@ new Promise(function testWithoutJSMarshaller(resolve) { })) .then((result) => assert.deepStrictEqual(result, expectedArray)) +// Start the thread in blocking mode, and assert that all values are passed. +// Quit after it's done. +// Doesn't pass the callback js function to napi_create_threadsafe_function. +// Instead, use an alternative reference to get js function called. +.then(() => testWithJSMarshaller({ + threadStarter: 'StartThreadNoJsFunc', + maxQueueSize: binding.MAX_QUEUE_SIZE, + quitAfter: binding.ARRAY_LENGTH +})) +.then((result) => assert.deepStrictEqual(result, expectedArray)) + // Start the thread in blocking mode with an infinite queue, and assert that all // values are passed. Quit after it's done. .then(() => testWithJSMarshaller({