LiquidPlayer / LiquidCore

Node.js virtual machine for Android and iOS
MIT License
1.01k stars 127 forks source link

Killing a node process #99

Open Bahlinc-Dev opened 5 years ago

Bahlinc-Dev commented 5 years ago

Hi,

First of all, great job on this project! I'm surprised how fast node is on mobile. Not sure if its due to some of the stuff being stripped out, but easily keeps up with my i7 desktop on an average android device. Amazing.

Anyways, I have a large cross-platform node project that is designed to be long running (think days or weeks). The code base is large and complex. Sometimes things just go wrong and require a fresh restart. I found service.getProcess().exit(), but strangely this doesn't seem to really release the process. Sometimes the onProcessExit() event is called on the java side, and sometimes not. It seems to depend on what the node process is doing when its called. I've even tried calling process.exit() from within node. That fires the process.on('exit') event in node, but modules like Express continue to listen to their ports and setIntervals() are still in the event loop.

Is there a trick to kill and release the node thread that I'm missing?

ericwlange commented 5 years ago

Hmm, interesting. Does it behave this way on desktop, too? Or don't you really know since you just Ctrl^C anyway?

I just added a nuclear option to Android which will be released in 0.6.1 by this weekend. It's there in master if you want to try it.

It is JSContextGroup.terminateExecution()

It isn't specific to Node, so I am not sure what will happen to running worker threads, but you can try it. Given what you are saying, though, I am not sure it will stop all the callbacks and may wind up crashing your app.

If you want to hack around a bit, the node process is actually started in java here. You may be able to forcefully kill that thread, however a quick read of StackOverflow suggests this may not be possible anymore.

The right way to do this is to execute process.exit() like you've done. In fact, that is all service.getProcess().exit() does anyway. I am surprised you get different results from doing it both ways.

If you can create a simple, repeatable test case, I can take a look more closely.

Bahlinc-Dev commented 5 years ago

Ok, I'll give that a try.

On desktop, after process.exit() call, the app takes maybe 2-3 seconds to close. So I'm not sure what node is doing during that time, but it does eventually exit. On android it just stays around forever. So maybe it doesnt actually close on desktop either, and the os kicks in and kills it at some point? Not sure.

I hope I can produce a simple test case for you. Working on that now. The app as it is has a bundled size if about 8mb. I haven't yet tried to narrow it down beyond just calling process.exit().

pdulapalli commented 4 years ago

I have encountered a similar issue as @AbortingMission . The HTTP servers that were created in the Node.js Microservice do not appear to release their ports on process exit (if the process even exits at all).

I've added logic to ServiceErrorListener and ServiceExitListener to initialize a new Microservice if the old one has crashed or exited. I've played around with delays up to 10 seconds between crash/exit and before calling the new Microservice startup routine, with no success.

I've attached a ZIP with two files:

sample-files.zip

ericwlange commented 4 years ago

@pdulapalli @AbortingMission I think I know why this is happening. I need to figure out how to deal with it, however. In node, when you call process.exit() it basically throws an abort signal and exits to the OS. I capture the exit call, and instead of aborting, I clear the event queue which will cause the process to exit when it finishes its current task without executing any new tasks that are in the queue. This is because I don't want process.exit() to crash the entire app. The problem with this is that if the current task is in an endless loop, then it never gets to where it can exit the event loop cleanly. I need to abort execution of the existing task, which I can do as mentioned above on Android. However, this will not work on iOS. There is no exposed public method in JavaScriptCore to do this (there is a private one, but using it will get your app declined in the app store).