Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 53297e6

Browse filesBrowse files
legendecastargos
legendecas
authored and
targos
committed
n-api: make func argument of napi_create_threadsafe_function optional
PR-URL: #27791 Refs: #27592 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
1 parent 4d12cef commit 53297e6
Copy full SHA for 53297e6

File tree

Expand file treeCollapse file tree

4 files changed

+68
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+68
-11
lines changed
Open diff view settings
Collapse file

‎doc/api/n-api.md‎

Copy file name to clipboardExpand all lines: doc/api/n-api.md
+8-2Lines changed: 8 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,8 @@ typedef void (*napi_threadsafe_function_call_js)(napi_env env,
478478
- `[in] env`: The environment to use for API calls, or `NULL` if the thread-safe
479479
function is being torn down and `data` may need to be freed.
480480
- `[in] js_callback`: The JavaScript function to call, or `NULL` if the
481-
thread-safe function is being torn down and `data` may need to be freed.
481+
thread-safe function is being torn down and `data` may need to be freed. It may
482+
also be `NULL` if the thread-safe function was created without `js_callback`.
482483
- `[in] context`: The optional data with which the thread-safe function was
483484
created.
484485
- `[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
46574658
<!-- YAML
46584659
added: v10.6.0
46594660
napiVersion: 4
4661+
changes:
4662+
- version: v10.17.0
4663+
pr-url: https://github.com/nodejs/node/pull/27791
4664+
description: Made `func` parameter optional with custom `call_js_cb`.
46604665
-->
46614666
```C
46624667
NAPI_EXTERN napi_status
@@ -4674,7 +4679,8 @@ napi_create_threadsafe_function(napi_env env,
46744679
```
46754680

46764681
- `[in] env`: The environment that the API is invoked under.
4677-
- `[in] func`: The JavaScript function to call from another thread.
4682+
- `[in] func`: An optional JavaScript function to call from another thread.
4683+
It must be provided if `NULL` is passed to `call_js_cb`.
46784684
- `[in] async_resource`: An optional object associated with the async work that
46794685
will be passed to possible `async_hooks` [`init` hooks][].
46804686
- `[in] async_resource_name`: A JavaScript string to provide an identifier for
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+11-4Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,14 @@ class ThreadSafeFunction : public node::AsyncResource {
319319
"ERR_NAPI_TSFN_STOP_IDLE_LOOP",
320320
"Failed to stop the idle loop") == napi_ok);
321321
} else {
322-
v8::Local<v8::Function> js_cb =
322+
napi_value js_callback = nullptr;
323+
if (!ref.IsEmpty()) {
324+
v8::Local<v8::Function> js_cb =
323325
v8::Local<v8::Function>::New(env->isolate, ref);
326+
js_callback = v8impl::JsValueFromV8LocalValue(js_cb);
327+
}
324328
call_js_cb(env,
325-
v8impl::JsValueFromV8LocalValue(js_cb),
329+
js_callback,
326330
context,
327331
data);
328332
}
@@ -1014,15 +1018,18 @@ napi_create_threadsafe_function(napi_env env,
10141018
napi_threadsafe_function_call_js call_js_cb,
10151019
napi_threadsafe_function* result) {
10161020
CHECK_ENV(env);
1017-
CHECK_ARG(env, func);
10181021
CHECK_ARG(env, async_resource_name);
10191022
RETURN_STATUS_IF_FALSE(env, initial_thread_count > 0, napi_invalid_arg);
10201023
CHECK_ARG(env, result);
10211024

10221025
napi_status status = napi_ok;
10231026

10241027
v8::Local<v8::Function> v8_func;
1025-
CHECK_TO_FUNCTION(env, v8_func, func);
1028+
if (func == nullptr) {
1029+
CHECK_ARG(env, call_js_cb);
1030+
} else {
1031+
CHECK_TO_FUNCTION(env, v8_func, func);
1032+
}
10261033

10271034
v8::Local<v8::Context> v8_context = env->context();
10281035

Collapse file

‎test/node-api/test_threadsafe_function/binding.c‎

Copy file name to clipboardExpand all lines: test/node-api/test_threadsafe_function/binding.c
+38-5Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,19 @@ static void call_js(napi_env env, napi_value cb, void* hint, void* data) {
129129
}
130130
}
131131

132+
static napi_ref alt_ref;
133+
// Getting the data into JS with the alternative referece
134+
static void call_ref(napi_env env, napi_value _, void* hint, void* data) {
135+
if (!(env == NULL || alt_ref == NULL)) {
136+
napi_value fn, argv, undefined;
137+
NAPI_CALL_RETURN_VOID(env, napi_get_reference_value(env, alt_ref, &fn));
138+
NAPI_CALL_RETURN_VOID(env, napi_create_int32(env, *(int*)data, &argv));
139+
NAPI_CALL_RETURN_VOID(env, napi_get_undefined(env, &undefined));
140+
NAPI_CALL_RETURN_VOID(env, napi_call_function(env, undefined, fn, 1, &argv,
141+
NULL));
142+
}
143+
}
144+
132145
// Cleanup
133146
static napi_value StopThread(napi_env env, napi_callback_info info) {
134147
size_t argc = 2;
@@ -168,20 +181,31 @@ static void join_the_threads(napi_env env, void *data, void *hint) {
168181
napi_call_function(env, undefined, js_cb, 0, NULL, NULL));
169182
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env,
170183
the_hint->js_finalize_cb));
184+
if (alt_ref != NULL) {
185+
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, alt_ref));
186+
alt_ref = NULL;
187+
}
171188
}
172189

173190
static napi_value StartThreadInternal(napi_env env,
174191
napi_callback_info info,
175192
napi_threadsafe_function_call_js cb,
176-
bool block_on_full) {
193+
bool block_on_full,
194+
bool alt_ref_js_cb) {
195+
177196
size_t argc = 4;
178197
napi_value argv[4];
179198

199+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
200+
if (alt_ref_js_cb) {
201+
NAPI_CALL(env, napi_create_reference(env, argv[0], 1, &alt_ref));
202+
argv[0] = NULL;
203+
}
204+
180205
ts_info.block_on_full =
181206
(block_on_full ? napi_tsfn_blocking : napi_tsfn_nonblocking);
182207

183208
NAPI_ASSERT(env, (ts_fn == NULL), "Existing thread-safe function");
184-
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
185209
napi_value async_name;
186210
NAPI_CALL(env, napi_create_string_utf8(env, "N-API Thread-safe Function Test",
187211
NAPI_AUTO_LENGTH, &async_name));
@@ -223,16 +247,24 @@ static napi_value Release(napi_env env, napi_callback_info info) {
223247

224248
// Startup
225249
static napi_value StartThread(napi_env env, napi_callback_info info) {
226-
return StartThreadInternal(env, info, call_js, true);
250+
return StartThreadInternal(env, info, call_js,
251+
/** block_on_full */true, /** alt_ref_js_cb */false);
227252
}
228253

229254
static napi_value StartThreadNonblocking(napi_env env,
230255
napi_callback_info info) {
231-
return StartThreadInternal(env, info, call_js, false);
256+
return StartThreadInternal(env, info, call_js,
257+
/** block_on_full */false, /** alt_ref_js_cb */false);
232258
}
233259

234260
static napi_value StartThreadNoNative(napi_env env, napi_callback_info info) {
235-
return StartThreadInternal(env, info, NULL, true);
261+
return StartThreadInternal(env, info, NULL,
262+
/** block_on_full */true, /** alt_ref_js_cb */false);
263+
}
264+
265+
static napi_value StartThreadNoJsFunc(napi_env env, napi_callback_info info) {
266+
return StartThreadInternal(env, info, call_ref,
267+
/** block_on_full */true, /** alt_ref_js_cb */true);
236268
}
237269

238270
// Module init
@@ -269,6 +301,7 @@ static napi_value Init(napi_env env, napi_value exports) {
269301
DECLARE_NAPI_PROPERTY("StartThread", StartThread),
270302
DECLARE_NAPI_PROPERTY("StartThreadNoNative", StartThreadNoNative),
271303
DECLARE_NAPI_PROPERTY("StartThreadNonblocking", StartThreadNonblocking),
304+
DECLARE_NAPI_PROPERTY("StartThreadNoJsFunc", StartThreadNoJsFunc),
272305
DECLARE_NAPI_PROPERTY("StopThread", StopThread),
273306
DECLARE_NAPI_PROPERTY("Unref", Unref),
274307
DECLARE_NAPI_PROPERTY("Release", Release),
Collapse file

‎test/node-api/test_threadsafe_function/test.js‎

Copy file name to clipboardExpand all lines: test/node-api/test_threadsafe_function/test.js
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@ new Promise(function testWithoutJSMarshaller(resolve) {
102102
}))
103103
.then((result) => assert.deepStrictEqual(result, expectedArray))
104104

105+
// Start the thread in blocking mode, and assert that all values are passed.
106+
// Quit after it's done.
107+
// Doesn't pass the callback js function to napi_create_threadsafe_function.
108+
// Instead, use an alternative reference to get js function called.
109+
.then(() => testWithJSMarshaller({
110+
threadStarter: 'StartThreadNoJsFunc',
111+
maxQueueSize: binding.MAX_QUEUE_SIZE,
112+
quitAfter: binding.ARRAY_LENGTH
113+
}))
114+
.then((result) => assert.deepStrictEqual(result, expectedArray))
115+
105116
// Start the thread in blocking mode with an infinite queue, and assert that all
106117
// values are passed. Quit after it's done.
107118
.then(() => testWithJSMarshaller({

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.