From e4c8455c4f3418dd9b1bcd319e9f2890e682e356 Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Sat, 7 Nov 2026 11:40:49 -0800 Subject: [PATCH] fix(ffi/objc): forward off-thread method and function callbacks to the JS thread Methods and C function pointers implemented in JS ran on the caller's thread when invoked from another one, which aborts V8. They now take the same blocking threadsafe-function path blocks use. One threadsafe function per env replaces the one created per block, and each forwarded call runs in its own autorelease pool. --- NativeScript/ffi/objc/napi/Block.mm | 12 -- NativeScript/ffi/objc/napi/Closure.h | 6 +- NativeScript/ffi/objc/napi/Closure.mm | 161 +++++++++++++++++------ NativeScript/ffi/objc/napi/ObjCBridge.mm | 1 + 4 files changed, 126 insertions(+), 54 deletions(-) diff --git a/NativeScript/ffi/objc/napi/Block.mm b/NativeScript/ffi/objc/napi/Block.mm index a99189a6a..5a69b4284 100644 --- a/NativeScript/ffi/objc/napi/Block.mm +++ b/NativeScript/ffi/objc/napi/Block.mm @@ -436,18 +436,6 @@ id registerBlock(napi_env env, Closure* closure, napi_value callback) { napi_create_external(env, block, nullptr, nullptr, &ptrExternal); napi_set_named_property(env, callback, "__ns_native_ptr", ptrExternal); - auto bridgeState = ObjCBridgeState::InstanceData(env); - -#ifndef ENABLE_JS_RUNTIME - if (napiSupportsThreadsafeFunctions(bridgeState->self_dl)) { - napi_value workName; - napi_create_string_utf8(env, "Block", NAPI_AUTO_LENGTH, &workName); - napi_create_threadsafe_function(env, nullptr, nullptr, workName, 0, 1, nullptr, nullptr, - closure, Closure::callBlockFromMainThread, &closure->tsfn); - if (closure->tsfn) napi_unref_threadsafe_function(env, closure->tsfn); - } -#endif // ENABLE_JS_RUNTIME - cacheBlockJsFunction(env, block, callback, closure); return (id)block; diff --git a/NativeScript/ffi/objc/napi/Closure.h b/NativeScript/ffi/objc/napi/Closure.h index d81a43f4d..a6c4da819 100644 --- a/NativeScript/ffi/objc/napi/Closure.h +++ b/NativeScript/ffi/objc/napi/Closure.h @@ -44,6 +44,11 @@ bool recordNapiNativeCallbackException(napi_env env, napi_value error); bool rethrowNapiNativeCallbackException( napi_env env, NapiNativeCallbackExceptionCapture& capture); +#ifndef ENABLE_JS_RUNTIME +// Must run on the JS thread before any closure can be invoked from another one. +void initForeignThreadCallbacks(napi_env env); +#endif + class Closure { public: static void callBlockFromMainThread(napi_env env, napi_value js_cb, @@ -68,7 +73,6 @@ class Closure { bool isSetter = false; std::string propertyName; SEL selector = nullptr; - napi_threadsafe_function tsfn = nullptr; std::thread::id jsThreadId = std::this_thread::get_id(); CFRunLoopRef jsRunLoop = CFRunLoopGetCurrent(); diff --git a/NativeScript/ffi/objc/napi/Closure.mm b/NativeScript/ffi/objc/napi/Closure.mm index 4ae8e5054..2c6ca9985 100644 --- a/NativeScript/ffi/objc/napi/Closure.mm +++ b/NativeScript/ffi/objc/napi/Closure.mm @@ -22,6 +22,7 @@ #include #include #include +#include #include namespace nativescript { @@ -388,13 +389,100 @@ void invokeInNativeCallbackScope(napi_env env, ClosureInvoker invoke, ffi_cif* c memset(ret, 0, cif->rtype->size); } } + +// A closure invoked on a thread other than the JS thread is forwarded to it +// through one threadsafe function per env, and the native caller waits for JS +// to return. +struct ForeignThreadCall { + ClosureInvoker invoke; + ffi_cif* cif; + void* ret; + void** args; + void* data; + + std::mutex mutex; + std::condition_variable cv; + bool done = false; + NSException* exception = nil; +}; + +std::mutex gForeignThreadDispatchersMutex; +std::unordered_map gForeignThreadDispatchers; + +void runForeignThreadCall(napi_env env, napi_value /*jsCallback*/, void* /*context*/, void* data) { + auto call = static_cast(data); + bool ran = false; + // env is null when the env is torn down with calls still pending. + if (env != nullptr) { + // The JS thread of a Node-API host has no autorelease pool of its own. + @autoreleasepool { + @try { + call->invoke(call->cif, call->ret, call->args, call->data); + ran = true; + } @catch (NSException* exception) { + call->exception = [exception retain]; + } + } + } + if (!ran && call->cif->rtype->type != FFI_TYPE_VOID) { + memset(call->ret, 0, call->cif->rtype->size); + } + + { + std::lock_guard lock(call->mutex); + call->done = true; + } + call->cv.notify_one(); +} + +void finalizeForeignThreadDispatcher(napi_env /*env*/, void* finalizeData, void* /*hint*/) { + std::lock_guard lock(gForeignThreadDispatchersMutex); + gForeignThreadDispatchers.erase(static_cast(finalizeData)); +} + +void callFromForeignThread(napi_env env, ClosureInvoker invoke, ffi_cif* cif, void* ret, + void* args[], void* data) { + ForeignThreadCall call; + call.invoke = invoke; + call.cif = cif; + call.ret = ret; + call.args = args; + call.data = data; + + std::unique_lock lock(call.mutex); + bool posted = false; + { + std::lock_guard dispatchersLock(gForeignThreadDispatchersMutex); + auto it = gForeignThreadDispatchers.find(env); + posted = it != gForeignThreadDispatchers.end() && + napi_call_threadsafe_function(it->second, &call, napi_tsfn_blocking) == napi_ok; + } + if (!posted) { + lock.unlock(); + if (cif->rtype->type != FFI_TYPE_VOID) { + memset(ret, 0, cif->rtype->size); + } + return; + } + call.cv.wait(lock, [&call] { return call.done; }); + lock.unlock(); + + if (call.exception != nil) { + @throw [call.exception autorelease]; + } +} #endif // ENABLE_JS_RUNTIME void JSMethodCallback(ffi_cif* cif, void* ret, void* args[], void* data) { #ifdef ENABLE_JS_RUNTIME invokeJSMethodCallback(cif, ret, args, data); #else - invokeInNativeCallbackScope(((Closure*)data)->env, invokeJSMethodCallback, cif, ret, args, data); + Closure* closure = (Closure*)data; + if (std::this_thread::get_id() != closure->jsThreadId) { + callFromForeignThread(closure->env, invokeJSMethodCallback, cif, ret, args, data); + return; + } + invokeInNativeCallbackScope(closure->env, invokeJSMethodCallback, cif, ret, args, data); #endif } @@ -402,8 +490,12 @@ void JSFunctionCallback(ffi_cif* cif, void* ret, void* args[], void* data) { #ifdef ENABLE_JS_RUNTIME invokeJSFunctionCallback(cif, ret, args, data); #else - invokeInNativeCallbackScope(((Closure*)data)->env, invokeJSFunctionCallback, cif, ret, args, - data); + Closure* closure = (Closure*)data; + if (std::this_thread::get_id() != closure->jsThreadId) { + callFromForeignThread(closure->env, invokeJSFunctionCallback, cif, ret, args, data); + return; + } + invokeInNativeCallbackScope(closure->env, invokeJSFunctionCallback, cif, ret, args, data); #endif } @@ -411,10 +503,6 @@ void JSFunctionCallback(ffi_cif* cif, void* ret, void* args[], void* data) { ffi_cif* cif; void* ret; void** args; - std::mutex mutex; - std::condition_variable cv; - bool done; - bool useCondvar; }; void Closure::callBlockFromMainThread(napi_env env, napi_value js_cb, void* context, void* data) { @@ -440,24 +528,12 @@ void JSFunctionCallback(ffi_cif* cif, void* ret, void* args[], void* data) { js_execute_pending_jobs(env); } #endif - if (ctx->useCondvar) { - { - std::lock_guard lock(ctx->mutex); - ctx->done = true; - } - ctx->cv.notify_one(); - } } #ifndef ENABLE_JS_RUNTIME void invokeJSBlockCallback(ffi_cif* cif, void* ret, void* args[], void* data) { Closure* closure = (Closure*)data; - JSBlockCallContext ctx; - ctx.cif = cif; - ctx.ret = ret; - ctx.args = args; - ctx.done = false; - ctx.useCondvar = false; + JSBlockCallContext ctx{cif, ret, args}; Closure::callBlockFromMainThread(closure->env, get_ref_value(closure->env, closure->func), closure, &ctx); } @@ -478,12 +554,7 @@ void JSBlockCallback(ffi_cif* cif, void* ret, void* args[], void* data) { auto currentThreadId = std::this_thread::get_id(); - JSBlockCallContext ctx; - ctx.cif = cif; - ctx.ret = ret; - ctx.args = args; - ctx.done = false; - ctx.useCondvar = false; + JSBlockCallContext ctx{cif, ret, args}; if (currentThreadId == closure->jsThreadId || shouldInvokeCallbackOnNativeCallerThread()) { #ifdef ENABLE_JS_RUNTIME @@ -494,16 +565,7 @@ void JSBlockCallback(ffi_cif* cif, void* ret, void* args[], void* data) { #endif } else { #ifndef ENABLE_JS_RUNTIME - if (!closure->tsfn) { - assert(false && "Threadsafe functions are not supported"); - } - - ctx.useCondvar = true; - napi_acquire_threadsafe_function(closure->tsfn); - std::unique_lock lock(ctx.mutex); - napi_call_threadsafe_function(closure->tsfn, &ctx, napi_tsfn_blocking); - ctx.cv.wait(lock, [&ctx] { return ctx.done; }); - napi_release_threadsafe_function(closure->tsfn, napi_tsfn_release); + callFromForeignThread(env, invokeJSBlockCallback, cif, ret, args, closure); #else auto runloop = closure->jsRunLoop; if (runloop == nullptr) { @@ -643,11 +705,6 @@ void JSBlockCallback(ffi_cif* cif, void* ret, void* args[], void* data) { } bridgeState = nullptr; bridgeStateToken = 0; -#ifndef ENABLE_JS_RUNTIME - if (tsfn != nullptr) { - napi_release_threadsafe_function(tsfn, napi_tsfn_abort); - } -#endif // ENABLE_JS_RUNTIME if (atypes != nullptr) { free(atypes); } @@ -663,4 +720,26 @@ void JSBlockCallback(ffi_cif* cif, void* ret, void* args[], void* data) { } } +#ifndef ENABLE_JS_RUNTIME +void initForeignThreadCallbacks(napi_env env) { + auto bridgeState = ObjCBridgeState::InstanceData(env); + if (bridgeState == nullptr || !napiSupportsThreadsafeFunctions(bridgeState->self_dl)) { + return; + } + + napi_value name = nullptr; + napi_threadsafe_function tsfn = nullptr; + napi_create_string_utf8(env, "NativeScriptForeignThreadCallback", NAPI_AUTO_LENGTH, &name); + if (napi_create_threadsafe_function(env, nullptr, nullptr, name, 0, 1, env, + finalizeForeignThreadDispatcher, nullptr, + runForeignThreadCall, &tsfn) != napi_ok) { + return; + } + napi_unref_threadsafe_function(env, tsfn); + + std::lock_guard lock(gForeignThreadDispatchersMutex); + gForeignThreadDispatchers[env] = tsfn; +} +#endif // ENABLE_JS_RUNTIME + } // namespace nativescript diff --git a/NativeScript/ffi/objc/napi/ObjCBridge.mm b/NativeScript/ffi/objc/napi/ObjCBridge.mm index e3d9d42bf..281e497ad 100644 --- a/NativeScript/ffi/objc/napi/ObjCBridge.mm +++ b/NativeScript/ffi/objc/napi/ObjCBridge.mm @@ -1093,6 +1093,7 @@ NAPI_EXPORT void nativescript_init(void* _env, const char* metadata_path, ObjCBridgeState* bridgeState = new ObjCBridgeState(env, metadata_path, metadata_ptr); #ifndef ENABLE_JS_RUNTIME NativeCallbackScope::initialize(env); + initForeignThreadCallbacks(env); #endif napi_value objc;