Visitar URL original
fix: route Java->JS calls to the runtime that created the binding ins… · NativeScript/android@53f9373 · GitHub
Skip to content

Commit 53f9373

Browse files
edusperoniedusperoni
committed
fix: route Java->JS calls to the runtime that created the binding instance
Binding classes carried no runtime identity, so com.tns.Runtime had to re-derive the target of every Java->JS call from the calling thread: the currentRuntime thread-local, then the entered V8 isolate, then a linear scan of every live runtime's object maps (getObjectRuntime). That is wrong in two ways and racy in a third. Ownership was inferred from "some runtime has a wrapper for this object", but a wrapper only records that the instance crossed into that isolate. An instance created by one runtime and later passed into another exists in both maps, so a call could dispatch into an isolate that holds a wrapper but not the user's JS implementation. The scan also reads other runtimes' strongJavaObjectToID / weakJavaObjectToID from a foreign thread; with enableMultithreadedJavascript off (the default) those are plain HashMap/NativeScriptHashMap being mutated concurrently by their owning threads. Both generators now emit com.tns.NativeScriptRuntimeBound: the static binding generator in Java source, and the runtime binding generator - which dexes the proxies behind new java.lang.Runnable({...}) and is the more common of the two - in asmdex alongside the __initialized field and equals__super it already emits. initInstance stamps the id once; later registrations by other runtimes (getOrCreateJavaObjectID, when the instance crosses into another isolate) must not overwrite it, because only the creating runtime holds the JS implementation behind the generated overrides. INVALID_RUNTIME_ID is -1, so both generators seed the field rather than rely on its zero default: 0 is the main runtime's id, and JsV8InspectorClient hard-codes it as such. The dex proxies seed it as early as a constructor may touch the instance - the verifier rejects field access on an uninitialized reference, so not before the superclass constructor. A proxied method invoked from within that superclass constructor therefore still reads 0 and resolves to the main runtime, which finds no object id registered for the instance and reports that; before this change the same call reported that no runtime held the instance. A call with nowhere left to run is dropped and logged rather than executed against an unrelated runtime. These arrive on Android callbacks - lifecycle, listeners, draw - so throwing would take down the process because a worker ended; the loss stays contained to the one call. Primitive returns reuse the existing default-value substitution so the generated cast can still unbox. - callJSMethod against a terminated owner drops instead of scanning and landing on whichever runtime happens to hold a wrapper. - dispatchCallJSMethodNative ignored the result of threadScheduler.post(); a post to a quitting worker looper already returned null silently, and now says so in the log. initInstance is the one case that still throws: it dereferenced a null runtime, so constructing a binding off a runtime thread produced a bare NPE. Dropping there would leave an unregistered instance whose every later call fails anyway, further from the cause, so it reports instead. passSuppressedExceptionToJs routed by calling thread, so an exception suppressed on behalf of another runtime was reported to the wrong one - and the message it built was never passed to passDiscardedExceptionToJs. It now takes the instance, resolves the same owner, and reports on that runtime's own thread without blocking. The two-argument overload stays for already-generated bindings. Bindings from an older generator carry no id and keep the previous thread-based resolution. Costs two methods and one int field per binding class; noted for dex budgets.
1 parent 8a4ac28 commit 53f9373

14 files changed

Lines changed: 300 additions & 23 deletions

File tree

