using class extension makes the app hangs

[farfromrefug]

I am working on a native module for Caro Mobile Sdk. You can find my plugin here: https://gitlab.com/farfromrefuge/nativescript-carto

On android I have an issue implementing the map listener.

The issue is from that line It makes the app hangs. If I comment it or even use:

        mapView.setMapEventListener(new com.carto.ui.MapEventListener());

The app works correctly.

Trying to use my class extension makes it hang. It might be related to the fact that the Carto Mobile is build upon jni. I mean all classes are just bridges to native classes implemented in the carto mobile framework.

That's why I am opening the issue here.

you can easily test it by cloning, and running tns run android in "dev_demo"

Thanks

[farfromrefug]

I tried to debug the app and pause the debugger while the app was hanging.

First the callback which is supposed to be called very frequently (app moves) is called once since I see a log I have put there. But then it hangs in the GLThread

[farfromrefug]

If I comment the "wait" then the app works fine. May I ask when do you need to use the wait? I mean in the runnable arr[1] is always put to true. So I am not sure I understand when you need the wait.

[farfromrefug]

Any comment on this? I really need help on this as I can't really get my plugin to work because of this

[farfromrefug]

I worked on this and found out that it was because my listener methods were not called on the main thread. Is there a "Nativescript way" to make sure my callbacks are run on the main thread? Something like a decorator, even on the Java side, would be awesome.

[darind]

There's no way to enforce the callback to be executed on the main thread if this callback is raised by an API that is outside of NativeScript's control. What you could do instead is to execute some code on the main thread inside the callback:

var mainHandler = new android.os.Handler(android.os.Looper.getMainLooper());
var MyRunnable = java.lang.Runnable.extend({
    run: () => {
        // Put some code that will be executed on the main thread
    }
});
mainHandler.post(new MyRunnable());

[farfromrefug]

@darind thanks that exactly how i do it on the java side: https://github.com/farfromrefug/nativescript-carto/blob/master/src-native/android/additions/src/main/java/com/akylas/carto/additions/AKMapView.java#L23

BTW in my case i can't do it on the javascript side using your code because it is actually the generated java class (when i extends the java listener) which tries to call the javascript method not from the main thread. That's why i think it would be great to have a typescript decorator in Nativescript which, when used, would add the java decorator to the java generated class, which would make sure the java method is called on the main thread, and thus to javascript method call.

Do you understand? Does that seem feasible? I could create a new issue for this if you want.

[darind]

The auto-generated java class only implements the interface. It doesn't make the actual method call. This is done by the underlying native implementation of the com.carto.ui.MapView object that you are passing your listener instance to. It is this implementation that normally has the knowledge of whether the callback should be invoked on the main or on a background thread.

If I understand you correctly you would like to have the possibility of decorating your js methods with some attribute which will wrap the auto-generated java method calls (done by the static binding generator) with a handler to ensure they are called on the main thread? In this case it is always possible to wrap this logic in a custom java class that will be used from js, as you already did. Decorators are still an experimental feature of typescript that we would like to avoid building into the runtime if possible.

[farfromrefug]

Here is an example of generated Listener class by the runtime:

package com.tns.gen.com.akylas.carto.additions;

public class AKVectorTileEventListener_vector___VectorTileEventListenerImpl extends com.akylas.carto.additions.AKVectorTileEventListener implements com.tns.NativeScriptHashCodeProvider {
    public AKVectorTileEventListener_vector___VectorTileEventListenerImpl(){
        super();
        com.tns.Runtime.initInstance(this);
    }

    public boolean onClicked(com.carto.ui.VectorTileClickInfo param_0)  {
        java.lang.Object[] args = new java.lang.Object[1];
        args[0] = param_0;
        return (boolean)com.tns.Runtime.callJSMethod(this, "onClicked", boolean.class, args);
    }

    public boolean equals__super(java.lang.Object other) {
        return super.equals(other);
    }

    public int hashCode__super() {
        return super.hashCode();
    }

}

What makes my app hangs is

return (boolean)com.tns.Runtime.callJSMethod(this, "onClicked", boolean.class, args);

because it called from a background thread.

What would be awesome (:D) woud be the the generated java function to look like this:

@runOnUIThread
public boolean onClicked(com.carto.ui.VectorTileClickInfo param_0)  {
        java.lang.Object[] args = new java.lang.Object[1];
        args[0] = param_0;
        return (boolean)com.tns.Runtime.callJSMethod(this, "onClicked", boolean.class, args);
    }

Then Nativescript android runtime could declare a java decorator which makes sure onClicked is called on the UI thread.

This is what i have done in Titanium Mobile here (commented code)

