LiquidCoreAndroid crash if an environment Var is set: NODE_OPTIONS="--expose-gc"

[fastcoding]

I got 'out-of-memory' fatal error after keeping sending pictures for about 5 minutes. I noticed both the total heap and actual usage are increasing all the time. So I tried to use manual gc to force garbage collection in my script. However, this doesn't work on Android. The LiquidCoreAndroid will crash if the environment NODE_OPTIONS is set to "--expose-gc".

Anyway to get this option work in LiquidCoreAndroid?

[ericwlange]

You would need to build the library from source. See line 93 of JSContextGroup.cpp. It is already there, just commented out. Uncomment and rebuild.

If you have a very simple example that demonstrates this behavior, please link to it here. I can debug heap issues.

[fastcoding]

Appreciated for your help. The -expose-gc option is successfully added to the flags in ContextGroup::init_v8(). global.gc function is now available in script.

I took some time to rebuild the whole project in my android studio. A little updates were made to cmake configuration in the module gradle file, added the following arguments:
cmake {
arguments "-DANDROID_TOOLCHAIN=gcc", "-DANDROID_STL=gnustl_static","-DANDROID_CPP_FEATURES=rtti exceptions"
}
otherwise, it will fail to build or crash during run time.

But the memory issue looks still there - the heap usage is growing up forever regardless of whether gc() is called or not, as long as the express server running in android is sending pictures through websocket. If I added --max-old-space-size=64, the express server will be killed quietly after heap.usage increased to 100M.

I will try to make a small example to reproduce such issue and update here later.

[fastcoding]

I just reproduced this memory leak issue. Please check it out from here.

[ericwlange]

Thanks @fastcoding . Let me look into it.

[ericwlange]

First off, thanks for the cmake arguments to get around the new NDK release. That saved me a lot of time!

I am running your app on my simulator but I am not sure I am seeing the issue. At the moment, I am running the profiler and it shows the memory usage as quite constant (~36MB) and it is not growing at all. The heap usage indicator in logcat is also constant, fluctuating between 26-29MB.