‎test-app/app/src/main/assets/app/tests/testsForRuntimeBindingGenerator.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ describe("Tests for runtime binding generator", function () {
109109

110110
var interfaces = clazz.getInterfaces();
111111

112-
var expectedInterfaces = ["java.util.jar.Pack200$Packer", "java.util.Formattable", "java.util.Observer", "java.util.jar.Pack200$Unpacker", "com.tns.NativeScriptHashCodeProvider"];
112+
var expectedInterfaces = ["java.util.jar.Pack200$Packer", "java.util.Formattable", "java.util.Observer", "java.util.jar.Pack200$Unpacker", "com.tns.NativeScriptHashCodeProvider", "com.tns.NativeScriptRuntimeBound"];
113113

114-
expect(interfaces.length).toBe(5);
114+
expect(interfaces.length).toBe(6);
115115

116116
for(var i = 0; i < interfaces.length; i++) {
117117
var interfaceName = interfaces[i].getName().toString();
@@ -168,9 +168,9 @@ describe("Tests for runtime binding generator", function () {
168168

169169
var interfaces = clazz.getInterfaces();
170170

171-
var expectedInterfaces = ["java.util.jar.Pack200$Packer", "java.util.Formattable", "java.util.Observer", "java.util.jar.Pack200$Unpacker", "com.tns.NativeScriptHashCodeProvider"];
171+
var expectedInterfaces = ["java.util.jar.Pack200$Packer", "java.util.Formattable", "java.util.Observer", "java.util.jar.Pack200$Unpacker", "com.tns.NativeScriptHashCodeProvider", "com.tns.NativeScriptRuntimeBound"];
172172

173-
expect(interfaces.length).toBe(5);
173+
expect(interfaces.length).toBe(6);
174174

175175
for(var i = 0; i < interfaces.length; i++) {
176176
var interfaceName = interfaces[i].getName().toString();

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/Generator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,8 @@ private void writeFieldsToWriter(Writer writer, JavaClass clazz) {
448448
String normalizedClassName = BcelNamingUtil.resolveClassName(clazz.getClassName());
449449
fieldsWriter.writeStaticThizField(normalizedClassName);
450450
}
451+
452+
fieldsWriter.writeRuntimeIdField();
451453
}
452454

453455
private void writeConstructorsToWriter(Writer writer, JavaClass clazz, DataRow dataRow, String generatedClassName, GenericHierarchyView genericHierarchyView) {
@@ -523,6 +525,7 @@ private void writeMethodsToWriter(Writer writer, GenericHierarchyView genericHie
523525

524526
methodsWriter.writeInternalRuntimeHashCodeMethod();
525527
methodsWriter.writeInternalRuntimeEqualsMethod();
528+
methodsWriter.writeInternalRuntimeIdAccessorMethods();
526529
}
527530

528531
/**

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/generating/writing/FieldsWriter.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22

33
public interface FieldsWriter extends JavaCodeWriter {
44
void writeStaticThizField(String className);
5+
void writeRuntimeIdField();
56
}

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/generating/writing/MethodsWriter.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ public interface MethodsWriter extends JavaCodeWriter {
88
void writeGetInstanceMethod(String className);
99
void writeInternalRuntimeEqualsMethod();
1010
void writeInternalRuntimeHashCodeMethod();
11+
void writeInternalRuntimeIdAccessorMethods();
1112
void writeInternalServiceOnCreateMethod();
1213
}

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/generating/writing/impl/ClassWriterImpl.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ public class ClassWriterImpl implements ClassWriter {
1111
private static final String PUBLIC_CLASS_KEYWORD = "public class";
1212
private static final String JAVASCRIPT_IMPLEMENTATION_FILE_NAME_PATTERN = "@com.tns.JavaScriptImplementation(javaScriptFile = \"./%s\")";
1313
private static final String NATIVESCRIPT_HASHCODE_PROVIDER_INTERFACE_NAME = "com.tns.NativeScriptHashCodeProvider";
14+
private static final String NATIVESCRIPT_RUNTIME_BOUND_INTERFACE_NAME = "com.tns.NativeScriptRuntimeBound";
1415
private static final String NOT_EXTENDING_ANY_CLASS_MESSAGE = "Not extending any class!";
1516

1617
private final Writer writer;
@@ -42,6 +43,9 @@ public void writeBeginningOfChildClass(String className, String extendedClassNam
4243
writer.write(IMPLEMENTS_KEYWORD);
4344
writer.write(SPACE_LITERAL);
4445
writer.write(NATIVESCRIPT_HASHCODE_PROVIDER_INTERFACE_NAME);
46+
writer.write(COMMA_LITERAL);
47+
writer.write(SPACE_LITERAL);
48+
writer.write(NATIVESCRIPT_RUNTIME_BOUND_INTERFACE_NAME);
4549

4650
if (!isEmpty(implementedInterfacesNames) && !isEmpty(implementedInterfacesNames.get(0))) {
4751
writer.write(COMMA_LITERAL);

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/generating/writing/impl/FieldsWriterImpl.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,21 @@ public class FieldsWriterImpl implements FieldsWriter {
77

88

99
private static final String THIZ_KEYWORD = "thiz";
10+
private static final String RUNTIME_ID_FIELD_DECLARATION = "private int runtimeId__ns = com.tns.NativeScriptRuntimeBound.INVALID_RUNTIME_ID;";
1011

1112
private final Writer writer;
1213

1314
public FieldsWriterImpl(final Writer writer) {
1415
this.writer = writer;
1516
}
1617

18+
@Override
19+
public void writeRuntimeIdField() {
20+
writer.write(TABULATION_LITERAL);
21+
writer.writeln(RUNTIME_ID_FIELD_DECLARATION);
22+
writer.writeln();
23+
}
24+
1725
@Override
1826
public void writeStaticThizField(String className) {
1927
writer.write(TABULATION_LITERAL);

‎test-app/build-tools/static-binding-generator/src/main/java/org/nativescript/staticbindinggenerator/generating/writing/impl/MethodsWriterImpl.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ public class MethodsWriterImpl implements MethodsWriter {
4747

4848
private static final String INTERNAL_RUNTIME_EQUALS_METHOD_SIGNATURE = "public boolean equals__super(java.lang.Object other)";
4949
private static final String INTERNAL_RUNTIME_HASHCODE_METHOD_SIGNATURE = "public int hashCode__super()";
50+
private static final String INTERNAL_RUNTIME_GET_RUNTIME_ID_METHOD_SIGNATURE = "public int getRuntimeId__ns()";
51+
private static final String INTERNAL_RUNTIME_GET_RUNTIME_ID_RETURN_STATEMENT = "return runtimeId__ns;";
52+
private static final String INTERNAL_RUNTIME_SET_RUNTIME_ID_METHOD_SIGNATURE = "public void setRuntimeId__ns(int runtimeId)";
53+
private static final String INTERNAL_RUNTIME_SET_RUNTIME_ID_ASSIGNMENT = "runtimeId__ns = runtimeId;";
5054
private static final String INTERNAL_SERVICES_ONCREATE_METHOD_SIGNATURE = "public void " + ON_CREATE_METHOD_NAME + "()";
5155

5256
private static final String RETURN_KEYWORD = "return";
@@ -369,7 +373,7 @@ private void writeCallJsMethodExceptionsSuppressBlockClosingIfNecessary(Type ret
369373
if (shouldSuppressCallJsMethodExceptions) {
370374
writer.write(CLOSING_CURLY_BRACKET_LITERAL);
371375
writer.write(GENERIC_CATCH_BLOCK_BEGINNING);
372-
writer.writeln("\t\t\tcom.tns.Runtime.passSuppressedExceptionToJs(throwable, \"" + methodName + "\");");
376+
writer.writeln("\t\t\tcom.tns.Runtime.passSuppressedExceptionToJs(this, throwable, \"" + methodName + "\");");
373377
writer.writeln(ANDROID_LOG_METHOD_CALL_STATEMENT);
374378

375379
if (!returnType.equals(Type.VOID)) {
@@ -398,6 +402,12 @@ public void writeInternalRuntimeHashCodeMethod() {
398402
writeInternalRuntimeMethod(INTERNAL_RUNTIME_HASHCODE_METHOD_SIGNATURE, INTERNAL_RUNTIME_HASHCODE_METHOD_RETURN_STATEMENT);
399403
}
400404

405+
@Override
406+
public void writeInternalRuntimeIdAccessorMethods() {
407+
writeInternalRuntimeMethod(INTERNAL_RUNTIME_GET_RUNTIME_ID_METHOD_SIGNATURE, INTERNAL_RUNTIME_GET_RUNTIME_ID_RETURN_STATEMENT);
408+
writeInternalRuntimeMethod(INTERNAL_RUNTIME_SET_RUNTIME_ID_METHOD_SIGNATURE, INTERNAL_RUNTIME_SET_RUNTIME_ID_ASSIGNMENT);
409+
}
410+
401411
private void writeInternalRuntimeMethod(String signature, String returnStatement) {
402412
writer.write(signature);
403413
writer.write(OPENING_CURLY_BRACKET_LITERAL);

‎test-app/build-tools/static-binding-generator/src/test/java/android/util/Log.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package android.util;
2+
3+
/**
4+
* Stub for compiling generated bindings in tests: the exception-suppressing
5+
* variant emits a call to android.util.Log, which is not on the test classpath.
6+
*/
7+
public class Log {
8+
public static int w(String tag, String msg) {
9+
return 0;
10+
}
11+
}

‎test-app/build-tools/static-binding-generator/src/test/java/com/tns/NativeScriptRuntimeBound.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.tns;
2+
3+
public interface NativeScriptRuntimeBound {
4+
int INVALID_RUNTIME_ID = -1;
5+
6+
int getRuntimeId__ns();
7+
8+
void setRuntimeId__ns(int runtimeId);
9+
}

‎test-app/build-tools/static-binding-generator/src/test/java/com/tns/Runtime.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ public static void initInstance(Object instance) {}
55
public static Object callJSMethod(Object javaObject, String methodName, Class<?> retType, Object... args) {
66
return null;
77
}
8+
public static void passSuppressedExceptionToJs(Object instance, Throwable ex, String methodName) {}
89
}

0 commit comments

Comments
 (0)