JDK-8164792 : Memory leak in JavaFX WebView
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8u102,9
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2016-08-25
  • Updated: 2017-09-22
  • Resolved: 2016-12-07
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Relates :  
JDK-8170610 - [TESTBUG] Validation of JSObject Memory leak
Relates :  
JDK-8170938 - Memory leak in JavaFX WebView
Relates :  
JDK-8170930 - WebView unit test crashes in 32-bit mode
Description
There is a memory leak in native code of JavaFX WebView of most recent JDK1.8.0 (tested on u112) related to Java/JavaScript interop. Probably releated to usage of

void initModel(JSObject thiz) {
  thiz.setMember("content", "longstring");
}

I am going to attach two versions of the application. One that does the operations in JavaScript (and seems to run without leaks in 8u112) and one that performs the same operation from Java instead (and increases the process size significantly with each "tick").

In addition to that I provide sources for the OK version and a patch to turn them into the Bad one. Should you have any issues reproducing the problem, please contact me.
Comments
This bug is now fixed in 9 and in 8u by JDK-8170938
12-12-2016
Moving to fix failed, since the fix caused a P1 regression in the unit tests, JDK-8170930, and is thus being backed out. The problem is that the current solution unconditionally calls gcUnprotect upon object disposal, which is only valid if gcProtect is always called unconditionally. This is not the case, so the solution will need to be adjusted.
08-12-2016
Changeset: 8c6d8ac28826 Author: mbilla Date: 2016-12-08 03:14 +0530 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/8c6d8ac28826
07-12-2016
Murali requested a simplified test to verify a bit of GC behavior that doesn't require changes to the production code. Here is one SimpleTestOfGCBehavior.diff
07-12-2016
We did some more testing today and all looks good. I did want to clear up the question surrounding whether we could have a case where multiple JSObject instances are created with the same peer. The answer is yes, and the underlying implementation already takes care of reference counting the peer. The native gcProtect function protects the object if this is the first reference to it, then increments a reference counter: if (!m_protectCountSet.contains(jsObject)) { JSC::JSLockHolder holder(&globalObject()->vm()); JSC::gcProtect(jsObject); } m_protectCountSet.add(jsObject); // <----- this method adds to the set and increments the counter The native gcUnprotect function unprotects the object if the count is 1 (that is, if it is the last reference), and then decrements the counter: if (m_protectCountSet.count(jsObject) == 1) { JSC::JSLockHolder holder(&globalObject()->vm()); JSC::gcUnprotect(jsObject); } m_protectCountSet.remove(jsObject); We instrumented both the JSObject code and the native code and verified that the reference counters are working as expected. So I am happy with the .03 version of the fix. +1
07-12-2016
+1: WebRev #03 looks smooth.
06-12-2016
+1
06-12-2016
webrev: http://cr.openjdk.java.net/~mbilla/8164792/webrev.03/
05-12-2016
+1 for webrev.02 with nits. + final SelfDisposer disposer = new SelfDisposer(peer, peer_type); + Disposer.addRecord(this, disposer); Storing an SelfDisposer instance in "disposer" reference doesn't have any usage, you can directly like "Disposer.addRecord(this, new SelfDisposer(peer, peer_type))". + static void dispose(long peer, int peer_type) { + unprotectImpl(peer, peer_type); + } Is it necessary to have one more indirection over unprotectImpl? You can directly call "unprotectImpl" from dispose() method.
02-12-2016
Thanks, Jaroslav. In theory, you are right that creating two JSObjects with the same peer could cause problems. In practice, I believe that the design precludes that, but it is worth checking a bit further. And thank you for the test.
01-12-2016
At my first sight the webrev.02 implementation seemed wrong. What if there are two JSObject instances for the same peer? However when I tried to write a test for that (see TestToVerifyBehaviorOfGCs.diff attachment), everything worked. So possibly the behavior is fine. At least my code can be put in as an example of a test that verifies how great everything works...
01-12-2016
webrev: http://cr.openjdk.java.net/~mbilla/8164792/webrev.02/ Unit test case will be covered as part of JDK-8170610
01-12-2016
Slight addendum to #2: it would also be good to add a check on the Java side for "peer != 0" in the DisposerRecord dispose method before calling the native dispose, and setting "peer = 0" after calling it.
01-12-2016
Additional review comments: 2) Regarding the null checks. you should check peer, rootObject, and ctx for being null (the previous native method has a good pattern to follow for the latter two). 3) Regarding the unit test, if it is easy to add a test case to the existing leak test, then please do; otherwise, please file a follow-on bug for adding this test case. 4) You do not need an instance variable for the disposer, so the following can be removed: private final SelfDisposer disposer; I will do some more testing tomorrow once you fix #2 and #4.
01-12-2016
I will review today, but here are three quick comments: 1) I prefer the pattern used in the .01 webrev of using an explicitly named static (nested) class. 2) As I mentioned privately to Murali, the native unprotectImpl method could use a couple null checks. 3) I strongly recommend adding an automated unit test for this.
30-11-2016
A lambda expression is roughly equivalent to an anonymous inner class, which holds a reference to the containing class (JSObject in this case). It is imperative that this not hold a reference, which is why a nested static class must be used as opposed to an inner class.
30-11-2016
Tested with lambda expression and dispose method not is called (Probably inner class holding JSObject this reference).
30-11-2016
disposer = new SelfDisposer(peer,peer_type); + Disposer.addRecord(this, disposer); } + private static final class SelfDisposer implements DisposerRecord { + final long peer; + final int peer_type; + + private SelfDisposer(long peer, int peer_type) { + this.peer = peer; + this.peer_type = peer_type; + } + + @Override public void dispose() { + JSObject.dispose(peer, peer_type); + } + } I think using lambda expression would produce much simpler code than having inner class. Disposer.addRecord(this, ()->unprotectImpl(this.peer, this.peer_type));
30-11-2016
Thanks @Jaroslav. That might be an issue, It should be something like below, (I mean without `this`) Disposer.addRecord(this, ()->unprotectImpl(peer, peer_type)); I'm even fine to have inner class if it exhibits good readability than lambda expressions.
30-11-2016
Alas, this kind of lambda wouldn't probably fix the memory leak problem. The disposer lambda (e.g. it's inner class generated behind the scene) would have a reference to JSObject's "this" and as such the dispose method would probably never be triggered, I am afraid. This might work: private static void registerDisposer(Object ref, long peer, long peer_type) { Disposer.addRecord(ref, () -> unprotectImpl(peer, peer_type); } While it is possible to write lambda that would work, maybe it is better to be explicit (e.g. state that the inner class is static), especially when we don't have an automated test to verify this behavior and it could easily regress.
30-11-2016
As far as I can tell the webrev.01 seems to fix the memory leak on my Linux machine. I've just done a clean build and the leak is gone and performance seems OK. Thanks.
30-11-2016
webrev: http://cr.openjdk.java.net/~mbilla/8164792/webrev.01/ I missed the space before peer_type. disposer = new SelfDisposer(peer,peer_type); I will correct this, along with other comments in next webrev.
29-11-2016
webrev: http://cr.openjdk.java.net/~mbilla/8164792/webrev.00/
25-11-2016
Done a preliminary testing 1) Loading few URLs in HelloWebView - didn't see any issues... 2) by allowing the test case to execute for 3000 times with above patch and can see JSObjects getting unProtected (from logs) Will be doing more testing
22-11-2016
I am attaching JavaFXWebViewSketchOfAFix.diff that shows how I think the issue can be fixed: * each created JSObject is associated with a WeakReference * once the WeakReference is enqueued, we notify the native code to unprotect the peer * we need a way to keep the reference alive - I am using the OBJECTS map * the OBJECTS map also ensures there is 1:1 mapping between JSObject and its peer (previously multiple JSObjects could be created for a single peer) * because of 1:1 mapping we can call unprotect as soon as a JSObject is GCed - without 1:1 mapping we would have to do counting of # of instances of JSObject per peer. The patch is just a sketch. I didn't try to compile it or run it. But based on similar experiences, I think this is an approach that could work.
22-11-2016
Discussed with Jaroslav and confirmed that setMember is not causing the leak. From the test application, looks like if you pass "null" instead of "self" in below code does not result in leak. if (SubViewModelJava(self, number)) { return; } In JNIUtilityPrivate.cpp, function convertValueToJValue, rootObject->gcProtect(object); is called for the root object and no calls for gcUnprotect is being made , which results in JS GC not able to free the root object. From logs, rootObject->gcProtect(object); is being called 5 times upon launching the application and each iteration of test application, 1000 array items being created and each array index, root object is being protected by creating 5 times.
22-11-2016
After a chat with Murali I was able to write simpler application that shows the problem. See MainApp.java - it generates objects with strings of random length and sends them to Java to count the string average length. The application leaks native memory. Btw. the same could be achieved without strings, just strings leak the memory faster. The hypothesis we have right now is: the native JavaScript object that gets send into Java as JSObject is marked as "protected" and never garbage collects in the JavaScriptCore engine. Thus we see no Java heap leaks, but steady increase in consumption of the native memory (by JavaScriptCore engine).
22-11-2016
I've just attached code for the "NoKnockout" application. Use as: $ unzip WebKit-NoKnockout-src.zip $ cd leaks $ mvn install $ mvn -f client/pom.xml exec:exec to simulate the memory leak.
21-11-2016
[~jtulach] As per my previous comments and from the attached application, below setMember function is not causing the leak. I request reporter to provide more info about the leak which they observed. window.setMember("javaCallback2", (new StringBuilder()).append("have a long string to take more memory .............................take more me" + "mory take more memory take more memory take more memory take more memory take mo" + "re memory take more memory take more memory take more memory take more memory ta" + "ke more memory take more memory take more memory take more memory take more memo" + "ry take more memory take more memory take more memory take more memory take more" + " memory take more memory take more memory take more memory take more memory take" + " more memory take more memory take more memory take more memory take more memory" + " take more memory ." ).append(number).toString());
21-11-2016
[~jtulach] Can you explain little more when u meant "It seems correct" ? If you want to build a debug native build(webkit) , you can use below command from folder "rt" : This will take some time. gradle -PCOMPILE_WEBKIT=true -PCONF=DebugNative If you want to compile non-debug mode, then u can use below command: gradle -PCOMPILE_WEBKIT=true
11-11-2016
I built OpenJFX too and started to check the code. It seems correct. Now I am about to use Oracle Studio Analyzer to find out where most of the leaks are happening. One problem that slows me down: when I change a .cpp file in WebKit sources, how do I rebuild quickly? "gradle assemble" (with native build enabled) takes too long...
11-11-2016
Debugged with logs for above setMember code and in this case, code gets executed for String case. Earlier leak was fixed for a similar bug JDK-8161053 if (env->IsInstanceOf(val, clString)) { if (env->IsInstanceOf(val, clString)) { JSStringRef value = asJSStringRef(env, (jstring) val); JSValueRef jsvalue = JSValueMakeString(ctx, value); JSStringRelease(value); return jsvalue; } Checking further if there any issue in JSValueMakeString.
11-11-2016
Attaching the sample application (WebEngineNotReleasingMemory.java) where i tested by passing below string to setMember. window.setMember("javaCallback2", (new StringBuilder()).append("have a long string to take more memory .............................take more me" + "mory take more memory take more memory take more memory take more memory take mo" + "re memory take more memory take more memory take more memory take more memory ta" + "ke more memory take more memory take more memory take more memory take more memo" + "ry take more memory take more memory take more memory take more memory take more" + " memory take more memory take more memory take more memory take more memory take" + " more memory take more memory take more memory take more memory take more memory" + " take more memory ." ).append(number).toString()); javaCallback2 is associated with the buttion 'Hi" as below: "<button type=\"button\" onClick=\"javaCallback2.sayHey()\">Hi</button>" + When you click on button "Hi", sayHey() function supposed to be called and prints "Hey" public void sayHey() { System.out.println("Hey"); } When i tested this scenario, "Hey" is not printed, which indicates that the local object which is passed to setMember is already GC'ed . But when i pass window.setMember("javaCallback", this);, and when i click on Hello button, "World!" is printed (inspite of running GC), which is expected behaviour. I suggest the reporter can check with the attached simple application to check again for setMember (assumed that above setMember is causing the leak).
11-11-2016
Hello Muralli, Abhijit. I don't seem to be able to access the heap dump. However from the [attached picture](https://bugs.openjdk.java.net/secure/attachment/64256/dukeScript-heapdump.png) it is clear that the application classes aren't leaking at all - thus the leak must be in other classes or in infrastructure. I've tried to run the WebViewFX-App-NoKnockout.zip on JDK1.8.0_112 for five minutes and observe it in jvisualvm. I was using "Sample"/"Memory"/"Deltas" - after "Perform GC" the amount of allocated objects always got down close to zero difference. Yet the `top` program showed that 50% of physical memory of my computer was occupied by the application. I conclude that it clearly indicates a native memory leakage and I am dare to say it is a bug in WebView Java interop. Is there anything else I can help you guys reproduce the issue? It seems that we aren't moving forward, right?
07-11-2016
Based on Murali's suggestion, checked this issue (WebViewFX-App-NoKnockout.zip) with jvisualvm more than one hour and attached the corresponding snapshot, heap dump, memory consumption files. From the result, observed that heap consumption is gradually increasing.
27-10-2016
WebViewFX-App-NoKnockout.zip is the rewritten version of the application that doesn't use knockout.js library, but otherwise keeps the same behavior. After unzipping it can be invoked as: $ java -jar WebViewFX-App-NoKnockout/app.jar and it starts to leak significantly (because of the three fields assignment via jsObject.setMember). You can also run the same application as $ java -Dinit.in.js=true -jar WebViewFX-App-NoKnockout/app.jar and then the enormous leak is gone, as the member assignments are performed on the JavaScript side. Let me know if I should attach also the Maven sources or if you are OK with the updated binary.
17-09-2016
I tested the WebViewFX-App-OK and WebViewFX-App-BAD against 8u112-b07 and JDK 9 on windows 7, 64 bit I allowed the test for 100 iterations and observed that though both App-OK and App-BAD versions are increasing memory constantly over period of time, but the pace of increase in memory consumption is more in App-BAD version. I checked the java heap growth (against Maxsize of 32m) for 100 iterations and java heap consumption remains constant (~12M). Hence seems like there is leak in native side...currently debugging native code..
30-08-2016
Yes, b07 is newer.
30-08-2016
I believe I am using 8u112-b07 and I am still able to reproduce the leaking problem: WebViewFX-App-Bad$ ~/bin/jdk1.8.0_112/bin/java -version java version "1.8.0_112" Java(TM) SE Runtime Environment (build 1.8.0_112-b07) Java HotSpot(TM) 64-Bit Server VM (build 25.112-b07, mixed mode) WebViewFX-App-Bad$ ~/bin/jdk1.8.0_112/bin/java -jar app.jar is b07 newer than b03 or b06, right?
30-08-2016
Possible duplicate of JDK-8161053 : Passing objects between JavaScript (JavaFX / WebKit) and Java causes a memory leak. Relates to : JDK-8151459 : Validation of new behaviour for JS callback memory leak JDK-8089681: JDK-8089681 : WebView leaks memory when containing object acts as javascript callback handler
29-08-2016
Also seen with 8u112. Removed affected version to avoid this showing up on our 16_04 radar.
25-08-2016
Thanks for the information.
25-08-2016
Hello Kevin. The state of interop in JavaFX WebView in previous JDK versions were horrible. JDK1.8.0_112 fixes many leaks and it is really a great improvement. Few are still left, so I reported this bug. Are you asking "is it a regression" because of the priority I assigned? Feel free to adjust it to your needs. But as far as I know the customers using my HTML/Java API don't have any workaround (and I can't envision one). So I started with P2.
25-08-2016
I am attaching the whole sources of the "OK" application for completeness. In case you need to work with them, here is a howto: $ unzip WebViewFX-Maven.zip $ cd leaks/ $ mvn clean install # to run the app: $ mvn -f client/pom.xml exec:exec
25-08-2016
The difference between the OK version of the application and the Bad version are in this single diff: https://bugs.openjdk.java.net/secure/attachment/62630/WebViewFX-App.diff The diff is also shown below. The only difference is that instead of assigning a JavaScript object attributes "num", "name", "content" in JavaScript, we now assign them in Java. The "content" is assigned by a very long string, so I assume that this is the source of the memory leak - the string is never garbage collected in the JavaScript engine. diff --git a/client/src/main/webapp/pages/js/test.js b/client/src/main/webapp/pages/js/test.js index e71cb67..fb31e96 100644 --- a/client/src/main/webapp/pages/js/test.js +++ b/client/src/main/webapp/pages/js/test.js @@ -2,9 +2,6 @@ function SubViewModel(number) { var self = this; SubViewModelJava(self, number); - self.num = number; - self.name = "anything"; - self.content = "long string" + number; } diff --git a/js/src/main/java/com/dukescript/test/dukescriptArrayDirect/js/Dialogs.java b/js/src/main/java/com/dukescript/test/dukescriptArrayDirect/js/Dialogs.java index b0b227b..795977d 100644 --- a/js/src/main/java/com/dukescript/test/dukescriptArrayDirect/js/Dialogs.java +++ b/js/src/main/java/com/dukescript/test/dukescriptArrayDirect/js/Dialogs.java @@ -35,8 +35,11 @@ public final class Dialogs { + "}\n") public static native void registerSubViewModel(); - static Object initModel(Object thiz, int num) { + static Object initModel(Object thiz, int number) { JSObject js = (JSObject) thiz; + js.setMember("num", number); + js.setMember("name", "anything"); + js.setMember("content", "long string" + number); return js; }
25-08-2016
Now repeat the same action with the "Bad" version of the application. Download and unzip the WebViewFX-App-Bad.zip and invoke: $ jdk1.8.0_112/bin/java -jar WebViewFX-App-Bad/app.jar click the button few times to start "ticks" and observe in `top` the memory goes up with each "tick", I see: 6333712 596556 70352 7,6 0:20.86 java 6637080 772440 70352 9,8 0:29.95 java 6928424 927612 70352 11,8 0:40.46 java e.g. the memory goes up quite fast.
25-08-2016
Unzip WebViewFX-App-OK and perform: $ jdk1.8.0_112/bin/java -jar WebViewFX-App-OK/app.jar a JavaFX window opens up with WebView and sample application with a button. Click the button to perform a "tick" every two second. Click the button multiple times to increase the load. On each "tick" the system generates an array of 1000 elements and replaces it in a knockout.js observable. The previous content is garbage collected. Start a utility like {{top}} and observe the process by memory usage. As can be observed the "OK" version of the application allocates some memory and then it remains constant for hours.
25-08-2016
Is this a regression?
25-08-2016