From c0e6be7722b0a372cfc6a7ee79d2331008f4bab8 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 00:09:32 -0300 Subject: [PATCH 1/2] fix: typed array arguments leak and type confusion - A typed array or ArrayBuffer passed to an object parameter was wrapped in a direct Java buffer pinned by a JNI global ref that was never deleted, so neither the buffer nor the JS memory could be collected, and ART aborted once 51200 had been passed. The object link alone keeps the JS memory alive while Java holds the buffer. The nio method ids and native byte order are now looked up once, and no local refs leak per conversion. - A typed array passed to a resolved array parameter (e.g. ByteBuffer.wrap(byte[]) or String.format's Object[]) was handed to Java as a direct buffer. It is now copied into a new array of the parameter's type: bit for bit when the element layouts match, element by element like a plain JS array otherwise. - Plain JS arrays converted to primitive arrays no longer go through a stack array sized by the JS array length. --- test-app/app/src/main/assets/app/mainpage.js | 1 + .../assets/app/tests/testTypedArrayArgs.js | 167 ++++++++ .../java/com/tns/tests/TypedArrayArgs.java | 69 ++++ .../runtime/src/main/cpp/JsArgConverter.cpp | 391 ++++++++++++------ .../runtime/src/main/cpp/JsArgConverter.h | 13 +- .../src/main/cpp/JsArgToArrayConverter.cpp | 97 +---- 6 files changed, 520 insertions(+), 218 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js create mode 100644 test-app/app/src/main/java/com/tns/tests/TypedArrayArgs.java diff --git a/test-app/app/src/main/assets/app/mainpage.js b/test-app/app/src/main/assets/app/mainpage.js index d39ad51f0..b847f9aa6 100644 --- a/test-app/app/src/main/assets/app/mainpage.js +++ b/test-app/app/src/main/assets/app/mainpage.js @@ -65,6 +65,7 @@ require("./tests/java-array-test"); require("./tests/field-access-test"); require("./tests/byte-buffer-test"); require("./tests/shared-array-buffer-test"); +require("./tests/testTypedArrayArgs"); require("./tests/dex-interface-implementation"); require("./tests/testClassForNameDiscovery"); require("./tests/testInterfaceImplementation"); diff --git a/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js b/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js new file mode 100644 index 000000000..54d73b859 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js @@ -0,0 +1,167 @@ +describe("Typed arrays passed to Java parameters", function () { + var T = com.tns.tests.TypedArrayArgs; + + function toArray(javaArray) { + var result = []; + for (var i = 0; i < javaArray.length; i++) { + result.push(javaArray[i]); + } + return result; + } + + it("copies a Uint8Array into a byte[] parameter", function () { + var u8 = new Uint8Array([1, 2, 127, 128, 255]); + var bb = java.nio.ByteBuffer.wrap(u8); + + expect(bb.isDirect()).toBe(false); + expect(bb.capacity()).toBe(5); + expect(toArray(bb.array())).toEqual([1, 2, 127, -128, -1]); + + u8[0] = 9; + expect(bb.array()[0]).toBe(1); + bb.array()[1] = 42; + expect(u8[1]).toBe(2); + }); + + it("copies only the bytes a view covers", function () { + var all = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]); + expect(toArray(T.echoBytes(new Uint8Array(all.buffer, 2, 3)))).toEqual([2, 3, 4]); + expect(toArray(T.echoBytes(all.subarray(6)))).toEqual([6, 7]); + expect(toArray(T.echoInts(new Int32Array(new ArrayBuffer(16), 4, 2)))).toEqual([0, 0]); + }); + + it("copies byte-sized arrays, ArrayBuffers and DataViews into byte[]", function () { + var bytes = [1, -1, 127, -128]; + var buffer = new Int8Array(bytes).buffer; + + expect(toArray(T.echoBytes(new Int8Array(bytes)))).toEqual(bytes); + expect(toArray(T.echoBytes(new Uint8ClampedArray([1, 255, 127, 128])))).toEqual(bytes); + expect(toArray(T.echoBytes(buffer))).toEqual(bytes); + expect(toArray(T.echoBytes(new DataView(buffer, 1, 2)))).toEqual([-1, 127]); + + var shared = new SharedArrayBuffer(2); + new Uint8Array(shared).set([5, 6]); + expect(toArray(T.echoBytes(shared))).toEqual([5, 6]); + }); + + it("copies each typed array into the primitive array of its element type", function () { + expect(toArray(T.echoShorts(new Int16Array([-32768, 1, 32767])))).toEqual([-32768, 1, 32767]); + expect(toArray(T.echoShorts(new Uint16Array([65535, 2])))).toEqual([-1, 2]); + expect(toArray(T.echoChars(new Uint16Array([0x41, 0x7a])))).toEqual(["A", "z"]); + expect(toArray(T.echoInts(new Int32Array([-2147483648, 3, 2147483647])))).toEqual([-2147483648, 3, 2147483647]); + expect(toArray(T.echoInts(new Uint32Array([4294967295, 4])))).toEqual([-1, 4]); + expect(toArray(T.echoLongs(new BigInt64Array([-5n, 6n])))).toEqual([-5, 6]); + expect(toArray(T.echoLongs(new BigUint64Array([7n])))).toEqual([7]); + expect(toArray(T.echoFloats(new Float32Array([0.5, -1.25])))).toEqual([0.5, -1.25]); + expect(toArray(T.echoDoubles(new Float64Array([0.1, -1e300])))).toEqual([0.1, -1e300]); + }); + + it("converts other element types the way a plain array with the same values converts", function () { + var values = [1.9, -2.5, 300, 70000, -70000]; + expect(toArray(T.echoInts(new Float64Array(values)))).toEqual(toArray(T.echoInts(values))); + expect(toArray(T.echoBytes(new Float64Array(values)))).toEqual(toArray(T.echoBytes(values))); + expect(toArray(T.echoShorts(new Int32Array([70000, -1])))).toEqual(toArray(T.echoShorts([70000, -1]))); + expect(toArray(T.echoBytes(new Int32Array([1, 255, 256, -1])))).toEqual([1, -1, 0, -1]); + expect(toArray(T.echoLongs(new Int32Array([-5, 7])))).toEqual([-5, 7]); + expect(toArray(T.echoDoubles(new Float32Array([0.5, 1.25])))).toEqual([0.5, 1.25]); + expect(toArray(T.echoBooleans(new Uint8Array([0, 1, 2])))).toEqual([false, true, true]); + }); + + it("converts a typed array for an object array parameter the way a plain array converts", function () { + expect(T.describeObjects(new Int32Array([1, 2]))).toBe(T.describeObjects([1, 2])); + expect(T.describeObjects(new Float64Array([0.5]))).toBe(T.describeObjects([0.5])); + }); + + it("rejects element types that have no conversion to the parameter's", function () { + expect(function () { T.echoInts(new BigInt64Array(2)); }).toThrow(); + expect(function () { T.echoInts(new ArrayBuffer(8)); }).toThrow(); + expect(function () { T.echoInts(new DataView(new ArrayBuffer(8))); }).toThrow(); + expect(function () { T.describeObjects(new ArrayBuffer(8)); }).toThrow(); + expect(function () { T.describeObjects(new BigInt64Array(1)); }).toThrow(); + expect(function () { T.countRows(new Int32Array(4)); }).toThrow(); + }); + + it("copies empty typed arrays", function () { + expect(T.echoBytes(new Uint8Array(0)).length).toBe(0); + expect(T.echoDoubles(new Float64Array(0)).length).toBe(0); + expect(T.echoBytes(new ArrayBuffer(0)).length).toBe(0); + expect(T.echoInts([]).length).toBe(0); + }); + + it("still passes a typed array to an object parameter as a buffer over its memory", function () { + var ints = new Int32Array(4); + expect(T.capacityOf(ints)).toBe(4); + expect(T.capacityOf(new Uint8Array(3))).toBe(3); + // A typed array that is already linked to a buffer is still copied into byte[]. + var bytes = new Uint8Array([3, 4]); + expect(T.capacityOf(bytes)).toBe(2); + expect(toArray(T.echoBytes(bytes))).toEqual([3, 4]); + }); + + // Each of these used to pin a JNI global reference, and ART aborts the process once + // its global reference table (51200 entries) overflows. + it("passes more typed arrays to object parameters than fit in the global reference table", function () { + var count = 60000; + var total = 0; + for (var i = 0; i < count; i++) { + total += T.capacityOf(new Uint8Array(16)); + } + expect(total).toBe(count * 16); + }); + + it("passes more typed arrays as Object[] elements than fit in the global reference table", function () { + var count = 60000; + var items = new Array(count); + for (var i = 0; i < count; i++) { + items[i] = new Float32Array(2); + } + expect(T.totalCapacity(items)).toBe(count * 2); + }); + + it("lets Java collect the buffer once JS drops the typed array", function (done) { + var weak = (function () { + return new java.lang.ref.WeakReference(new Uint8Array(1024)); + })(); + var attempts = 0; + function collect() { + gc(); + java.lang.System.gc(); + if (weak.get() === null) { + done(); + } else if (++attempts >= 40) { + expect(weak.get()).toBeNull(); + done(); + } else { + setTimeout(collect, 50); + } + } + setTimeout(collect, 0); + }); + + it("keeps the typed array's memory alive while Java holds the buffer", function () { + var holder = new com.tns.tests.ByteBufferHolder(); + (function () { + var bytes = new Uint8Array(4096); + bytes[0] = 42; + bytes[4095] = 7; + holder.hold(bytes); + })(); + for (var i = 0; i < 3; i++) { + gc(); + java.lang.System.gc(); + } + expect(holder.get(0)).toBe(42); + expect(holder.get(4095)).toBe(7); + }); + + it("converts a JS array too large for the thread's stack", function () { + var count = 4 * 1024 * 1024; + var values = new Array(count); + var expected = 0; + for (var i = 0; i < count; i++) { + values[i] = i & 0xff; + expected += i & 0xff; + } + expect(T.sumInts(values)).toBe(expected); + }); +}); diff --git a/test-app/app/src/main/java/com/tns/tests/TypedArrayArgs.java b/test-app/app/src/main/java/com/tns/tests/TypedArrayArgs.java new file mode 100644 index 000000000..1429ec581 --- /dev/null +++ b/test-app/app/src/main/java/com/tns/tests/TypedArrayArgs.java @@ -0,0 +1,69 @@ +package com.tns.tests; + +import java.nio.Buffer; +import java.util.Arrays; + +// Each method has a single overload of its arity, so calls skip overload resolution and +// convert straight to the declared parameter type. +public class TypedArrayArgs { + public static byte[] echoBytes(byte[] a) { + return a; + } + + public static short[] echoShorts(short[] a) { + return a; + } + + public static char[] echoChars(char[] a) { + return a; + } + + public static int[] echoInts(int[] a) { + return a; + } + + public static long[] echoLongs(long[] a) { + return a; + } + + public static float[] echoFloats(float[] a) { + return a; + } + + public static double[] echoDoubles(double[] a) { + return a; + } + + public static boolean[] echoBooleans(boolean[] a) { + return a; + } + + public static long sumInts(int[] a) { + long sum = 0; + for (int v : a) { + sum += v; + } + return sum; + } + + public static String describeObjects(Object[] items) { + return Arrays.toString(items); + } + + public static int countRows(int[][] rows) { + return rows.length; + } + + public static int capacityOf(Object o) { + return o instanceof Buffer ? ((Buffer) o).capacity() : -1; + } + + // Elements of an Object[] go through the single-value converter. + public static long totalCapacity(Object[] items) { + long total = 0; + for (Object o : items) { + total += capacityOf(o); + } + return total; + } +} diff --git a/test-app/runtime/src/main/cpp/JsArgConverter.cpp b/test-app/runtime/src/main/cpp/JsArgConverter.cpp index bd4cafbca..25918dd68 100644 --- a/test-app/runtime/src/main/cpp/JsArgConverter.cpp +++ b/test-app/runtime/src/main/cpp/JsArgConverter.cpp @@ -8,12 +8,87 @@ #include "NativeScriptException.h" #include "Runtime.h" #include "V8GlobalHelpers.h" +#include "NativeScriptAssert.h" #include +#include using namespace v8; using namespace std; using namespace tns; +namespace { + struct NioBuffers { + jmethodID order; + jobject nativeOrder; + // Indexed by BufferCastType; no view method for Byte. + jmethodID asView[6]; + jclass viewClass[6]; + }; + + // Method ids and global refs are valid VM-wide, so one set serves every runtime. + const NioBuffers &GetNioBuffers() { + static const NioBuffers nio = [] { + JEnv env; + NioBuffers n{}; + jclass byteBuffer = env.FindClass("java/nio/ByteBuffer"); + jclass byteOrder = env.FindClass("java/nio/ByteOrder"); + NS_CHECK(byteBuffer != nullptr && byteOrder != nullptr); + + n.order = env.GetMethodID(byteBuffer, "order", "(Ljava/nio/ByteOrder;)Ljava/nio/ByteBuffer;"); + auto nativeOrderId = env.GetStaticMethodID(byteOrder, "nativeOrder", "()Ljava/nio/ByteOrder;"); + JniLocalRef nativeOrder(env.CallStaticObjectMethodA(byteOrder, nativeOrderId, nullptr)); + n.nativeOrder = env.NewGlobalRef(nativeOrder); + + struct View { + BufferCastType type; + const char *method; + const char *className; + }; + const View views[] = { + {BufferCastType::Byte, nullptr, "java/nio/ByteBuffer"}, + {BufferCastType::Short, "asShortBuffer", "java/nio/ShortBuffer"}, + {BufferCastType::Int, "asIntBuffer", "java/nio/IntBuffer"}, + {BufferCastType::Long, "asLongBuffer", "java/nio/LongBuffer"}, + {BufferCastType::Float, "asFloatBuffer", "java/nio/FloatBuffer"}, + {BufferCastType::Double, "asDoubleBuffer", "java/nio/DoubleBuffer"}, + }; + for (const auto &view : views) { + auto i = static_cast(view.type); + n.viewClass[i] = env.FindClass(view.className); + NS_CHECK(n.viewClass[i] != nullptr); + if (view.method != nullptr) { + n.asView[i] = env.GetMethodID(byteBuffer, view.method, + string("()L") + view.className + ";"); + } + } + return n; + }(); + return nio; + } + + // The element size when the typed array's elements have the Java element type's + // layout, else 0. + size_t SameLayoutElementSize(const Local &array, char javaElementType) { + switch (javaElementType) { + case 'B': + return array->IsInt8Array() || array->IsUint8Array() || array->IsUint8ClampedArray() ? 1 : 0; + case 'S': + case 'C': + return array->IsInt16Array() || array->IsUint16Array() ? 2 : 0; + case 'I': + return array->IsInt32Array() || array->IsUint32Array() ? 4 : 0; + case 'J': + return array->IsBigInt64Array() || array->IsBigUint64Array() ? 8 : 0; + case 'F': + return array->IsFloat32Array() ? 4 : 0; + case 'D': + return array->IsFloat64Array() ? 8 : 0; + default: + return 0; + } + } +} + JsArgConverter::JsArgConverter(const Local &caller, const v8::FunctionCallbackInfo &args, const string &methodSignature, MetadataEntry *entry) @@ -114,6 +189,52 @@ tns::BufferCastType JsArgConverter::GetCastType(const v8::Local &object) { + return object->IsArrayBuffer() || object->IsSharedArrayBuffer() || object->IsArrayBufferView(); +} + +JniLocalRef JsArgConverter::LinkNewJavaBuffer(ObjectManager *objectManager, const Local &object) { + auto castType = BufferCastType::Byte; + uint8_t *data; + size_t length; + if (object->IsArrayBuffer()) { + auto buffer = object.As(); + data = static_cast(buffer->Data()); + length = buffer->ByteLength(); + } else if (object->IsSharedArrayBuffer()) { + auto buffer = object.As(); + data = static_cast(buffer->Data()); + length = buffer->ByteLength(); + } else { + // Buffer() moves an on-heap view's elements off the heap, where they stay put. + auto view = object.As(); + data = static_cast(view->Buffer()->Data()) + view->ByteOffset(); + length = view->ByteLength(); + castType = GetCastType(view); + } + + if (length > INT32_MAX) { + throw NativeScriptException("Cannot pass a buffer larger than 2 GiB to Java"); + } + + const auto &nio = GetNioBuffers(); + auto type = static_cast(castType); + + JEnv env; + JniLocalRef bytes(env.NewDirectByteBuffer(data, static_cast(length))); + JniLocalRef ordered(env.CallObjectMethod(bytes, nio.order, nio.nativeOrder)); + JniLocalRef view; + if (nio.asView[type] != nullptr) { + view = JniLocalRef(env.CallObjectMethodA(ordered, nio.asView[type], nullptr)); + } + JniLocalRef &buffer = view.IsNull() ? ordered : view; + + int id = objectManager->GetOrCreateObjectId(buffer); + objectManager->Link(object, id, nio.viewClass[type]); + + return std::move(buffer); +} + bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { bool success = false; @@ -153,7 +274,7 @@ bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { } if (success) { - success = ConvertJavaScriptArray(jsArr, index); + success = ConvertJavaScriptArray(jsArr, static_cast(jsArr->Length()), index); } } @@ -270,102 +391,20 @@ bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { case CastType::None: - obj = objectManager->GetJavaObjectByJsObject(jsObject); - - if (obj.IsNull() && (jsObject->IsTypedArray() || jsObject->IsArrayBuffer() || - jsObject->IsArrayBufferView() || - jsObject->IsSharedArrayBuffer())) { - - BufferCastType bufferCastType = tns::BufferCastType::Byte; - shared_ptr store; - size_t offset = 0; - size_t length; - uint8_t *data = nullptr; - if (jsObject->IsArrayBuffer()) { - auto array = jsObject.As(); - store = array->GetBackingStore(); - length = array->ByteLength(); - data = static_cast(store->Data()); - } else if (jsObject->IsSharedArrayBuffer()) { - auto array = jsObject.As(); - store = array->GetBackingStore(); - length = array->ByteLength(); - data = static_cast(store->Data()); - } else if (jsObject->IsArrayBufferView()) { - auto array = jsObject.As(); - offset = array->ByteOffset(); - length = array->ByteLength(); - store = array->Buffer()->GetBackingStore(); - data = static_cast(store->Data()) + offset; - bufferCastType = JsArgConverter::GetCastType(array); - } else { - auto array = jsObject.As(); - offset = array->ByteOffset(); - store = array->Buffer()->GetBackingStore(); - length = array->ByteLength(); - data = static_cast(store->Data()) + offset; - bufferCastType = JsArgConverter::GetCastType(array); + if (typeSignature[0] == '[' && IsArrayBufferLike(jsObject)) { + success = ConvertArrayBufferToJavaArray(jsObject, index); + if (!success) { + auto typeName = ArgConverter::ConvertToString(jsObject->GetConstructorName()); + snprintf(buff, sizeof(buff), "Cannot convert %s to %s at index %d", + typeName.c_str(), typeSignature.c_str(), index); } + break; + } - auto directBuffer = env.NewDirectByteBuffer( - data, - length); - - auto directBufferClazz = env.GetObjectClass(directBuffer); - - auto byteOrderId = env.GetMethodID(directBufferClazz, "order", - "(Ljava/nio/ByteOrder;)Ljava/nio/ByteBuffer;"); - - auto byteOrderClazz = env.FindClass("java/nio/ByteOrder"); - - auto byteOrderEnumId = env.GetStaticMethodID(byteOrderClazz, "nativeOrder", - "()Ljava/nio/ByteOrder;"); - - auto nativeByteOrder = env.CallStaticObjectMethodA(byteOrderClazz, - byteOrderEnumId, - nullptr); - - directBuffer = env.CallObjectMethod(directBuffer, byteOrderId, nativeByteOrder); - - jobject buffer; - - if (bufferCastType == BufferCastType::Short) { - - auto id = env.GetMethodID(directBufferClazz, "asShortBuffer", - "()Ljava/nio/ShortBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Int) { - - auto id = env.GetMethodID(directBufferClazz, "asIntBuffer", - "()Ljava/nio/IntBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Long) { - - auto id = env.GetMethodID(directBufferClazz, "asLongBuffer", - "()Ljava/nio/LongBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Float) { - - auto id = env.GetMethodID(directBufferClazz, "asFloatBuffer", - "()Ljava/nio/FloatBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Double) { - - auto id = env.GetMethodID(directBufferClazz, "asDoubleBuffer", - "()Ljava/nio/DoubleBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else { - buffer = directBuffer; - } - - buffer = env.NewGlobalRef(buffer); - - int id = objectManager->GetOrCreateObjectId(buffer); - auto clazz = env.GetObjectClass(buffer); - - objectManager->Link(jsObject, id, clazz); + obj = objectManager->GetJavaObjectByJsObject(jsObject); - obj = objectManager->GetJavaObjectByJsObject(jsObject); + if (obj.IsNull() && IsArrayBufferLike(jsObject)) { + obj = LinkNewJavaBuffer(objectManager, jsObject); } V8GetPrivateValue(m_isolate, jsObject, V8StringConstants::GetNullNodePrivate(m_isolate), @@ -508,13 +547,11 @@ bool JsArgConverter::ConvertJavaScriptString(const Local &jsValue, int in return true; } -bool JsArgConverter::ConvertJavaScriptArray(const Local &jsArr, int index) { +bool JsArgConverter::ConvertJavaScriptArray(const Local &source, jsize arrLength, int index) { bool success = true; jarray arr = nullptr; - jsize arrLength = jsArr->Length(); - const auto &arraySignature = m_tokens->at(index); auto context = m_isolate->GetCurrentContext(); @@ -529,106 +566,106 @@ bool JsArgConverter::ConvertJavaScriptArray(const Local &jsArr, int index switch (elementTypePrefix) { case 'Z': { arr = env.NewBooleanArray(arrLength); - jboolean bools[arrLength]; + std::vector bools(arrLength); for (jsize i = 0; i < arrLength; i++) { - bools[i] = jsArr + bools[i] = source ->Get(context, i) .ToLocalChecked() ->BooleanValue(m_isolate); } - env.SetBooleanArrayRegion((jbooleanArray) arr, 0, arrLength, bools); + env.SetBooleanArrayRegion((jbooleanArray) arr, 0, arrLength, bools.data()); break; } case 'B': { arr = env.NewByteArray(arrLength); - jbyte bytes[arrLength]; + std::vector bytes(arrLength); for (jsize i = 0; i < arrLength; i++) { - bytes[i] = jsArr + bytes[i] = source ->Get(context, i) .ToLocalChecked() ->Int32Value(context) .ToChecked(); } - env.SetByteArrayRegion((jbyteArray) arr, 0, arrLength, bytes); + env.SetByteArrayRegion((jbyteArray) arr, 0, arrLength, bytes.data()); break; } case 'C': { arr = env.NewCharArray(arrLength); - jchar chars[arrLength]; + std::vector chars(arrLength); for (jsize i = 0; i < arrLength; i++) { - String::Utf8Value utf8(m_isolate, jsArr->Get(context, i).ToLocalChecked()->ToString( + String::Utf8Value utf8(m_isolate, source->Get(context, i).ToLocalChecked()->ToString( context).ToLocalChecked()); JniLocalRef s(env.NewString((jchar *) *utf8, 1)); const char *singleChar = env.GetStringUTFChars(s, nullptr); chars[i] = *singleChar; env.ReleaseStringUTFChars(s, singleChar); } - env.SetCharArrayRegion((jcharArray) arr, 0, arrLength, chars); + env.SetCharArrayRegion((jcharArray) arr, 0, arrLength, chars.data()); break; } case 'S': { arr = env.NewShortArray(arrLength); - jshort shorts[arrLength]; + std::vector shorts(arrLength); for (jsize i = 0; i < arrLength; i++) { - shorts[i] = jsArr + shorts[i] = source ->Get(context, i) .ToLocalChecked() ->Int32Value(context) .ToChecked(); } - env.SetShortArrayRegion((jshortArray) arr, 0, arrLength, shorts); + env.SetShortArrayRegion((jshortArray) arr, 0, arrLength, shorts.data()); break; } case 'I': { arr = env.NewIntArray(arrLength); - jint ints[arrLength]; + std::vector ints(arrLength); for (jsize i = 0; i < arrLength; i++) { - ints[i] = jsArr + ints[i] = source ->Get(context, i) .ToLocalChecked() ->Int32Value(context) .ToChecked(); } - env.SetIntArrayRegion((jintArray) arr, 0, arrLength, ints); + env.SetIntArrayRegion((jintArray) arr, 0, arrLength, ints.data()); break; } case 'J': { arr = env.NewLongArray(arrLength); - jlong longs[arrLength]; + std::vector longs(arrLength); for (jsize i = 0; i < arrLength; i++) { - longs[i] = jsArr + longs[i] = source ->Get(context, i) .ToLocalChecked() ->NumberValue(context) .ToChecked(); } - env.SetLongArrayRegion((jlongArray) arr, 0, arrLength, longs); + env.SetLongArrayRegion((jlongArray) arr, 0, arrLength, longs.data()); break; } case 'F': { arr = env.NewFloatArray(arrLength); - jfloat floats[arrLength]; + std::vector floats(arrLength); for (jsize i = 0; i < arrLength; i++) { - floats[i] = jsArr + floats[i] = source ->Get(context, i) .ToLocalChecked() ->NumberValue(context) .ToChecked(); } - env.SetFloatArrayRegion((jfloatArray) arr, 0, arrLength, floats); + env.SetFloatArrayRegion((jfloatArray) arr, 0, arrLength, floats.data()); break; } case 'D': { arr = env.NewDoubleArray(arrLength); - jdouble doubles[arrLength]; + std::vector doubles(arrLength); for (jsize i = 0; i < arrLength; i++) { - doubles[i] = jsArr + doubles[i] = source ->Get(context, i) .ToLocalChecked() ->NumberValue(context) .ToChecked(); } - env.SetDoubleArrayRegion((jdoubleArray) arr, 0, arrLength, doubles); + env.SetDoubleArrayRegion((jdoubleArray) arr, 0, arrLength, doubles.data()); break; } case 'L': @@ -636,7 +673,7 @@ bool JsArgConverter::ConvertJavaScriptArray(const Local &jsArr, int index elementClass = env.FindClass(strippedClassName); arr = env.NewObjectArray(arrLength, elementClass, nullptr); for (int i = 0; i < arrLength; i++) { - auto v = jsArr->Get(context, i).ToLocalChecked(); + auto v = source->Get(context, i).ToLocalChecked(); JsArgToArrayConverter c(context, v, false, (int) Type::Null); jobject o = c.GetConvertedArg(); env.SetObjectArrayElement((jobjectArray) arr, i, o); @@ -654,6 +691,116 @@ bool JsArgConverter::ConvertJavaScriptArray(const Local &jsArr, int index return success; } +bool JsArgConverter::ConvertArrayBufferToJavaArray(const Local &source, int index) { + const char elementType = m_tokens->at(index)[1]; + + const uint8_t *bytes; + size_t byteLength; + size_t elementSize; + std::vector onHeapCopy; + + if (source->IsArrayBuffer() || source->IsSharedArrayBuffer()) { + if (elementType != 'B') { + return false; + } + elementSize = 1; + if (source->IsArrayBuffer()) { + auto buffer = source.As(); + bytes = static_cast(buffer->Data()); + byteLength = buffer->ByteLength(); + } else { + auto buffer = source.As(); + bytes = static_cast(buffer->Data()); + byteLength = buffer->ByteLength(); + } + } else { + auto view = source.As(); + if (view->IsDataView()) { + if (elementType != 'B') { + return false; + } + elementSize = 1; + } else { + auto array = view.As(); + elementSize = SameLayoutElementSize(array, elementType); + if (elementSize == 0) { + if (array->IsBigInt64Array() || array->IsBigUint64Array()) { + return false; + } + // Element by element, exactly as a plain array holding the same numbers. + return ConvertJavaScriptArray(source, static_cast(array->Length()), index); + } + } + + byteLength = view->ByteLength(); + if (view->HasBuffer()) { + bytes = static_cast(view->Buffer()->Data()) + view->ByteOffset(); + } else { + onHeapCopy.resize(byteLength); + view->CopyContents(onHeapCopy.data(), byteLength); + bytes = onHeapCopy.data(); + } + } + + size_t count = byteLength / elementSize; + if (count > INT32_MAX) { + return false; + } + auto length = static_cast(count); + + JEnv env; + jarray arr; + switch (elementType) { + case 'B': + arr = env.NewByteArray(length); + if (length > 0) { + env.SetByteArrayRegion((jbyteArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'S': + arr = env.NewShortArray(length); + if (length > 0) { + env.SetShortArrayRegion((jshortArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'C': + arr = env.NewCharArray(length); + if (length > 0) { + env.SetCharArrayRegion((jcharArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'I': + arr = env.NewIntArray(length); + if (length > 0) { + env.SetIntArrayRegion((jintArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'J': + arr = env.NewLongArray(length); + if (length > 0) { + env.SetLongArrayRegion((jlongArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'F': + arr = env.NewFloatArray(length); + if (length > 0) { + env.SetFloatArrayRegion((jfloatArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + case 'D': + arr = env.NewDoubleArray(length); + if (length > 0) { + env.SetDoubleArrayRegion((jdoubleArray) arr, 0, length, reinterpret_cast(bytes)); + } + break; + default: + return false; + } + + SetConvertedObject(index, arr); + return true; +} + template bool JsArgConverter::ConvertFromCastFunctionObject(T value, int index) { bool success = false; diff --git a/test-app/runtime/src/main/cpp/JsArgConverter.h b/test-app/runtime/src/main/cpp/JsArgConverter.h index d3954a018..59b3b49f6 100644 --- a/test-app/runtime/src/main/cpp/JsArgConverter.h +++ b/test-app/runtime/src/main/cpp/JsArgConverter.h @@ -4,11 +4,14 @@ #include #include #include "JEnv.h" +#include "JniLocalRef.h" #include "v8.h" #include "MetadataEntry.h" namespace tns { + class ObjectManager; + enum class BufferCastType { Byte, Short, @@ -54,11 +57,19 @@ namespace tns { static BufferCastType GetCastType(const v8::Local& view); + static bool IsArrayBufferLike(const v8::Local& object); + + // A direct Java buffer over the object's memory, linked to the object so that the + // object, and with it the memory, lives as long as the buffer. + static JniLocalRef LinkNewJavaBuffer(ObjectManager* objectManager, const v8::Local& object); + private: bool ConvertArg(const v8::Local& jsArg, int index); - bool ConvertJavaScriptArray(const v8::Local& jsArr, int index); + bool ConvertJavaScriptArray(const v8::Local& source, jsize length, int index); + + bool ConvertArrayBufferToJavaArray(const v8::Local& source, int index); bool ConvertJavaScriptNumber(const v8::Local& jsValue, int index); diff --git a/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp b/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp index 8f6800259..3ac24144a 100644 --- a/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp +++ b/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp @@ -253,103 +253,10 @@ bool JsArgToArrayConverter::ConvertArg(Local context, const LocalGetJavaObjectByJsObject(jsObj); - - if (obj.IsNull() && (jsObj->IsTypedArray() || jsObj->IsArrayBuffer() || - jsObj->IsArrayBufferView() || jsObj->IsSharedArrayBuffer())) { - - BufferCastType bufferCastType = tns::BufferCastType::Byte; - shared_ptr store; - size_t offset = 0; - size_t length; - uint8_t *data = nullptr; - if (jsObj->IsArrayBuffer()) { - auto array = jsObj.As(); - store = array->GetBackingStore(); - length = array->ByteLength(); - data = static_cast(store->Data()); - } else if (jsObj->IsSharedArrayBuffer()) { - auto array = jsObj.As(); - store = array->GetBackingStore(); - length = array->ByteLength(); - data = static_cast(store->Data()); - } else if (jsObj->IsArrayBufferView()) { - auto array = jsObj.As(); - - offset = array->ByteOffset(); - length = array->ByteLength(); - store = array->Buffer()->GetBackingStore(); - bufferCastType = JsArgConverter::GetCastType(array); - data = static_cast(store->Data()) + offset; - } else { - auto array = jsObj.As(); - offset = array->ByteOffset(); - store = array->Buffer()->GetBackingStore(); - length = array->ByteLength(); - bufferCastType = JsArgConverter::GetCastType(array); - data = static_cast(store->Data()) + offset; - } - - auto directBuffer = env.NewDirectByteBuffer( - data, - length); - - auto directBufferClazz = env.GetObjectClass(directBuffer); - - auto byteOrderId = env.GetMethodID(directBufferClazz, "order", - "(Ljava/nio/ByteOrder;)Ljava/nio/ByteBuffer;"); - - auto byteOrderClazz = env.FindClass("java/nio/ByteOrder"); - - auto byteOrderEnumId = env.GetStaticMethodID(byteOrderClazz, "nativeOrder", - "()Ljava/nio/ByteOrder;"); - - auto nativeByteOrder = env.CallStaticObjectMethodA(byteOrderClazz, - byteOrderEnumId, - nullptr); - - directBuffer = env.CallObjectMethod(directBuffer, byteOrderId, nativeByteOrder); - - jobject buffer; - - if (bufferCastType == BufferCastType::Short) { - - auto id = env.GetMethodID(directBufferClazz, "asShortBuffer", - "()Ljava/nio/ShortBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Int) { - - auto id = env.GetMethodID(directBufferClazz, "asIntBuffer", - "()Ljava/nio/IntBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Long) { - - auto id = env.GetMethodID(directBufferClazz, "asLongBuffer", - "()Ljava/nio/LongBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Float) { - - auto id = env.GetMethodID(directBufferClazz, "asFloatBuffer", - "()Ljava/nio/FloatBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else if (bufferCastType == BufferCastType::Double) { - - auto id = env.GetMethodID(directBufferClazz, "asDoubleBuffer", - "()Ljava/nio/DoubleBuffer;"); - buffer = env.CallObjectMethodA(directBuffer, id, nullptr); - } else { - buffer = directBuffer; - } - - buffer = env.NewGlobalRef(buffer); - - int id = objectManager->GetOrCreateObjectId(buffer); - auto clazz = env.GetObjectClass(buffer); - objectManager->Link(jsObj, id, clazz); - - obj = objectManager->GetJavaObjectByJsObject(jsObj); + if (obj.IsNull() && JsArgConverter::IsArrayBufferLike(jsObj)) { + obj = JsArgConverter::LinkNewJavaBuffer(objectManager, jsObj); } - V8GetPrivateValue(isolate, jsObj, V8StringConstants::GetNullNodePrivate(isolate), castValue); From 8524b936fd06a40eb6f5e58292241329a834297b Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 01:01:58 -0300 Subject: [PATCH 2/2] fix: relink collected buffers; throw for unconvertible array elements - A typed array whose Java buffer Java had collected (after JS dropped it, Java handed the buffer back and then let it go) failed every later call with "Attempt to use cleared object reference". Once Java confirms the buffer is gone, the stale link is released on both sides and a new buffer is linked. A link whose Java object is still alive is never released. - Converting a JS array to a Java array aborted the process when an element could not be read or converted: a BigInt or Symbol element, a throwing getter or valueOf. It now throws "Cannot convert array to at index N: ". A BigInt passed to a scalar long parameter, already rejected, now says so instead of reporting an empty message. - A JS array or typed array longer than a Java array can be fails with the usual conversion error instead of aborting. --- .../assets/app/tests/testTypedArrayArgs.js | 60 ++++++ .../runtime/src/main/cpp/JsArgConverter.cpp | 187 ++++++++++-------- .../runtime/src/main/cpp/JsArgConverter.h | 6 +- .../src/main/cpp/JsArgToArrayConverter.cpp | 8 +- .../runtime/src/main/cpp/ObjectManager.cpp | 42 ++++ test-app/runtime/src/main/cpp/ObjectManager.h | 8 + .../src/main/java/com/tns/Runtime.java | 10 + 7 files changed, 237 insertions(+), 84 deletions(-) diff --git a/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js b/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js index 54d73b859..44de6ed33 100644 --- a/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js +++ b/test-app/app/src/main/assets/app/tests/testTypedArrayArgs.js @@ -154,6 +154,66 @@ describe("Typed arrays passed to Java parameters", function () { expect(holder.get(4095)).toBe(7); }); + it("links a new buffer when Java has collected the one a reused typed array had", function () { + var list = new java.util.ArrayList(); + (function () { + list.add(new Uint8Array([9, 8, 7])); + })(); + // Collecting the typed array makes Java hold its buffer weakly from then on. + for (var i = 0; i < 3; i++) { + gc(); + java.lang.System.gc(); + } + var bytes = list.remove(0); + expect(bytes instanceof Uint8Array).toBe(true); + for (var i = 0; i < 3; i++) { + java.lang.System.gc(); + } + + list.add(bytes); + expect(list.get(0)).toBe(bytes); + expect(T.totalCapacity(list.toArray())).toBe(3); + var copy = java.nio.ByteBuffer.allocate(3); + copy.put(bytes); + expect(toArray(copy.array())).toEqual([9, 8, 7]); + }); + + function conversionError(call) { + try { + call(); + } catch (e) { + return e.message; + } + return "no error"; + } + + it("throws for array elements that cannot convert to the element type", function () { + expect(conversionError(function () { T.echoLongs([1n, 2n]); })) + .toContain("Cannot convert array to [J at index 0"); + expect(conversionError(function () { T.echoInts([1, 2n]); })).toContain("BigInt"); + expect(conversionError(function () { T.echoInts([1, Symbol("s")]); })).toContain("Symbol"); + expect(conversionError(function () { T.echoChars([Symbol("s")]); })).toContain("Symbol"); + expect(conversionError(function () { + T.echoDoubles([{ valueOf: function () { throw new Error("no value"); } }]); + })).toContain("no value"); + + var withThrowingGetter = [1, 2]; + Object.defineProperty(withThrowingGetter, 1, { get: function () { throw new Error("no element"); } }); + expect(conversionError(function () { T.echoInts(withThrowingGetter); })).toContain("no element"); + expect(conversionError(function () { T.describeObjects(withThrowingGetter); })).toContain("no element"); + }); + + it("throws for a BigInt passed to a long parameter, as in an array", function () { + expect(conversionError(function () { java.lang.Long.toHexString(5n); })) + .toContain("Cannot convert bigint to J at index 0"); + }); + + it("throws for an array longer than a Java array can be", function () { + var sparse = []; + sparse.length = 2147483648; + expect(conversionError(function () { T.echoInts(sparse); })).toContain("Cannot convert array to [I"); + }); + it("converts a JS array too large for the thread's stack", function () { var count = 4 * 1024 * 1024; var values = new Array(count); diff --git a/test-app/runtime/src/main/cpp/JsArgConverter.cpp b/test-app/runtime/src/main/cpp/JsArgConverter.cpp index 25918dd68..7be683046 100644 --- a/test-app/runtime/src/main/cpp/JsArgConverter.cpp +++ b/test-app/runtime/src/main/cpp/JsArgConverter.cpp @@ -235,10 +235,18 @@ JniLocalRef JsArgConverter::LinkNewJavaBuffer(ObjectManager *objectManager, cons return std::move(buffer); } +JniLocalRef JsArgConverter::GetOrLinkJavaBuffer(ObjectManager *objectManager, const Local &object) { + auto buffer = objectManager->GetLiveJavaObjectByJsObject(object); + if (buffer.IsNull()) { + buffer = LinkNewJavaBuffer(objectManager, object); + } + return buffer; +} + bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { bool success = false; - char buff[1024]; + char buff[1024] = ""; const auto &typeSignature = m_tokens->at(index); @@ -274,7 +282,16 @@ bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { } if (success) { - success = ConvertJavaScriptArray(jsArr, static_cast(jsArr->Length()), index); + TryCatch tc(m_isolate); + success = ConvertJavaScriptArray(jsArr, jsArr->Length(), index); + if (!success && tc.HasCaught()) { + trapThrew = true; + auto context = m_isolate->GetCurrentContext(); + string reason = ArgConverter::ConvertToString( + tc.Exception()->ToString(context).FromMaybe(Local())); + snprintf(buff, sizeof(buff), "Cannot convert array to %s at index %d: %s", + typeSignature.c_str(), index, reason.c_str()); + } } } @@ -401,11 +418,9 @@ bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { break; } - obj = objectManager->GetJavaObjectByJsObject(jsObject); - - if (obj.IsNull() && IsArrayBufferLike(jsObject)) { - obj = LinkNewJavaBuffer(objectManager, jsObject); - } + obj = IsArrayBufferLike(jsObject) + ? GetOrLinkJavaBuffer(objectManager, jsObject) + : objectManager->GetJavaObjectByJsObject(jsObject); V8GetPrivateValue(m_isolate, jsObject, V8StringConstants::GetNullNodePrivate(m_isolate), castValue); @@ -435,6 +450,9 @@ bool JsArgConverter::ConvertArg(const Local &jsArg, int index) { } else { SetConvertedObject(index, nullptr); success = false; + auto typeName = ArgConverter::ConvertToString(arg->TypeOf(m_isolate)); + snprintf(buff, sizeof(buff), "Cannot convert %s to %s at index %d", typeName.c_str(), + typeSignature.c_str(), index); } if (!success) { @@ -547,148 +565,161 @@ bool JsArgConverter::ConvertJavaScriptString(const Local &jsValue, int in return true; } -bool JsArgConverter::ConvertJavaScriptArray(const Local &source, jsize arrLength, int index) { - bool success = true; - - jarray arr = nullptr; +bool JsArgConverter::ConvertJavaScriptArray(const Local &source, size_t length, int index) { + // Java arrays are indexed by jint. + if (length > INT32_MAX) { + return false; + } + auto arrLength = static_cast(length); const auto &arraySignature = m_tokens->at(index); auto context = m_isolate->GetCurrentContext(); - string elementType = arraySignature.substr(1); - - const char elementTypePrefix = elementType[0]; + // Each read fails, with a JS exception scheduled, when an element getter or a + // conversion throws (a BigInt or Symbol element, a throwing valueOf). + auto elementAt = [&](jsize i, Local &element) { + return source->Get(context, i).ToLocal(&element); + }; + auto int32At = [&](jsize i, int32_t &value) { + Local element; + return elementAt(i, element) && element->Int32Value(context).To(&value); + }; + auto numberAt = [&](jsize i, double &value) { + Local element; + return elementAt(i, element) && element->NumberValue(context).To(&value); + }; - jclass elementClass; - string strippedClassName; + jarray arr = nullptr; JEnv env; - switch (elementTypePrefix) { + switch (arraySignature[1]) { case 'Z': { - arr = env.NewBooleanArray(arrLength); std::vector bools(arrLength); for (jsize i = 0; i < arrLength; i++) { - bools[i] = source - ->Get(context, i) - .ToLocalChecked() - ->BooleanValue(m_isolate); + Local element; + if (!elementAt(i, element)) { + return false; + } + bools[i] = element->BooleanValue(m_isolate); } + arr = env.NewBooleanArray(arrLength); env.SetBooleanArrayRegion((jbooleanArray) arr, 0, arrLength, bools.data()); break; } case 'B': { - arr = env.NewByteArray(arrLength); std::vector bytes(arrLength); for (jsize i = 0; i < arrLength; i++) { - bytes[i] = source - ->Get(context, i) - .ToLocalChecked() - ->Int32Value(context) - .ToChecked(); + int32_t value; + if (!int32At(i, value)) { + return false; + } + bytes[i] = (jbyte) value; } + arr = env.NewByteArray(arrLength); env.SetByteArrayRegion((jbyteArray) arr, 0, arrLength, bytes.data()); break; } case 'C': { - arr = env.NewCharArray(arrLength); std::vector chars(arrLength); for (jsize i = 0; i < arrLength; i++) { - String::Utf8Value utf8(m_isolate, source->Get(context, i).ToLocalChecked()->ToString( - context).ToLocalChecked()); + Local element; + Local str; + if (!elementAt(i, element) || !element->ToString(context).ToLocal(&str)) { + return false; + } + String::Utf8Value utf8(m_isolate, str); JniLocalRef s(env.NewString((jchar *) *utf8, 1)); const char *singleChar = env.GetStringUTFChars(s, nullptr); chars[i] = *singleChar; env.ReleaseStringUTFChars(s, singleChar); } + arr = env.NewCharArray(arrLength); env.SetCharArrayRegion((jcharArray) arr, 0, arrLength, chars.data()); break; } case 'S': { - arr = env.NewShortArray(arrLength); std::vector shorts(arrLength); for (jsize i = 0; i < arrLength; i++) { - shorts[i] = source - ->Get(context, i) - .ToLocalChecked() - ->Int32Value(context) - .ToChecked(); + int32_t value; + if (!int32At(i, value)) { + return false; + } + shorts[i] = (jshort) value; } + arr = env.NewShortArray(arrLength); env.SetShortArrayRegion((jshortArray) arr, 0, arrLength, shorts.data()); break; } case 'I': { - arr = env.NewIntArray(arrLength); std::vector ints(arrLength); for (jsize i = 0; i < arrLength; i++) { - ints[i] = source - ->Get(context, i) - .ToLocalChecked() - ->Int32Value(context) - .ToChecked(); + int32_t value; + if (!int32At(i, value)) { + return false; + } + ints[i] = value; } + arr = env.NewIntArray(arrLength); env.SetIntArrayRegion((jintArray) arr, 0, arrLength, ints.data()); break; } case 'J': { - arr = env.NewLongArray(arrLength); std::vector longs(arrLength); for (jsize i = 0; i < arrLength; i++) { - longs[i] = source - ->Get(context, i) - .ToLocalChecked() - ->NumberValue(context) - .ToChecked(); + double value; + if (!numberAt(i, value)) { + return false; + } + longs[i] = (jlong) value; } + arr = env.NewLongArray(arrLength); env.SetLongArrayRegion((jlongArray) arr, 0, arrLength, longs.data()); break; } case 'F': { - arr = env.NewFloatArray(arrLength); std::vector floats(arrLength); for (jsize i = 0; i < arrLength; i++) { - floats[i] = source - ->Get(context, i) - .ToLocalChecked() - ->NumberValue(context) - .ToChecked(); + double value; + if (!numberAt(i, value)) { + return false; + } + floats[i] = (jfloat) value; } + arr = env.NewFloatArray(arrLength); env.SetFloatArrayRegion((jfloatArray) arr, 0, arrLength, floats.data()); break; } case 'D': { - arr = env.NewDoubleArray(arrLength); std::vector doubles(arrLength); for (jsize i = 0; i < arrLength; i++) { - doubles[i] = source - ->Get(context, i) - .ToLocalChecked() - ->NumberValue(context) - .ToChecked(); + if (!numberAt(i, doubles[i])) { + return false; + } } + arr = env.NewDoubleArray(arrLength); env.SetDoubleArrayRegion((jdoubleArray) arr, 0, arrLength, doubles.data()); break; } - case 'L': - strippedClassName = elementType.substr(1, elementType.length() - 2); - elementClass = env.FindClass(strippedClassName); - arr = env.NewObjectArray(arrLength, elementClass, nullptr); - for (int i = 0; i < arrLength; i++) { - auto v = source->Get(context, i).ToLocalChecked(); - JsArgToArrayConverter c(context, v, false, (int) Type::Null); - jobject o = c.GetConvertedArg(); - env.SetObjectArrayElement((jobjectArray) arr, i, o); + case 'L': { + auto elementClass = env.FindClass(arraySignature.substr(2, arraySignature.length() - 3)); + JniLocalRef objects(env.NewObjectArray(arrLength, elementClass, nullptr)); + for (jsize i = 0; i < arrLength; i++) { + Local element; + if (!elementAt(i, element)) { + return false; + } + JsArgToArrayConverter c(context, element, false, (int) Type::Null); + env.SetObjectArrayElement((jobjectArray) objects, i, c.GetConvertedArg()); } + arr = (jarray) objects.Move(); break; + } default: - success = false; - break; - } - - if (success) { - SetConvertedObject(index, arr); + return false; } - return success; + SetConvertedObject(index, arr); + return true; } bool JsArgConverter::ConvertArrayBufferToJavaArray(const Local &source, int index) { @@ -728,7 +759,7 @@ bool JsArgConverter::ConvertArrayBufferToJavaArray(const Local &source, return false; } // Element by element, exactly as a plain array holding the same numbers. - return ConvertJavaScriptArray(source, static_cast(array->Length()), index); + return ConvertJavaScriptArray(source, array->Length(), index); } } diff --git a/test-app/runtime/src/main/cpp/JsArgConverter.h b/test-app/runtime/src/main/cpp/JsArgConverter.h index 59b3b49f6..6c70b0bc6 100644 --- a/test-app/runtime/src/main/cpp/JsArgConverter.h +++ b/test-app/runtime/src/main/cpp/JsArgConverter.h @@ -63,11 +63,15 @@ namespace tns { // object, and with it the memory, lives as long as the buffer. static JniLocalRef LinkNewJavaBuffer(ObjectManager* objectManager, const v8::Local& object); + // The Java buffer linked to an array buffer or view, linking a new one when there is + // none or Java has collected it. + static JniLocalRef GetOrLinkJavaBuffer(ObjectManager* objectManager, const v8::Local& object); + private: bool ConvertArg(const v8::Local& jsArg, int index); - bool ConvertJavaScriptArray(const v8::Local& source, jsize length, int index); + bool ConvertJavaScriptArray(const v8::Local& source, size_t length, int index); bool ConvertArrayBufferToJavaArray(const v8::Local& source, int index); diff --git a/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp b/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp index 3ac24144a..5c31d357f 100644 --- a/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp +++ b/test-app/runtime/src/main/cpp/JsArgToArrayConverter.cpp @@ -251,11 +251,9 @@ bool JsArgToArrayConverter::ConvertArg(Local context, const LocalGetJavaObjectByJsObject(jsObj); - - if (obj.IsNull() && JsArgConverter::IsArrayBufferLike(jsObj)) { - obj = JsArgConverter::LinkNewJavaBuffer(objectManager, jsObj); - } + obj = JsArgConverter::IsArrayBufferLike(jsObj) + ? JsArgConverter::GetOrLinkJavaBuffer(objectManager, jsObj) + : objectManager->GetJavaObjectByJsObject(jsObj); V8GetPrivateValue(isolate, jsObj, V8StringConstants::GetNullNodePrivate(isolate), castValue); diff --git a/test-app/runtime/src/main/cpp/ObjectManager.cpp b/test-app/runtime/src/main/cpp/ObjectManager.cpp index 164401677..71395e499 100644 --- a/test-app/runtime/src/main/cpp/ObjectManager.cpp +++ b/test-app/runtime/src/main/cpp/ObjectManager.cpp @@ -75,6 +75,10 @@ void ObjectManager::InitializeJNI() { env.GetMethodID(runtimeClass, "releaseNativeCounterpart", "(I)V"); NS_CHECK(RELEASE_NATIVE_INSTANCE_METHOD_ID != nullptr); + RELEASE_IF_COLLECTED_METHOD_ID = + env.GetMethodID(runtimeClass, "releaseIfCollected", "(I)Z"); + NS_CHECK(RELEASE_IF_COLLECTED_METHOD_ID != nullptr); + JAVA_LANG_CLASS = env.FindClass("java/lang/Class"); NS_CHECK(JAVA_LANG_CLASS != nullptr); @@ -128,6 +132,43 @@ JniLocalRef ObjectManager::GetJavaObjectByJsObject( return JniLocalRef(); } +JniLocalRef ObjectManager::GetLiveJavaObjectByJsObject( + const Local& object) { + JSInstanceInfo* jsInstanceInfo = GetJSInstanceInfo(object); + if (jsInstanceInfo == nullptr) { + return JniLocalRef(); + } + auto javaObjectID = jsInstanceInfo->javaObjectID; + + // Only a collected object may be relinked: releasing the link of a live one would + // leave it pointing at memory nothing keeps alive any more. + JniLocalRef javaObject; + try { + javaObject = GetJavaObjectByJsObject(object); + } catch (const NativeScriptException&) { + if (!ReleaseIfCollected(javaObjectID)) { + throw; + } + return JniLocalRef(); + } + if (javaObject.IsNull() && !ReleaseIfCollected(javaObjectID)) { + throw NativeScriptException("Failed to get Java object by ID. id=" + + std::to_string(javaObjectID)); + } + return javaObject; +} + +bool ObjectManager::ReleaseIfCollected(int javaObjectID) { + JEnv env; + AssertIsolateLocked(); + if (env.CallBooleanMethod(m_javaRuntimeObject, RELEASE_IF_COLLECTED_METHOD_ID, + javaObjectID) != JNI_TRUE) { + return false; + } + ReleaseCollectedWrapper(javaObjectID); + return true; +} + ObjectManager::JSInstanceInfo* ObjectManager::GetJSInstanceInfo( const Local& object) { if (object->IsProxy()) { @@ -896,3 +937,4 @@ jmethodID ObjectManager::FIND_JAVA_OBJECT_ID_METHOD_ID = nullptr; jmethodID ObjectManager::MAKE_INSTANCE_STRONG_METHOD_ID = nullptr; jmethodID ObjectManager::PROCESS_FINALIZED_BATCH_METHOD_ID = nullptr; jmethodID ObjectManager::RELEASE_NATIVE_INSTANCE_METHOD_ID = nullptr; +jmethodID ObjectManager::RELEASE_IF_COLLECTED_METHOD_ID = nullptr; diff --git a/test-app/runtime/src/main/cpp/ObjectManager.h b/test-app/runtime/src/main/cpp/ObjectManager.h index 5f94f1385..469a18898 100644 --- a/test-app/runtime/src/main/cpp/ObjectManager.h +++ b/test-app/runtime/src/main/cpp/ObjectManager.h @@ -43,6 +43,10 @@ class ObjectManager { JniLocalRef GetJavaObjectByJsObject(const v8::Local& object); + // As GetJavaObjectByJsObject, but a link whose Java object was already collected is + // released and reported as no link, so the JS object can be linked again. + JniLocalRef GetLiveJavaObjectByJsObject(const v8::Local& object); + void UpdateCache(int objectID, jobject obj); jclass GetJavaClass(const v8::Local& instance); @@ -246,6 +250,9 @@ class ObjectManager { void ReleaseCollectedWrapper(int javaObjectID); + // Drops id's registrations on both sides if Java has collected its object. + bool ReleaseIfCollected(int javaObjectID); + // Ids whose JS wrapper was collected, waiting for one JNI call that weakens // (first collection) or liveness-checks (later ones) their Java objects. std::vector m_pendingWeaken; @@ -297,6 +304,7 @@ class ObjectManager { static jmethodID PROCESS_FINALIZED_BATCH_METHOD_ID; static jmethodID RELEASE_NATIVE_INSTANCE_METHOD_ID; + static jmethodID RELEASE_IF_COLLECTED_METHOD_ID; v8::Persistent* m_poJsWrapperFunc; v8::Persistent m_jsWrapperTemplate; diff --git a/test-app/runtime/src/main/java/com/tns/Runtime.java b/test-app/runtime/src/main/java/com/tns/Runtime.java index e2bf3189d..940a1997f 100644 --- a/test-app/runtime/src/main/java/com/tns/Runtime.java +++ b/test-app/runtime/src/main/java/com/tns/Runtime.java @@ -1148,6 +1148,16 @@ private Integer getJavaObjectID(Object obj) { return id == ObjectTable.NO_ID ? null : id; } + // Removes id's entry if its object was collected; returns whether it was. + @RuntimeCallable + private boolean releaseIfCollected(int javaObjectID) { + if (objectTable.get(javaObjectID) != null) { + return false; + } + objectTable.remove(javaObjectID); + return true; + } + @RuntimeCallable private int findJavaObjectID(Object obj) { return objectTable.findId(obj);