Skip to content

Commit 725630f

Browse files
toyobayashitoyobayashi
authored
fix data race and use-after-free in napi_threadsafe_function (#198)
1 parent 8b0592d commit 725630f

28 files changed

Lines changed: 681 additions & 268 deletions

‎packages/core/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"./package.json": "./package.json"
1818
},
1919
"dependencies": {
20-
"@emnapi/wasi-threads": "1.0.4",
20+
"@emnapi/wasi-threads": "1.2.0",
2121
"tslib": "^2.4.0"
2222
},
2323
"scripts": {

‎packages/emnapi/CMakeLists.txt‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,15 @@ endif()
185185
if(NODE_WANT_INTERNALS)
186186
add_compile_definitions("NODE_WANT_INTERNALS=${NODE_WANT_INTERNALS}")
187187
endif()
188+
if(EMNAPI_WORKER_POOL_SIZE)
189+
add_compile_definitions("EMNAPI_WORKER_POOL_SIZE=${EMNAPI_WORKER_POOL_SIZE}")
190+
endif()
191+
if(EMNAPI_NEXTTICK_TYPE)
192+
add_compile_definitions("EMNAPI_NEXTTICK_TYPE=${EMNAPI_NEXTTICK_TYPE}")
193+
endif()
194+
if(EMNAPI_USE_PROXYING)
195+
add_compile_definitions("EMNAPI_USE_PROXYING=${EMNAPI_USE_PROXYING}")
196+
endif()
188197

189198
add_library(${EMNAPI_TARGET_NAME} STATIC ${EMNAPI_SRC} ${UV_SRC})
190199
target_include_directories(${EMNAPI_TARGET_NAME} PUBLIC ${EMNAPI_INCLUDE})
@@ -195,11 +204,10 @@ if(IS_EMSCRIPTEN)
195204
endif()
196205

197206
if(EMNAPI_BUILD_BASIC_LIBS)
198-
add_library(${EMNAPI_BASIC_TARGET_NAME} STATIC ${ENAPI_BASIC_SRC})
207+
add_library(${EMNAPI_BASIC_TARGET_NAME} STATIC ${ENAPI_BASIC_SRC} ${UV_SRC})
199208
target_include_directories(${EMNAPI_BASIC_TARGET_NAME} PUBLIC ${EMNAPI_INCLUDE})
200209
target_compile_definitions(${EMNAPI_BASIC_TARGET_NAME}
201210
PUBLIC ${EMNAPI_DEFINES}
202-
PRIVATE "EMNAPI_DISABLE_UV"
203211
)
204212
if(IS_EMSCRIPTEN)
205213
set_target_properties(${EMNAPI_BASIC_TARGET_NAME} PROPERTIES INTERFACE_LINK_DEPENDS ${EMNAPI_JS_LIB})
@@ -229,13 +237,13 @@ if(EMNAPI_BUILD_FOR_NAPI_RS)
229237
if(EMNAPI_BUILD_BASIC_LIBS)
230238
add_library(${EMNAPI_BASIC_NAPI_RS_MT_TARGET_NAME} STATIC
231239
${ENAPI_BASIC_SRC}
240+
${UV_SRC}
232241
"${CMAKE_CURRENT_SOURCE_DIR}/src/thread/async_worker_create.c"
233242
"${CMAKE_CURRENT_SOURCE_DIR}/src/thread/async_worker_init.S"
234243
)
235244
target_include_directories(${EMNAPI_BASIC_NAPI_RS_MT_TARGET_NAME} PUBLIC ${EMNAPI_INCLUDE})
236245
target_compile_definitions(${EMNAPI_BASIC_NAPI_RS_MT_TARGET_NAME}
237246
PUBLIC ${EMNAPI_DEFINES} "NAPI_EXTERN=__attribute__((__import_module__(\"env\")))"
238-
PRIVATE "EMNAPI_DISABLE_UV"
239247
)
240248
endif()
241249
endif()
@@ -244,14 +252,14 @@ if(EMNAPI_BUILD_BASIC_LIBS)
244252
if(EMNAPI_BUILD_BASIC_MT)
245253
add_library(${EMNAPI_BASIC_MT_TARGET_NAME} STATIC
246254
${ENAPI_BASIC_SRC}
255+
${UV_SRC}
247256
"${CMAKE_CURRENT_SOURCE_DIR}/src/thread/async_worker_create.c"
248257
"${CMAKE_CURRENT_SOURCE_DIR}/src/thread/async_worker_init.S"
249258
)
250259
target_compile_options(${EMNAPI_BASIC_MT_TARGET_NAME} PUBLIC "-matomics" "-mbulk-memory")
251260
target_include_directories(${EMNAPI_BASIC_MT_TARGET_NAME} PUBLIC ${EMNAPI_INCLUDE})
252261
target_compile_definitions(${EMNAPI_BASIC_MT_TARGET_NAME}
253262
PUBLIC ${EMNAPI_DEFINES}
254-
PRIVATE "EMNAPI_DISABLE_UV"
255263
)
256264

257265
if(IS_EMSCRIPTEN)
@@ -279,15 +287,6 @@ if(EMNAPI_BUILD_MT)
279287
set_target_properties(${EMNAPI_MT_TARGET_NAME} PROPERTIES INTERFACE_LINK_DEPENDS ${EMNAPI_JS_LIB})
280288
target_link_options(${EMNAPI_MT_TARGET_NAME} INTERFACE "--js-library=${EMNAPI_JS_LIB}")
281289
endif()
282-
if(EMNAPI_WORKER_POOL_SIZE)
283-
target_compile_definitions(${EMNAPI_MT_TARGET_NAME} PRIVATE "EMNAPI_WORKER_POOL_SIZE=${EMNAPI_WORKER_POOL_SIZE}")
284-
endif()
285-
if(EMNAPI_NEXTTICK_TYPE)
286-
target_compile_definitions(${EMNAPI_MT_TARGET_NAME} PRIVATE "EMNAPI_NEXTTICK_TYPE=${EMNAPI_NEXTTICK_TYPE}")
287-
endif()
288-
if(EMNAPI_USE_PROXYING)
289-
target_compile_definitions(${EMNAPI_MT_TARGET_NAME} PRIVATE "EMNAPI_USE_PROXYING=${EMNAPI_USE_PROXYING}")
290-
endif()
291290

292291
if(EMNAPI_BUILD_V8)
293292
add_library(${V8_MT_TARGET_NAME} STATIC ${V8_SRC})

‎packages/emnapi/emnapi.gyp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,21 @@
4848
'target_name': 'emnapi_basic',
4949
'type': 'static_library',
5050
'defines': [
51-
'EMNAPI_DISABLE_UV'
5251
],
5352
'sources': [
5453
'src/js_native_api.c',
5554
'src/node_api.c',
5655
'src/async_cleanup_hook.c',
5756
'src/async_context.c',
5857
'src/wasi_wait.c',
58+
59+
'src/uv/uv-common.c',
60+
'src/uv/threadpool.c',
61+
'src/uv/unix/loop.c',
62+
'src/uv/unix/posix-hrtime.c',
63+
'src/uv/unix/thread.c',
64+
'src/uv/unix/async.c',
65+
'src/uv/unix/core.c',
5966
],
6067
'link_settings': {
6168
'target_conditions': [

‎packages/emnapi/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
"scripts": {
1616
"build": "node ./script/build.js",
1717
"version": "node ./script/version.js",
18-
"generate-struct-info-wasm32": "node -e \"fs.mkdirSync('script/out',{recursive:true})\" && emcc -Iinclude/node script/generate_struct_info.c -sNODERAWFS -o script/out/generate_struct_info_wasm32.js && node ./script/out/generate_struct_info_wasm32.js",
19-
"generate-struct-info-wasm64": "node -e \"fs.mkdirSync('script/out',{recursive:true})\" && emcc -Iinclude/node script/generate_struct_info.c -sNODERAWFS -sMEMORY64 -o script/out/generate_struct_info_wasm64.js && node ./script/out/generate_struct_info_wasm64.js",
18+
"generate-struct-info-wasm32": "node -e \"fs.mkdirSync('script/out',{recursive:true})\" && emcc -Iinclude/node -pthread script/generate_struct_info.c -sNODERAWFS -o script/out/generate_struct_info_wasm32.js && node ./script/out/generate_struct_info_wasm32.js",
19+
"generate-struct-info-wasm64": "node -e \"fs.mkdirSync('script/out',{recursive:true})\" && emcc -Iinclude/node -pthread script/generate_struct_info.c -sNODERAWFS -sMEMORY64 -o script/out/generate_struct_info_wasm64.js && node ./script/out/generate_struct_info_wasm64.js",
2020
"generate-struct-info": "npm run generate-struct-info-wasm32 && npm run generate-struct-info-wasm64"
2121
},
2222
"repository": {

‎packages/emnapi/script/build.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ async function build () {
353353
coreTsconfigPath,
354354
path.join(__dirname, '../src/core/async-work.ts'),
355355
path.join(outputDir, 'async-work.js'),
356-
['emnapiCtx', 'emnapiEnv', 'emnapiNodeBinding', 'emnapiAsyncWorkPoolSize'],
356+
['emnapiCtx', 'emnapiNodeBinding', 'emnapiAsyncWorkPoolSize'],
357357
['napi']
358358
),
359359
buildNonEmscriptenPlugin(
@@ -362,7 +362,6 @@ async function build () {
362362
path.join(outputDir, 'threadsafe-function.js'),
363363
[
364364
'emnapiCtx',
365-
'emnapiEnv',
366365
'emnapiNodeBinding',
367366
'_emnapi_node_emit_async_destroy: __emnapi_node_emit_async_destroy',
368367
'_emnapi_node_emit_async_init: __emnapi_node_emit_async_init',

‎packages/emnapi/script/generate_struct_info.c‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
#include <stdio.h>
22
#include "../src/emnapi_internal.h"
3+
#include "../src/threadsafe_function.h"
34

45
#ifdef __wasm64__
56
#define WASM_BIT "64"
67
#else
78
#define WASM_BIT "32"
89
#endif
910

10-
int main() {
11-
FILE* f = fopen("src/typings/struct-wasm" WASM_BIT ".d.ts", "w");
12-
if (f == NULL) {
13-
fprintf(stderr, "Failed to open file\n");
14-
return 1;
15-
}
16-
11+
void print_napi_env_info(FILE* f) {
1712
fprintf(f, "declare const enum NapiEnvOffset" WASM_BIT " {\n");
1813
fprintf(f, " __size__ = %zu,\n", sizeof(node_api_base_env__));
1914
fprintf(f, " vptr = %zu,\n", offsetof(node_api_base_env__, vptr));
@@ -28,6 +23,48 @@ int main() {
2823
fprintf(f, " last_error_engine_error_code = %zu,\n", offsetof(node_api_base_env__, last_error) + offsetof(napi_extended_error_info, engine_error_code));
2924
fprintf(f, " last_error_error_code = %zu,\n", offsetof(node_api_base_env__, last_error) + offsetof(napi_extended_error_info, error_code));
3025
fprintf(f, "}\n");
26+
}
27+
28+
void print_napi_threadsafe_function_info(FILE* f) {
29+
fprintf(f, "declare const enum NapiTSFNOffset" WASM_BIT " {\n");
30+
fprintf(f, " __size__ = %zu,\n", sizeof(struct napi_threadsafe_function__));
31+
fprintf(f, " async_resource = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource));
32+
fprintf(f, " async_resource___size__ = %zu,\n", sizeof(optional_async_resource));
33+
fprintf(f, " async_resource_resource = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource) + offsetof(optional_async_resource, resource_));
34+
fprintf(f, " async_resource_async_context = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource) + offsetof(optional_async_resource, async_context_));
35+
fprintf(f, " async_resource_async_context___size__ = %zu,\n", sizeof(async_context));
36+
fprintf(f, " async_resource_async_context_async_id = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource) + offsetof(optional_async_resource, async_context_) + offsetof(async_context, async_id));
37+
fprintf(f, " async_resource_async_context_trigger_async_id = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource) + offsetof(optional_async_resource, async_context_) + offsetof(async_context, trigger_async_id));
38+
fprintf(f, " async_resource_is_some = %zu,\n", offsetof(struct napi_threadsafe_function__, async_resource) + offsetof(optional_async_resource, is_some));
39+
fprintf(f, " mutex = %zu,\n", offsetof(struct napi_threadsafe_function__, mutex));
40+
fprintf(f, " cond = %zu,\n", offsetof(struct napi_threadsafe_function__, cond));
41+
fprintf(f, " queue_size = %zu,\n", offsetof(struct napi_threadsafe_function__, queue_size));
42+
fprintf(f, " queue = %zu,\n", offsetof(struct napi_threadsafe_function__, queue));
43+
fprintf(f, " async = %zu,\n", offsetof(struct napi_threadsafe_function__, async));
44+
fprintf(f, " thread_count = %zu,\n", offsetof(struct napi_threadsafe_function__, thread_count));
45+
fprintf(f, " state = %zu,\n", offsetof(struct napi_threadsafe_function__, state));
46+
fprintf(f, " dispatch_state = %zu,\n", offsetof(struct napi_threadsafe_function__, dispatch_state));
47+
fprintf(f, " context = %zu,\n", offsetof(struct napi_threadsafe_function__, context));
48+
fprintf(f, " max_queue_size = %zu,\n", offsetof(struct napi_threadsafe_function__, max_queue_size));
49+
fprintf(f, " ref = %zu,\n", offsetof(struct napi_threadsafe_function__, ref));
50+
fprintf(f, " env = %zu,\n", offsetof(struct napi_threadsafe_function__, env));
51+
fprintf(f, " finalize_data = %zu,\n", offsetof(struct napi_threadsafe_function__, finalize_data));
52+
fprintf(f, " finalize_cb = %zu,\n", offsetof(struct napi_threadsafe_function__, finalize_cb));
53+
fprintf(f, " call_js_cb = %zu,\n", offsetof(struct napi_threadsafe_function__, call_js_cb));
54+
fprintf(f, " handles_closing = %zu,\n", offsetof(struct napi_threadsafe_function__, handles_closing));
55+
fprintf(f, " async_ref = %zu,\n", offsetof(struct napi_threadsafe_function__, async_ref));
56+
fprintf(f, "}\n");
57+
}
58+
59+
int main() {
60+
FILE* f = fopen("src/typings/struct-wasm" WASM_BIT ".d.ts", "w");
61+
if (f == NULL) {
62+
fprintf(stderr, "Failed to open file\n");
63+
return 1;
64+
}
65+
66+
print_napi_env_info(f);
67+
print_napi_threadsafe_function_info(f);
3168

3269
fclose(f);
3370
return 0;

‎packages/emnapi/src/async_cleanup_hook.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ _emnapi_ach_handle_create(node_api_basic_env env,
8282
return handle;
8383
}
8484

85-
EMNAPI_INTERNAL_EXTERN void _emnapi_set_immediate(void (*callback)(void*), void* data);
86-
8785
static void _emnapi_ach_handle_env_unref(void* arg) {
8886
node_api_basic_env env = (node_api_basic_env) arg;
8987
_emnapi_env_unref(env);

‎packages/emnapi/src/core/async-work.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable @stylistic/indent */
2-
import { onCreateWorker, napiModule, singleThreadAsyncWork, _emnapi_async_work_pool_size } from 'emnapi:shared'
2+
import { emnapiEnv, onCreateWorker, napiModule, singleThreadAsyncWork, _emnapi_async_work_pool_size } from 'emnapi:shared'
33
import { PThread, ENVIRONMENT_IS_NODE, ENVIRONMENT_IS_PTHREAD, wasmInstance, _free, wasmMemory, _malloc } from 'emscripten:runtime'
44
import { POINTER_SIZE, to64, makeDynCall, makeSetValue, from64 } from 'emscripten:parse-tools'
55
import { emnapiAWST } from '../async-work'
@@ -9,7 +9,6 @@ import { $CHECK_ENV_NOT_IN_GC, $CHECK_ARG, $CHECK_ENV } from '../macro'
99

1010
declare var emnapiPluginCtx: any
1111
declare var emnapiCtx: Context
12-
declare var emnapiEnv: Env
1312
declare var emnapiNodeBinding: NodeBinding | undefined
1413

1514
const {
@@ -100,6 +99,9 @@ var emnapiAWMT = {
10099
const worker = onCreateWorker({ type: 'async-work', name: 'emnapi-async-worker' })
101100
const p = PThread.loadWasmModuleToWorker(worker)
102101
emnapiAWMT.addListener(worker)
102+
if (typeof emnapiPluginCtx.emnapiTSFN !== 'undefined') {
103+
emnapiPluginCtx.emnapiTSFN.addListener(worker)
104+
}
103105
promises.push(p.then(() => {
104106
if (typeof worker.unref === 'function') {
105107
worker.unref()

‎packages/emnapi/src/core/async.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ function emnapiGetWorkerByPthreadPtr (pthreadPtr: number): any {
2323
return worker
2424
}
2525

26+
/** @__sig vp */
27+
export function _emnapi_worker_ref (pthreadPtr: number): void {
28+
if (ENVIRONMENT_IS_PTHREAD) return
29+
const worker = emnapiGetWorkerByPthreadPtr(pthreadPtr)
30+
if (worker && typeof worker.ref === 'function') {
31+
worker.ref()
32+
}
33+
}
34+
2635
/** @__sig vp */
2736
export function _emnapi_worker_unref (pthreadPtr: number): void {
2837
if (ENVIRONMENT_IS_PTHREAD) return

‎packages/emnapi/src/emnapi_internal.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
#ifndef EMNAPI_INTERNAL_H
2+
#define EMNAPI_INTERNAL_H
3+
14
#include "emnapi.h"
25

36
#if defined(__EMSCRIPTEN__) || defined(__wasi__)
@@ -103,6 +106,12 @@ EMNAPI_INTERNAL_EXTERN int _emnapi_is_main_browser_thread();
103106
EMNAPI_INTERNAL_EXTERN int _emnapi_is_main_runtime_thread();
104107
EMNAPI_INTERNAL_EXTERN double _emnapi_get_now();
105108
EMNAPI_INTERNAL_EXTERN void _emnapi_unwind();
109+
EMNAPI_INTERNAL_EXTERN void _emnapi_set_immediate(void (*callback)(void*), void* data);
110+
EMNAPI_INTERNAL_EXTERN napi_status _emnapi_create_function(const char* utf8name,
111+
size_t length,
112+
napi_callback cb,
113+
void* data,
114+
napi_value* result);
106115

107116
#if defined(__EMSCRIPTEN_PTHREADS__) || defined(_REENTRANT)
108117
#define EMNAPI_HAVE_THREADS 1
@@ -182,3 +191,5 @@ void _emnapi_env_check_gc_access(napi_env env);
182191
EXTERN_C_END
183192

184193
#include "js_native_api_internal.h"
194+
195+
#endif

0 commit comments

Comments
 (0)