I get your point on typescript decorator. Is there any way to do that in a plugin? I think ii could create a nativescript plugin which defines the typescript decorator, add the java decorator. But i don't see how i could make it so that that the java decorator is added to the java class

[darind]

As I already explained, this method is called from a background thread because that's how the MapView class is implemented. NativeScript doesn't spawn background threads to invoke methods on it.

The auto-generated class is created by the static binding generator at compile-time by analyzing the javascript code (it doesn't know anything about typescript). The problem is that at the moment I do not see a convenient way in javascript to decorate your methods in order to indicate to the parser to emit the corresponding attributes on the Java class.

[farfromrefug]

@darind i never said Nativescript was the reason why that listener was called on a background thread :D I know it s Carto, it's actually written in their doc.

Ok no way to do that right now. I really hope you put that somewhere in your future features list. This is a common issue with Nativescript plugins.

Thanks @darind for your great help!

[NathanaelA]

@farfromrefug - This issue really I believe should remain open. This is a valid issue, and we should figure out some way to potentially fix it.

@darind; what about using a comment something like // java: @runOnUIThread or /* java: @runOnUIThread */ before the function to cause the SBG to tag the function with anything after the // java: on the next generated java function?

[darind]

@NathanaelA - while comments might work in some simple cases, they will probably be stripped off in more real world scenarios where people would use a bundler such as webpack to bundle and minify their code in a production build. The SBG runs on the final JS that is eventually fed to V8.

[NathanaelA]

@darind - Ah, didn't realize SBG ran at the end; then I believe I have a difference solution, along the same lines. ;-)

Way back in JavaScripts past; we needed to add an annotation (i.e. an ExpressionStatement) to JS files to put the engine into strict mode. The solution was to add "use strict"; at whatever scope you needed it.
http://ecma-international.org/ecma-262/5.1/#sec-14.1

We can use the exact same method; a standalone string is 100% valid JS, even if it doesn't do anything. "java: @runOnUIThread"; I just tested webpack and it does leave these string literals in the source code; which means it would be available for the SBG to pick up. Unfortunately --env.uglify eliminates it in the global scope (might be a setting to retain it???) --- but it does preserve it in the function scope.

So a working sample would be:

public boolean onClicked(com.carto.ui.VectorTileClickInfo param_0)  {
               "java: @runOnUIThread, @someOtherValue, @andAnother";
        java.lang.Object[] args = new java.lang.Object[1];
        args[0] = param_0;
        return (boolean)com.tns.Runtime.callJSMethod(this, "onClicked", boolean.class, args);
    }

And the SBG could pick up the expression directive if it is the first statement and starts with "java:" ;-)

[darind]

@NathanaelA, actually NativeScript always invokes the callback on the main thread because V8 is single threaded: https://github.com/NativeScript/android-runtime/blob/master/test-app/runtime/src/main/java/com/tns/Runtime.java#L1126

The issue here appears to be some racing condition because of the current implementation which waits for the callback to complete.

Long story short, there doesn't appear to be immediate need of supporting annotations in the static binding generator. This being said, I have tested your suggestion of using string literals and they appear to be working quite nicely in case we decide to go that way.

We will continue investigating this issue and hopefully provide a fix. In the meantime the current workaround is to implement a wrapper Java class that will handle the marshaling to the main thread.

[farfromrefug]

@darind thanks for the explanation and the testing. Will go that way for now. I still really hope to see some work done on the "threading" handling in Nativescript ;)

Thanks!

[NathanaelA]

@darind - Just my 0.2cents -- If being able to do "java: @runOnUiThread"; would annotate the JS and would fix this issue. Then getting this support into SBG is something we should do in the future.

Because then in these cases where there is a background thread event that really needs to run on the main thread (which are rare, I admit) it could be easily solved via annotation. Switching to Java is the last thing we want to "force" a developer to do; the whole idea is NativeScript is supposed to eliminate that as much as possible. :grinning:

[darind]

