diff options
author | legendecas <legendecas@gmail.com> | 2019-05-21 08:48:35 +0300 |
---|---|---|
committer | Beth Griggs <Bethany.Griggs@uk.ibm.com> | 2019-09-20 12:53:13 +0300 |
commit | f8622762e37f6b6881db1b4ce5d4392b9d233e4a (patch) | |
tree | 7111a77707149bb70c5b6535134b80cd9dfa779b | |
parent | 4f41e4f471180e9b86c92e51814b9aa7c5fe00dc (diff) |
n-api: make func argument of napi_create_threadsafe_function optional
PR-URL: https://github.com/nodejs/node/pull/27791
Backport-PR-URL: https://github.com/nodejs/node/pull/28399
Refs: https://github.com/nodejs/node/issues/27592
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
-rw-r--r-- | doc/api/n-api.md | 11 | ||||
-rw-r--r-- | src/node_api.cc | 15 | ||||
-rw-r--r-- | test/addons-napi/test_threadsafe_function/binding.c | 43 | ||||
-rw-r--r-- | test/addons-napi/test_threadsafe_function/test.js | 11 |
4 files changed, 69 insertions, 11 deletions
diff --git a/doc/api/n-api.md b/doc/api/n-api.md index c9bed390b11..69c7969ddb1 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -368,7 +368,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 @@ -4543,6 +4544,11 @@ prevent the event loop from exiting. The APIs `napi_ref_threadsafe_function` and <!-- YAML added: v10.6.0 +napiVersion: 4 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/27791 + description: Made `func` parameter optional with custom `call_js_cb`. --> ```C NAPI_EXTERN napi_status @@ -4560,7 +4566,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 e892d1e76b0..f04a5bbfb93 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1089,10 +1089,14 @@ class ThreadSafeFunction : public node::AsyncResource { "ERR_NAPI_TSFN_STOP_IDLE_LOOP", "Failed to stop the idle loop") == napi_ok); } else { - v8::Local<v8::Function> js_cb = + napi_value js_callback = nullptr; + if (!ref.IsEmpty()) { + v8::Local<v8::Function> js_cb = v8::Local<v8::Function>::New(env->isolate, ref); + js_callback = v8impl::JsValueFromV8LocalValue(js_cb); + } call_js_cb(env, - v8impl::JsValueFromV8LocalValue(js_cb), + js_callback, context, data); } @@ -4168,7 +4172,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); @@ -4176,7 +4179,11 @@ napi_create_threadsafe_function(napi_env env, napi_status status = napi_ok; v8::Local<v8::Function> 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> v8_context = env->context(); diff --git a/test/addons-napi/test_threadsafe_function/binding.c b/test/addons-napi/test_threadsafe_function/binding.c index d78710cac13..93415fe85b6 100644 --- a/test/addons-napi/test_threadsafe_function/binding.c +++ b/test/addons-napi/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/addons-napi/test_threadsafe_function/test.js b/test/addons-napi/test_threadsafe_function/test.js index cfd00285308..3603d79ee6b 100644 --- a/test/addons-napi/test_threadsafe_function/test.js +++ b/test/addons-napi/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({ |