"JS function to Java Interface" Proxy leaking V8 native resources when GC-ed

[AlexTrotsenko]

Currently it's possible to provide Java functional interface for the cases where JS function is passed from JS to Java.
It's implemented using Proxy and V8FunctionInvocationHandler.

Currently .release() of native j2v8 objects is implemented:

1. in .finalize() method, which called during the GC.
2. If Proxy was not GC yet - during the call to V8JavaObjectUtils.releaseV8Resources(): Underlying V8Function function is stored in the v8Resources and thus released.

Issue: while 2nd option working well and tested in the unit tests, the 1st option is leaking j2v8 native objections (v8 function and v8 receiver).

It's hard to test 1st case as it involves GC, which runs at not determined period of time. Also following comment in the source code highlight possible problem as well:

// TODO: How do we release if this gets executed on a different thread?

I have tested it and indeed GC is run on the separate thread. E.g. on Nexus 5/Android 6 it's running on FinalizerDaemon. But all J2V8 objects requires .release() to be called on the same "v8 thread".

As result the Proxy object is released, but native j2v8 object are leaking.

Possible solution:

1. By default implement the proxy as call-back function, but not listeners. E.g. allow functions to be called once. This way V8Function could be released right after it's invoked.
2. If listener behaviour is required - introduce annotation like @JsListener. I could be present either on java interface declaration or when 3rd party interface is used as method argument.
3. Listener implementation could be implemented in 2 ways:
   3.1. Keep hard reference to all the call-back functions and thus they will be released in the V8JavaObjectUtils.releaseV8Resources() as mentioned above.
   3.2. Provide "V8 executor" during V8-adapter initialisation. When .finalize() is invoked - this executor can run .release() of underlying V8Function on the proper v8 thread.

Implementing listeners as 3.1 will result in retaining all V8Functionlisteners in memory until V8 is released.
Implementing listeners as 3.2 would require passing V8 executor, which leads to additional requirements of end users of v8-adapter lib.

Also different behaviour could be implement if V8 executor is present or not - but this sounds to much complicated for now.

Also as an option V8FunctionInvocationHandler and resulting proxy could implement Releasable interface and released manually if they need to be called known amount of times. E.g. 2 or 3 , etc.

@crahda as for me, adding v8 executor does not make things to much complicated for end users as it would be need only for listeners (but not for call-backs). Instead it would allow proper memory clean-up.

Additionally if this behaviour is implemented - some default functional interface like JsFunction could be created. And if function is present inside of the JS object - it could be automatically converted to Map without worries of leaking memory.

[AlexTrotsenko]

@crahda I have created 2 pull requests:

• #25 is way to manually release JS proxy-based call-backs resources, which is currently not released at GC and leaking.
• #26 is proper fix to avoid memory leaks by making JS function to be call-backs by default + proper resources clean-up if it was not called. Also if JS v8 executor is provided - releases unused v8 resources at the time of Java GC.

Seconds PR contains changes from 1st one already. Also changes in 1st could be used to manually release listeners if needed.

[AlexTrotsenko]

@crahda I added 3rd PR #27 - for reading V8Function as default Java interface (JsBasedCallBack) if exact Java interface is not provided.
This PR is related to #11 but can be implemented only now after PR #26 for issues #24. It's also performance efficient as it's does not used Proxy.

[AlexTrotsenko]

@crahda just in case: if you like all 3 PR - last one #27 contains changes from #26 and #25.

[caer]

@AlexTrotsenko apologies for my radio silence --- I am traveling overseas and have been swamped in other projects. I have gone ahead and approved all of your PRs, and they are now deployed to Maven Central as version 1.59-SNAPSHOT! I will deploy them as a full release (1.59) in the morning once my new PGP keys finish synchronizing...

Thank you so much for your continued support of this project.

[AlexTrotsenko]

Hi @crahda, thanks for taking care of PRs and your kind words. It's clear, that each of us has many things to deal with it - don't worry about it.

Also would like to mention, that I have found today a small bug with "injection's override" api in specific condition and already created PR - please check it when you will have a time: #29

[caer]

@AlexTrotsenko Thanks so much -- I've pulled in your PR and it is now in Maven as 1.591-SNAPSHOT. Moving forward, I'll try to only push minor builds when we add new features...