@NathanaelA, actually it will not fix the issue. The thing is that this callback needs to be executed on the main thread because it needs to call into javascript code (that's because we have implemented the business logic in js). For this call to happen we need to run the js code on the main thread (V8 requirement), collect the returned values from the js method, marshal them to their Java counterparts, and finally return them to the calling Java method on the background thread. Since this operation is async we need to block the background thread until we execute the js code on the main one (thus the .wait() that the OP has noticed and which is causing the racing condition).

Hopefully now you realize that this is not as simple as a fire and forget operation. In this case it will work because the callbacks do not return any results (that's why when the OP removed the .wait call his example worked) but obviously this cannot be generalized.

Another thing to note is that simply annotating a method will not make it execute on the main thread. There must be some code that we write to inspect the annotations and make the required logic (which we already do without any annotations)

[NathanaelA]

@darind - Thank Darin, that explains the underlying issue a whole lot clearer. I did know about the threading on NS, and how it works (I even did a talk at the last NS-DevDays conf on NS threading. :grinning: ). I just was under the impression that pushing the handler automatically into the main thread would have fixed the issue, and so basically to handle this automatically their is a lot more work then just the annotation, which is all I was thinking was needed... :grinning: I want to keep the bar as low as possible to make plugins work.. :grinning:

[farfromrefug]

@darind i made some research recently and it seems that v8 can be used in a multi thread environment. You need to use a v8:: Locker. see this commit Could you look at this? {N} is already multi thread on iOS, could really use that on Android

[darind]

@farfromrefug, while using v8::Locker would allow accessing the Isolate from multiple threads, it will create contention and obviously block the main thread from executing any javascript while the background thread is running which will bring similar issues to what we are trying to solve in the first place.

The recommended way for doing multithreading in v8 is through separate Isolates, which in NativeScript is achieved with WebWorkers.

What exactly do you mean by {N} is already multi thread on iOS?

[farfromrefug]

@darind on iOS any js called from native will run on the thread the native call was made in. an example is camera live processing. I implemented for N in nativescript-opencv ( through the common delegate pattern used in N). and it runs perfectly. I have run a lot of tests and the JS is called on the background thread of the camera. on android it is slow because the JS call is made on the UI thread(camera live processing also on background thread)

about the v8::looper would nt it only block the v8 event loop from the ui thread and not the ui thread itself? I have seen a lot of references of people using it. Would it be fairly easy to test it?

and the pattern of different isolate and v8 seem to be for different use cases for me. I dont want a worker in this case

[farfromrefug]

it appears it is exactly what iOS does https://developer.apple.com/documentation/javascriptcore/jsvirtualmachine all other threads simply wait. in my tests it works perfectly fine and I don't see hickups. I hope we can try this on android. if it works we would have same behavior on iOS and android. and an option in the app package.json could be used to make the truly singlw threaded or not.

[darind]

@farfromrefug, you are correct that a v8::Locker will not block the UI thread but rather the V8 Isolate preventing it from executing any javascript. Which, in the case of NativeScript, is pretty similar because most of the logic is written in javascript.

Unfortunately the android runtime is not designed in a way that would allow to test this easily. It is not just a simple matter of placing a v8::Locker in the JNI calls. There are quite a lot of thread local caches and assumptions that a runtime instance is single threaded. Even if it works in your particular case, that would be more by chance, rather than something expected. Multi-threading is complicated and might lead to very subtle bugs that are difficult to find while developing.

[farfromrefug]

@darind i understand. Though i disagree with your point on UI thread being blocked because N logic is in JS. UI widgets are mostly native stuff which would still interact correctly. And that's mostly what we are talking about. If in the background JS call you do an endless loop yes you will block your app. And then go for a webworker i agree! My case is for all "async" stuff which should be done in a background thread while keeping the JS context. Like image processing (where the actual processing will be done in native thus wont stop the UI at all). This is the kind of stuff N has been trying to get around by implementing all in native. If i take the example of the Image widget. It s supposed to be async but it will block the UI thread while trying to load images. That s exactly where this would get awesome!

In fact right now we are seeing about exactly the contrary. The fact that all JS calls have to go through the UI thread actually can create hickhups on the UI Thread. Those native call are made on a background thread for a reason and we actually kind of try to "transfer" them to the UI thread. I am not saying N is wrong, far from me to say this :D i love it! And i totally get how "hard" it could get. My point is:

• The fact that the behavior is not the same on iOS and Android really bugs me. It can create really unexplained behaviors for the user. But don't disable this on iOS ! :D that would be such a shame as it works great.
• consider this as a FR please. i agree it is not easy, but i bet it would even solve some weird hickups some user might be seeing. And i would be more than willing to help in implementing this.

If you agree we could open a FR request separated for this?

PS : i just mention @mbektchiev here as he is the one who helped me on iOS with this and he might have some insight on the iOS side

[darind]

@farfromrefug, rewriting the android runtime is currently out of the scope. That being said, if you are willing to contribute to this, we would be happy to review any PRs.

[farfromrefug]

@darind sure will try and look at this. Though i will need your guidance, i don't have your knowledge of the runtime. For example you talk of local cache and assumptions. Can you refer to one or 2 so that i can wrap my head around this?

[darind]

@farfromrefug, here's one example: https://github.com/NativeScript/android-runtime/blob/master/test-app/runtime/src/main/java/com/tns/Runtime.java#L172