I just started the app, switched to chrome and loaded the URL. The little guy is happily smoking away (or eating pistachios or blowing kisses, I'm not sure). Do I need to do something else to trigger the issue? It's been running for about 10 minutes.

Edit: after a few more minutes, the heap dropped to under 20MB and is now constant there. Still running.

[fastcoding]

Hi Eric, You need to check the heap usage output from javascript, As the heap is allocated by C++ codes, java cannot detect its existence. Here are my two screenshots taken from the testing on 2 devices. The results are the same: After 1000 seconds, both total heap usage grow up to 60-70M.

• Running on an old device with android 5.

• Running on a device with android 7.

[fastcoding]

Add some description about my testing codes.

At bin/www.js#137, I created a timer that periodically reports the heap usage using nodes api :
process.memoryUsage()

At bin/www.js#151, another timer is created for broadcasting pictures, which in turn calls into work.js ,
and finally gets to the callback to Java at work.js#40

android_fetch_pic()

[ericwlange]

ok, let me take a closer look

[fastcoding]

Hi Eric, I noticed that in each callback to JAVA code, all parameters and return value will be saved as Persist v8:Value into a list(m_managedValues) in ContextGroup, and they are only Disposed only when Java garbage collection starts to finalize JSValue. All the weak pointers in m_managedValues list will not be removed until the whole JS context is disposed. No idea what's wrong with such process yet.

I just replaced several pictures with big ones so that the heap issue can quickly come out - crash with 'Out of Heap' error. also added LiquidCoreAndroid as a submodule for debug.

[ericwlange]

Hey -- I still haven't had a chance to dig in yet. I need to finish my taxes! But I will take a closer look, I promise!

[fastcoding]

Thanks a lot. Comparing to the other 2 tools, jxcore or j2v8, I love LiquidCoreAndroid more. It's a pity that I cannot use it in my current project. Due to the time issue, I've stopped to dig deeper into your codes and changed my solution to convert the whole vendor's java/jni interface to node/v8 during the past one week. The embedded server contains no more java codes now and the memory issue has already gone. But I still keep your libnode.so in my project, to be linked with the new node-addon.

[ericwlange]

Hi Han. I'm glad you found a workaround that suits your needs. It's for the best, as it isn't clear to me what is causing the issue and it may take me some time to debug. But I will figure it out. Aside from my taxes, I have been preoccupied with iOS. I have a "sort-of" working version for iOS under private development. It's very near to being alpha-ready. So context switching between iOS and Android has been challenging. But it is my hope that you would find the ability to run node.js on iOS equally valuable to your project, so stay tuned for that.

[ericwlange]

Ok, I now was able to spend a few solid days on this problem. I wound up refactoring quite a bit of the Java<->C++ logic, but in the end that turned out to be cosmetic. The real culprit was rather simple. In the SharedWrap destructor, which is called in the Java finalizer, instead of deleting the wrapper, I defer it until later by putting it into a zombie queue. A task is supposed to run periodically to free the zombies in the queue. This is done for two reasons: (1) doing anything crazy in the finalizer thread can lead to timeouts and crashes, so I keep it simple, and (2) when running in node mode, everything JS-wise must be run in a single thread.

Unfortunately, the task to free the zombies was never getting called until the process was freed. It was a simple fix to ensure the cleanup function is called in the node thread on a regular basis.

This is fixed in 0.5.0-experimental-7 and will be pushed into the full 0.5.0 release that I should get out in the next day or two after I finish documentation and some manual testing. Feel free to give it a try if you have time.

@fastcoding Thanks for putting together the test app. It was a great help. I will simplify it further and get it into the test suite.

[ericwlange]

Marked as fixed in 0.5.0. Please re-open if this doesn't solve the problem.

[adielgur]

Hi, I'm using LiquidCore as a JavascriptCore engine for Android, by using this guide- https://github.com/LiquidPlayer/LiquidCore/wiki/LiquidCore-as-a-Native-Javascript-Engine

I'm experiencing the same issue with the memory leak using version 0.5.1 (that still has the JSCShim.cpp file) where the finalize function is not called while the program runs, even when calling JSGarbageCollect(globalContext).

When the context is released the finalize function is called for each instance.

I've reproduced this issue with a simple Point class definition for JS; evaluating JS code that creates 100,000 instances of that class within a loop.

https://github.com/Mythos666/LiquidCoreTest

If you uncomment the context release function in the end of the native-lib.cpp file it'll print 0 since then deallocation occurs.

[ericwlange]

Have you tried the latest version (0.6.2)? Does it still occur there?

…

On Thu, Jun 20, 2019, 1:36 PM Mythos666 ***@***.***> wrote: Hi, I'm using LiquidCore as a JavascriptCore engine for Android, by using this guide- https://github.com/LiquidPlayer/LiquidCore/wiki/LiquidCore-as-a-Native-Javascript-Engine I'm experiencing the same issue with the memory leak using version 0.5.1 (that still has the JSCShim.cpp file) where the finalize function is not called while the program runs, even when calling JSGarbageCollect(globalContext). [image: Screen Shot 2019-06-20 at 2 00 39 PM] <https://user-images.githubusercontent.com/32482114/59845952-4c263e00-9367-11e9-9c8f-d758f92c7e12.png> When the context is released the finalize function is called for each instance. I've reproduced this issue with a simple Point class definition for JS; evaluating JS code that creates 100,000 instances of that class within a loop. https://github.com/Mythos666/LiquidCoreTest If you uncomment the context release function in the end of the native-lib.cpp file it'll print 0 since then deallocation occurs. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#52?email_source=notifications&email_token=AAJC4ZHJAE6PPW22ZS7KY33P3NTVHA5CNFSM4FIRKO4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYFEYGA#issuecomment-503991320>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJC4ZDVBNTDBBAOLYKCVSDP3NTVHANCNFSM4FIRKO4A> .

[adielgur]

Well, I'm using the JSC->V8 bridge feature which requires JSCShim.staticInit() and for some reason it appears that this file-
https://github.com/LiquidPlayer/LiquidCore/blob/0.5.1/LiquidCoreAndroid/src/main/java/org/liquidplayer/jscshim/JSCShim.java

was removed in 0.6.0-
ee0e82a

[ericwlange]

First of all, I love that you are using JSC2V8. I wasn't aware that anyone but me was using it.

I removed the JSCShim business in 0.6.0 when I introduced support for node modules. This was a workaround to deal with the fact that one .so needs to rely on another one. This is unscalable because native node modules will need to link against node.h and v8.h as well and this can't be shimmed. The right way to do it is to get gradle and cmake to allow you to link. So since 0.6.0, I package the header files inside the .aar and you can extract them and the libraries to link against.

You can see the gradle file I use for ReactNativeSurface (which is now distributed as a node module). The relevant parts are:

configurations {
    liquidcore {}
}

dependencies {
    /* Other stuff */
    ...

    if (findProject(':LiquidCore') != null) {
        liquidcore project(path: ':LiquidCore', configuration: 'default')
        implementation project(':LiquidCore')
    } else {
        liquidcore 'com.github.LiquidPlayer:LiquidCore:0.6.2'
        implementation 'com.github.LiquidPlayer:LiquidCore:0.6.2'
    }
}

task extractAddOnLibsAndHeaders(type: Sync) {
    dependsOn configurations.liquidcore

    from {
        configurations.liquidcore.collect {zipTree(it)}
    }
    include "jni/**/*.so", "include/**/*"
    into "build/liquidcore-addon"
}

afterEvaluate {
    def addon = file('build/liquidcore-addon')
    if (!addon.exists()) {
        if (project.hasProperty("externalNativeBuildDebug")) {
            externalNativeBuildDebug.dependsOn extractAddOnLibsAndHeaders
        }
        if (project.hasProperty("externalNativeBuildRelease")) {
            externalNativeBuildRelease.dependsOn extractAddOnLibsAndHeaders
        }
    }
}

What this does is extract the public header files (including the JavaScriptCore/*.h files) and the .so files into /build/liquidcore_addon/{jni,include} so that your CMakeLists.txt can find them. See the ReactNativeSurface CMakeLists.txt file for example. Relevant bit:

include_directories(
        build/liquidcore-addon/include/
)

add_library(
        liquidcore-lib
        SHARED
        IMPORTED
)
set_target_properties(
        liquidcore-lib
        PROPERTIES IMPORTED_LOCATION
        ${PROJECT_SOURCE_DIR}/build/liquidcore-addon/jni/${ANDROID_ABI}/libliquidcore.so
)

target_link_libraries(
        YOUR_TARGET ...
        OTHER_LIBS ...
        liquidcore-lib
)

Now you should be able to #include <JavaScriptCore/JavaScript.h> (or even v8.h if you wish) and it will link correctly.

[adielgur]

Hi Eric,
Thank you for your detailed response.
I was able to modify my test project to compile using 0.6.2 and the instructions you've given.
However the memory issue appears to behave the same in this version.

[ericwlange]

Ok @Mythos666, let me try your project and see what I can see.

[ericwlange]

Opened a new issue (#115) to address this.

This has a similar result to the original issue, but it is not exactly the same, so moving it out of here. This particular issue as originally reported (related to JNI) is still fixed. Your issue is specific to JSC2V8, and in particular, objects created from a JSClassRef.