From b11c1487ef74b85cb35bac1bf1c4f37a2adf272c Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:20:52 -0300 Subject: [PATCH 1/2] test(runtime): use one function as a block and as a function pointer A function wrapped in interop.FunctionReference and then marshalled as a block loses its FunctionReference state, so passing it as a C function pointer afterwards asserts. Marshalling it as a block after interop.FunctionReference also stops reusing the block it cached. Cover both orders, interop.handleof across them, a function pointer argument that is not a FunctionReference, collection of such functions, and a worker torn down while native code holds the block. --- .../app/tests/FunctionReferenceBlockTests.js | 122 ++++++++++++++++++ .../app/tests/functionReferenceBlockWorker.js | 17 +++ TestRunner/app/tests/index.js | 1 + 3 files changed, 140 insertions(+) create mode 100644 TestRunner/app/tests/FunctionReferenceBlockTests.js create mode 100644 TestRunner/app/tests/functionReferenceBlockWorker.js diff --git a/TestRunner/app/tests/FunctionReferenceBlockTests.js b/TestRunner/app/tests/FunctionReferenceBlockTests.js new file mode 100644 index 00000000..4ea0dc1f --- /dev/null +++ b/TestRunner/app/tests/FunctionReferenceBlockTests.js @@ -0,0 +1,122 @@ +// A function can be marshalled both as a block and, once wrapped in +// interop.FunctionReference, as a C function pointer. Neither use may take over +// the state the other keeps on the function. +describe("Function used as a block and as a function pointer", function () { + function square(x) { + return x * x; + } + + afterEach(function () { + TNSClearOutput(); + }); + + it("is called through a function pointer after being a block", function () { + var blockCalls = []; + var fn = new interop.FunctionReference(function (x) { + blockCalls.push(x); + return square(x); + }); + + TNSTestNativeCallbacks.repeatPausingAfter(2, fn); + expect(blockCalls).toEqual([0, 1]); + + functionWithSimpleFunctionPointer(fn); + expect(TNSGetOutput()).toBe("4"); + }); + + it("is called as a block after being a function pointer", function () { + var blockCalls = []; + var fn = new interop.FunctionReference(function (x) { + blockCalls.push(x); + return square(x); + }); + + functionWithSimpleFunctionPointer(fn); + expect(TNSGetOutput()).toBe("4"); + + TNSTestNativeCallbacks.repeatPausingAfter(2, fn); + expect(blockCalls).toEqual([2, 0, 1]); + + TNSClearOutput(); + functionWithSimpleFunctionPointer(fn); + expect(TNSGetOutput()).toBe("4"); + }); + + it("keeps its cached block across interop.FunctionReference", function () { + var fn = function () {}; + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 1000); + var block = interop.handleof(fn).toNumber(); + + expect(new interop.FunctionReference(fn)).toBe(fn); + expect(interop.handleof(fn).toNumber()).toBe(block); + + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 1000); + expect(interop.handleof(fn).toNumber()).toBe(block); + }); + + it("is reported by interop.handleof as its function pointer once it has one", function () { + var fn = function () {}; + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 1000); + var block = interop.handleof(fn).toNumber(); + + new interop.FunctionReference(fn); + functionWithSimpleFunctionPointer(fn); + var trampoline = interop.handleof(fn).toNumber(); + expect(trampoline).not.toBe(block); + + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 1000); + expect(interop.handleof(fn).toNumber()).toBe(trampoline); + }); + + it("throws when passed as a function pointer without interop.FunctionReference", function () { + var fn = function () {}; + TNSTestNativeCallbacks.repeatPausingAfter(1, fn); + expect(function () { + functionWithSimpleFunctionPointer(fn); + }).toThrowError(/FunctionReference/); + expect(function () { + functionWithSimpleFunctionPointer(function () {}); + }).toThrowError(/FunctionReference/); + }); + + it("is collected once its blocks are gone", function (done) { + for (var i = 0; i < 50; i++) { + var fn = new interop.FunctionReference(function () {}); + if (i % 2) { + TNSTestNativeCallbacks.repeatPausingAfter(1, fn); + } else { + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 5); + } + } + setTimeout(function () { + __collect(); + __collect(); + done(); + }, 50); + }); + + describe("in a worker", function () { + var originalTimeout; + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; + }); + afterEach(function () { + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + it("is torn down while native code still holds its block", function (done) { + var worker = new Worker("./functionReferenceBlockWorker.js"); + worker.onmessage = function (msg) { + expect(msg.data).toEqual({ pointerOutput: "4", blockKept: true }); + worker.terminate(); + setTimeout(done, 600); + }; + worker.onerror = function (e) { + expect(String(e && e.message ? e.message : e)).toBe(""); + done(); + }; + worker.postMessage(0); + }); + }); +}); diff --git a/TestRunner/app/tests/functionReferenceBlockWorker.js b/TestRunner/app/tests/functionReferenceBlockWorker.js new file mode 100644 index 00000000..fe86a234 --- /dev/null +++ b/TestRunner/app/tests/functionReferenceBlockWorker.js @@ -0,0 +1,17 @@ +// Leaves native code holding a block built from a function that is also an +// interop.FunctionReference with a trampoline, past this worker's teardown. +onmessage = function () { + var fn = new interop.FunctionReference(function (x) { + return x * x; + }); + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 300); + var block = interop.handleof(fn).toNumber(); + + TNSClearOutput(); + functionWithSimpleFunctionPointer(fn); + var pointerOutput = String(TNSGetOutput()); + TNSClearOutput(); + + TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 300); + postMessage({ pointerOutput: pointerOutput, blockKept: block !== 0 }); +}; diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index f2f9e6fe..bcd27313 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -131,6 +131,7 @@ require("./ApiTests"); require("./NsRuntimeTests"); require("./GCFinalizerTests"); require("./BlockCacheRaceTests"); +require("./FunctionReferenceBlockTests"); require("./WorkerConcurrentStartupTests"); require("./WorkerOptionsTests"); require("./WorkerResourceLimitsTests"); From 06b5c7e6df325c928227a08187d52d6157e11147 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:28:05 -0300 Subject: [PATCH 2/2] fix(runtime): keep a JS function's block apart from its own wrapper A JS function marshalled as a block cached its BlockWrapper in the same slot that interop.FunctionReference keeps its wrapper in, so each use evicted the other: the FunctionReferenceWrapper leaked and a later function pointer marshal asserted, or the block cache stopped hitting. The block cache now lives in its own private slot on the function. The JSBlock still owns its BlockWrapper and only clears that slot while it still holds that wrapper, so ObjectManager never sees a JS block's wrapper. interop.handleof reports a FunctionReference's trampoline once it has one, and the live block otherwise. A second interop.FunctionReference on the same function keeps the existing wrapper and trampoline. A function pointer argument that is not a pointer, native function pointer or FunctionReference throws instead of asserting. Fixes #503 --- NativeScript/runtime/FunctionReference.cpp | 10 ++++- NativeScript/runtime/Helpers.h | 7 +++ NativeScript/runtime/Helpers.mm | 40 +++++++++++++++++ NativeScript/runtime/Interop.mm | 45 ++++++++++--------- NativeScript/runtime/InteropTypes.mm | 22 +++++---- .../app/tests/FunctionReferenceBlockTests.js | 9 ++++ .../app/tests/blockTeardownReleaseWorker.js | 4 +- 7 files changed, 106 insertions(+), 31 deletions(-) diff --git a/NativeScript/runtime/FunctionReference.cpp b/NativeScript/runtime/FunctionReference.cpp index b7210eca..226a9e96 100644 --- a/NativeScript/runtime/FunctionReference.cpp +++ b/NativeScript/runtime/FunctionReference.cpp @@ -62,11 +62,19 @@ void FunctionReference::FunctionReferenceConstructorCallback( tns::Assert(info[0]->IsFunction(), isolate); Local arg = info[0].As(); + info.GetReturnValue().Set(arg); + + // The existing wrapper may already hold the trampoline native code calls. + BaseDataWrapper* existing = tns::GetValue(isolate, arg); + if (existing != nullptr && + existing->Type() == WrapperType::FunctionReference) { + return; + } + std::shared_ptr> poArg = ObjectManager::Register(context, arg); FunctionReferenceWrapper* wrapper = new FunctionReferenceWrapper(poArg); tns::SetValue(isolate, arg, wrapper); - info.GetReturnValue().Set(arg); } } // namespace tns diff --git a/NativeScript/runtime/Helpers.h b/NativeScript/runtime/Helpers.h index 5b48ade5..0f9718f0 100644 --- a/NativeScript/runtime/Helpers.h +++ b/NativeScript/runtime/Helpers.h @@ -285,6 +285,13 @@ void SetReleasedObjectPolicy(ReleasedObjectPolicy policy); BaseDataWrapper* GetValueOrReport(v8::Isolate* isolate, const v8::Local& val, const char* operation); void DeleteValue(v8::Isolate* isolate, const v8::Local& val); +// The block a JS function was last marshalled as (see Interop::JSBlock, which +// owns the wrapper). Kept apart from GetValue's slot so the function's own +// wrapper and its block never evict each other. +void SetJSBlockWrapper(v8::Isolate* isolate, const v8::Local& fn, + BlockWrapper* wrapper); +BlockWrapper* GetJSBlockWrapper(v8::Isolate* isolate, const v8::Local& val); +void DeleteJSBlockWrapper(v8::Isolate* isolate, const v8::Local& val); bool DeleteWrapperIfUnused(v8::Isolate* isolate, const v8::Local& obj, BaseDataWrapper* value); std::vector> ArgsToVector(const v8::FunctionCallbackInfo& info); diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm index ef35ddaa..494f6c5d 100644 --- a/NativeScript/runtime/Helpers.mm +++ b/NativeScript/runtime/Helpers.mm @@ -527,6 +527,46 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { tns::Assert(success, isolate); } +namespace { + +constexpr const char* kJSBlockKey = "jsBlock"; + +} // namespace + +void tns::SetJSBlockWrapper(Isolate* isolate, const Local& fn, + BlockWrapper* wrapper) { + Local ext = External::New(isolate, wrapper, v8::kExternalPointerTypeTagDefault); + tns::SetPrivateValue(fn, tns::ToV8String(isolate, kJSBlockKey), ext); +} + +tns::BlockWrapper* tns::GetJSBlockWrapper(Isolate* isolate, const Local& val) { + if (val.IsEmpty() || !val->IsFunction()) { + return nullptr; + } + + Local prop = tns::GetPrivateValue(val.As(), tns::ToV8String(isolate, kJSBlockKey)); + if (prop.IsEmpty() || !prop->IsExternal()) { + return nullptr; + } + + return static_cast(prop.As()->Value(v8::kExternalPointerTypeTagDefault)); +} + +void tns::DeleteJSBlockWrapper(Isolate* isolate, const Local& val) { + if (val.IsEmpty() || !val->IsFunction()) { + return; + } + + Local obj = val.As(); + Local context; + bool success = obj->GetCreationContext(isolate).ToLocal(&context); + tns::Assert(success, isolate); + Local privateKey = Private::ForApi(isolate, tns::ToV8String(isolate, kJSBlockKey)); + + success = obj->DeletePrivate(context, privateKey).FromMaybe(false); + tns::Assert(success, isolate); +} + std::vector> tns::ArgsToVector(const FunctionCallbackInfo& info) { std::vector> args; args.reserve(info.Length()); diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 998742e9..8cb6906b 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -30,6 +30,10 @@ static_cast(kUint64AllBitsSet << 53) + 1; // -9007199254740991 (-(2^53-1)) static constexpr int64_t kMaxSafeInteger = -kMinSafeInteger; // 9007199254740991 (2^53-1) +static constexpr const char* kNotAFunctionPointer = + "A function pointer argument takes an interop.Pointer, a native function pointer or a " + "function wrapped in interop.FunctionReference."; + Interop::JSBlock::JSBlockDescriptor Interop::JSBlock::kJSBlockDescriptor = { .reserved = 0, .size = sizeof(JSBlock), @@ -61,8 +65,8 @@ if (!callback.IsEmpty() && callback->IsObject()) { // The slot may hold another wrapper by now; only our own is // cleared from it. - if (tns::GetValue(isolate, callback) == blockWrapper) { - tns::DeleteValue(isolate, callback); + if (tns::GetJSBlockWrapper(isolate, callback) == blockWrapper) { + tns::DeleteJSBlockWrapper(isolate, callback); } } // Unconditional: an already-detached callback still owns its @@ -518,7 +522,9 @@ inline bool isBool() { } else if (argHelper.isObject() && typeEncoding->type == BinaryTypeEncodingType::FunctionPointerEncoding) { BaseDataWrapper* wrapper = tns::GetValue(isolate, arg.As()); - tns::Assert(wrapper != nullptr, isolate); + if (wrapper == nullptr) { + throw NativeScriptException(kNotAFunctionPointer); + } if (wrapper->Type() == WrapperType::Pointer) { PointerWrapper* pointerWrapper = static_cast(wrapper); void* data = pointerWrapper->Data(); @@ -528,7 +534,6 @@ inline bool isBool() { void* data = functionWrapper->Data(); Interop::SetValue(dest, data); } else if (wrapper->Type() == WrapperType::FunctionReference) { - tns::Assert(wrapper != nullptr && wrapper->Type() == WrapperType::FunctionReference, isolate); FunctionReferenceWrapper* funcWrapper = static_cast(wrapper); const TypeEncoding* functionTypeEncoding = typeEncoding->details.functionPointer.signature.first(); @@ -557,28 +562,28 @@ inline bool isBool() { Interop::SetValue(dest, functionPointer); } else { - tns::Assert(false, isolate); + throw NativeScriptException(kNotAFunctionPointer); } } else if (arg->IsFunction() && typeEncoding->type == BinaryTypeEncodingType::BlockEncoding) { const TypeEncoding* blockTypeEncoding = typeEncoding->details.block.signature.first(); int argsCount = typeEncoding->details.block.signature.count - 1; CFTypeRef blockPtr = nullptr; + // The callee takes the block at +0 and copies it if it needs to keep it, + // so the reference that keeps it alive across the call must be balanced: + // the JSBlock dispose helper owns the ffi closure and the callback wrapper + // and only runs once the last reference goes away. BaseDataWrapper* baseWrapper = tns::GetValue(isolate, arg); - if (baseWrapper != nullptr && baseWrapper->Type() == WrapperType::Block) { - BlockWrapper* wrapper = static_cast(baseWrapper); - // The callee takes the block at +0 and copies it if it needs to keep it, - // so the reference that keeps it alive across the call must be balanced: - // the JSBlock dispose helper owns the ffi closure and the callback wrapper - // and only runs once the last reference goes away. - if (wrapper->OwnsBlock()) { - // A native block; the wrapper's own Block_copy keeps it alive. - blockPtr = CFAutorelease(Block_copy(wrapper->Block())); - } else if (TryRetainJSBlock(static_cast(wrapper->Block()))) { - // Reading the block is safe even when its last release raced ahead: - // until the isolate's teardown closes its gate, dispose clears this - // slot under the Locker this thread holds, before libclosure frees - // the block. + if (baseWrapper != nullptr && baseWrapper->Type() == WrapperType::Block && + static_cast(baseWrapper)->OwnsBlock()) { + // A native block; the wrapper's own Block_copy keeps it alive. + blockPtr = CFAutorelease(Block_copy(static_cast(baseWrapper)->Block())); + } else if (BlockWrapper* wrapper = tns::GetJSBlockWrapper(isolate, arg)) { + // Reading the block is safe even when its last release raced ahead: + // until the isolate's teardown closes its gate, dispose clears this + // slot under the Locker this thread holds, before libclosure frees + // the block. + if (TryRetainJSBlock(static_cast(wrapper->Block()))) { blockPtr = CFAutorelease(wrapper->Block()); } } @@ -592,7 +597,7 @@ inline bool isBool() { BlockWrapper* wrapper = new BlockWrapper((void*)blockPtr, blockTypeEncoding, false); reinterpret_cast((void*)blockPtr)->blockWrapper = wrapper; - tns::SetValue(isolate, arg.As(), wrapper); + tns::SetJSBlockWrapper(isolate, arg.As(), wrapper); } Interop::SetValue(dest, blockPtr); diff --git a/NativeScript/runtime/InteropTypes.mm b/NativeScript/runtime/InteropTypes.mm index a149d055..f51af9a5 100644 --- a/NativeScript/runtime/InteropTypes.mm +++ b/NativeScript/runtime/InteropTypes.mm @@ -589,6 +589,10 @@ new PrimitiveDataWrapper(sizeof(void*), } } + if (size == 0 && tns::GetJSBlockWrapper(isolate, arg) != nullptr) { + size = sizeof(void*); + } + if (size == 0) { throw NativeScriptException("Unknown type"); } else { @@ -696,20 +700,22 @@ new PrimitiveDataWrapper(sizeof(void*), if (blockWrapper->OwnsBlock()) { return Pointer::NewInstance(context, blockWrapper->Block()); } - // A JS function does not keep its block alive: only native - // references do. A live block is kept for the rest of the turn, - // like one passed to a native call; a dying one is no handle. - JSBlock* block = static_cast(blockWrapper->Block()); - if (TryRetainJSBlock(block)) { - CFAutorelease(block); - return Pointer::NewInstance(context, block); - } break; } default: break; } } + if (BlockWrapper* blockWrapper = tns::GetJSBlockWrapper(isolate, obj)) { + // A JS function does not keep its block alive: only native + // references do. A live block is kept for the rest of the turn, + // like one passed to a native call; a dying one is no handle. + JSBlock* block = static_cast(blockWrapper->Block()); + if (TryRetainJSBlock(block)) { + CFAutorelease(block); + return Pointer::NewInstance(context, block); + } + } } } else if (value->IsNull()) { return v8::Null(isolate); diff --git a/TestRunner/app/tests/FunctionReferenceBlockTests.js b/TestRunner/app/tests/FunctionReferenceBlockTests.js index 4ea0dc1f..f8de0771 100644 --- a/TestRunner/app/tests/FunctionReferenceBlockTests.js +++ b/TestRunner/app/tests/FunctionReferenceBlockTests.js @@ -42,6 +42,15 @@ describe("Function used as a block and as a function pointer", function () { expect(TNSGetOutput()).toBe("4"); }); + it("keeps its function pointer across a second interop.FunctionReference", function () { + var fn = new interop.FunctionReference(square); + functionWithSimpleFunctionPointer(fn); + var trampoline = interop.handleof(fn).toNumber(); + + expect(new interop.FunctionReference(fn)).toBe(fn); + expect(interop.handleof(fn).toNumber()).toBe(trampoline); + }); + it("keeps its cached block across interop.FunctionReference", function () { var fn = function () {}; TNSTestNativeCallbacks.keepBlockForMilliseconds(fn, 1000); diff --git a/TestRunner/app/tests/blockTeardownReleaseWorker.js b/TestRunner/app/tests/blockTeardownReleaseWorker.js index c60e22a5..4e14fbf8 100644 --- a/TestRunner/app/tests/blockTeardownReleaseWorker.js +++ b/TestRunner/app/tests/blockTeardownReleaseWorker.js @@ -1,7 +1,7 @@ // Leaves a native object as the only owner of a block built from a function // that interop.FunctionReference registered first. Teardown disposes registered -// objects newest first, so it releases the block (and runs its dispose) before -// it reaches the function whose slot still points at the block's wrapper. +// objects newest first, so it releases the block (and runs its dispose, which +// frees the block's wrapper) before it disposes the function's own wrapper. onmessage = function () { var fn = function () {}; new interop.FunctionReference(fn);