Node service thread is stopped incorrectly

[aschrijver]

In NodeService.java at NodeService.java#L25 the thread is stopped incorrectly which will lead to memory leaks and eventually crashes.

    @Override
    public void onDestroy() {
        super.onDestroy();
        if (t != null) {
            t.stop();
            t = null;
        }
    }

In this code t.stop() has been deprecated (for 11 years already, it was a mistake to ever add it) and is implemented with an UnsupportedOperationException.
Also setting the thread to null does not help. While the reference to the thread is gone, there is still a reference to it in the ThreadPool causing it to not be GC'ed and keeps happily running.

The way to implement stopping the node service is by letting the thread stop itself. You could do this by signalling t.interrupt() so thread can check Thread.currentThread().isInterrupted().

Problem is the thread is in private native void startNode(String... app) and will not detect this if this call is blocking (I didn't test, but I have similar issues with J2V8 nodeJs.handleMessage() now, where the node app is running an express server).

The solution for this last issue should be report to node app that it should (gracefully) stop running tasks that are blocking, then stop its execution, so the thread can continue to terminate itself.

[mafintosh]

Send a PR that fixes this. Would be appreciated.

[aschrijver]

If I fix it for J2V8 I will look if that can be applied here, or refer to the solution. Have little time available though. Besides it could well be that there are issues with stability of NodeJS on android.

When you continue with this project you should first test if you have similar issues as I do: app crashes after stopping and starting the node service (which in my case does nothing more than run an express server that says 'Hello world' when queried on localhost:5000 by a fetch from react-native, which is completely unaware of the node background service).

The long-running express server causes J2V8 event loop in Java to block on handleMessage() and no clean way of notifying node to stop gracefully (after which release() in Java releases native resources). If you want to bring Dat to android it should be similarly long-running, I imagine, so beware.