MarkusJx / node-java-bridge

A bridge between Node.js and Java
https://markusjx.github.io/node-java-bridge/
MIT License
115 stars 6 forks source link

Listener dies unexpectedly #57

Closed vladislavbagnyuk closed 1 year ago

vladislavbagnyuk commented 1 year ago

Hello,

I'm trying to use this library to connect to a scale using one Java library. It starts correctly and returns scale values for some time, but then the scale listener stops. However, if I use https://github.com/joeferner/node-java this library to bridge JS and Java it works - but I can't use that library because it doesn't support new Electron releases. I've made minimal examples using your library and the other one and attached the files. scaleTest.js is made using your library, scaleTestJava.js is using joeferner/node-java. example.zip

Could you please help me? Thank you

MarkusJx commented 1 year ago

The biggest issue I see here is that you create a proxy and presumably call it in a synchronous context. As the documentation states, this will cause the program to hang, as the js code waits for the java code to run but since js is single-threaded, the proxy can't run as the only js thread is already occupied waiting for the java code to finish.

This was already mentioned in another issue: https://github.com/MarkusJx/node-java-bridge/issues/6#issuecomment-1160864176.

So you should probably just call the method (which itself calls the proxy) in an async context, as this will resolve your issue. The joeferner library creates and calls proxies in a different way, I don't know why I did it differently but there was probably a reason for it. I'll look into the differences and probably document why I've chosen this route.

MarkusJx commented 1 year ago

Looks like I can't change anything about this, nan seems to handle method calls differently than n-api.

MarkusJx commented 1 year ago

Sooo, turns out I can do something about that, you may want to try version 2.2.3-beta.1. By forcing the event loop to run while waiting, proxies can now be used in a synchronous context. Assuming this was your issue to begin with.

But this somewhat depends on https://github.com/napi-rs/napi-rs/pull/1499 being merged, the current solution in #58 is somewhat hideous.

Let me know if that resolves your issue

vladislavbagnyuk commented 1 year ago

Hello. 2.2.3-beta.1 and beta.2 still behave the same in my case, the listener dies after a short time.

MarkusJx commented 1 year ago

Does your heartbeat still work or is the application just freezing up completely? Can you maybe provide some java code for me to reproduce this problem? Do you get any errors while running the code?

vladislavbagnyuk commented 1 year ago

The heartbeat still works - console.log("heartbeat") is still working. It doesn't show any errors and doesn't freeze, it just shows correct values for some time and then falls to outputing just "heartbeat":

Screenshot 2023-02-27 at 10 51 14

I'll probably try to set up communication between the Java and JS parts using WebSockets instead of a bridge library.

MarkusJx commented 1 year ago

So I was able to reproduce this issue using the following code:

Code Java: ```java package org.example; import java.util.function.Consumer; public class Main { public static void heartbeat(Consumer consumer) { new Thread(() -> { int i = 0; while (true) { consumer.accept("heartbeat " + i++); try { Thread.sleep(1000); } catch (InterruptedException e) { e.printStackTrace(); } } }).start(); } } ``` JS: ```js const java = require('java-bridge'); java.classpath.append('build/libs/test-1.0-SNAPSHOT.jar'); const main = java.importClass('org.example.Main'); const proxy = java.newProxy('java.util.function.Consumer', { accept: function (value) { console.log('Got value: ' + value); } }); main.heartbeatSync(proxy); setInterval(() => console.log('tick'), 1000); ```

The issue is that the proxy gets destroyed after a while as the v8 assumes it's not used anymore. In most cases, this issue simply wouldn't exist, as there will always be code after the proxy has been created, so keeping it as a top-level variable would work just fine. In your case, there is basically no code after the proxy has been created, so logically it can't be used by anything anymore, so it gets destroyed. Bad luck mate.

Also, you should've received an exception like this:

Exception in thread "Thread-0" java.lang.reflect.UndeclaredThrowableException
        at com.sun.proxy.$Proxy0.accept(Unknown Source)
        at org.example.Main.lambda$heartbeat$0(Main.java:14)
        at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: java.lang.IllegalAccessException: The proxy interface isn't valid anymore
        at io.github.markusjx.bridge.JavaFunctionCaller.invoke(JavaFunctionCaller.java:73)
        ... 3 more

This clearly indicates that the proxy has been destroyed.

A simple (but pretty awful) solution to this problem would be keeping the proxy alive manually:

// The proxy still is in use, I guess
setInterval(() => proxy 1000);

I will post a better solution probably later today.

FYI: The reason why your code works with joeferner/node-java is beacause that library just keeps the proxies alive at all times which will prevent your application from exiting after its done which is also less than ideal, I think. But if you'd like an option to not destroy proxies, let me know, I can make that work.

vladislavbagnyuk commented 1 year ago

Hello. Thank you for your help. Keeping the proxy alive manually via setInterval works on 2.2.3-beta.1 (not on 2.2.3-beta.2) on an Intel x64 Mac, but for some reason still doesn't work on Linux on arm64 (Raspberry Pi).

MarkusJx commented 1 year ago

I've added a new option which allows you to keep proxies alive even after they have been garbage collected, but I'm currently facing some issues with this feature, I'll notify you once I've fixed those issues

vladislavbagnyuk commented 1 year ago

Thanks

vladislavbagnyuk commented 1 year ago

Hello. How is it going with the option to keep proxies alive?

MarkusJx commented 1 year ago

It's kind of done. The actual problem is with the option to run proxies in synchronous contexts (which is not requires for your use case), the program sometimes crashes, this may be me screwing up or an issue with node.js, as there are some reports with the node runtime crashing when forcing the event loop to run. Will probably keep that feature as an experimental one and investigate later.

And in case of me not forgetting this, I'll create a new beta release later today with the changes you'll need

vladislavbagnyuk commented 1 year ago

Thank you, beta release later today would be great.

MarkusJx commented 1 year ago

Version 2.2.3-beta.3 is out now. There is now a new option when creating a proxy to keep it alive:

let proxy = java.newProxy('java.util.function.Function', {
  apply: (arg: string): string => {
    return arg.toUpperCase();
  },
}, {
  keepAsDaemon: true,
});

If you want to shut down your program, simply delete all daemon proxies by calling:

java.clearDaemonProxies();
vladislavbagnyuk commented 1 year ago

beta.3 is working, thank you!