JDK-8176729 : com.sun.webkit.dom.NodeImpl#SelfDisposer is not called
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows_7
  • CPU: x86_64
  • Submitted: 2017-03-10
  • Updated: 2017-10-11
  • Resolved: 2017-04-23
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 10 JDK 8 JDK 9
10Fixed 8u151Fixed 9.0.1Fixed
Related Reports
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
Windows 7x64
Mac OS X 10.11.6

A DESCRIPTION OF THE PROBLEM :
If you access the document and nodes of a javafx.scene.web.WebEngine from Java, the SelfDisposer of com.sun.webkit.dom.NodeImpl does not get called. Thus, the underlying JNI-Object is not disposed and the memory used by the native code increases but never decreases with each new document loaded.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1. Create a new Swing applikation
2. Add a JFXPanel with a WebView to the applikation
3. Optional: set the history maximum size of the WebEngine to 0
4. Load new html document into the WebEngine
5. Access the document and its nodes
6. Repeat no. 4 and no. 5 at leisure

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Memory-Usage of the JVM stays more or less the same.
Amount of NodeImpl#SelfDisposer in heap stays more or less the same
ACTUAL -
Memory-Usage of the JVM increases steadily
Heap-Usage stays more or less the same
Amount of NodeImpl#SelfDisposer in heap successively increases

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import javafx.application.Platform;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.concurrent.Worker;
import javafx.embed.swing.JFXPanel;
import javafx.scene.Scene;
import javafx.scene.layout.BorderPane;
import javafx.scene.web.WebEngine;
import javafx.scene.web.WebView;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.w3c.dom.html.HTMLElement;
import org.w3c.dom.html.HTMLInputElement;
import org.w3c.dom.html.HTMLSelectElement;
import org.w3c.dom.html.HTMLTextAreaElement;

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.concurrent.CountDownLatch;

/**
 * Created by mathias on 09.03.17.
 */
public class NodeImplMemoryLeak {

    public static void main(String[] args) throws Exception {
        new NodeImplMemoryLeak();
    }

    private BorderPane borderPane;
    private WebView webView;

    public NodeImplMemoryLeak() throws Exception {
        final URL resource1 = NodeImplMemoryLeak.class.getResource("page1.html");
        final URL resource2 = NodeImplMemoryLeak.class.getResource("page2.html");
        final ArrayList<URL> resources = new ArrayList<>(Arrays.asList(resource1, resource2));

        new JFXPanel();

        final CountDownLatch countDownLatch = new CountDownLatch(1);

        Platform.runLater(() -> {
            webView = new WebView();

            final WebEngine engine = webView.getEngine();
            webView.setContextMenuEnabled(false);
            webView.getEngine().getHistory().setMaxSize(0);
            webView.getEngine().getLoadWorker().stateProperty().addListener(new ChangeListener<Worker.State>() {
                @Override
                public void changed(final ObservableValue<? extends Worker.State> observable, final Worker.State oldValue, final Worker.State newValue) {
                    if (newValue == Worker.State.SUCCEEDED) {
                        contentEditable(engine.getDocument());
                    }
                }
            });

            final URL resource = resources.get(1);
            webView.getEngine().load(resource.toExternalForm());

            countDownLatch.countDown();

            borderPane = new BorderPane();
            borderPane.setCenter(webView);
        });

        countDownLatch.await();

        try {
            SwingUtilities.invokeAndWait(() -> {
                final JFXPanel jfxPanel = new JFXPanel();
                final Scene scene = new Scene(borderPane);
                jfxPanel.setScene(scene);

                final JButton button = new JButton(new AbstractAction("Toggle") {
                    @Override
                    public void actionPerformed(final ActionEvent e) {
                        final URL resource = resources.remove(0);
                        resources.add(resource);

                        Platform.runLater(() -> webView.getEngine().load(resource.toExternalForm()));
                    }
                });

                final JPanel content = new JPanel(new BorderLayout());
                content.add(jfxPanel, BorderLayout.CENTER);
                content.add(button, BorderLayout.SOUTH);

                final JFrame frame = new JFrame();
                frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
                frame.setContentPane(content);
                frame.pack();
                frame.setVisible(true);
            });
        } catch (final InterruptedException | InvocationTargetException e) {
            throw new IllegalStateException(e);
        }
    }

    private static void contentEditable(final Document document) {
        if (document == null) {
            return;
        }
        final NodeList bodyList = document.getElementsByTagName("body");
        final Element element = (Element) bodyList.item(0);
        element.setAttribute("contenteditable", "false");
        checkElement(element);
    }

    private static void checkElement(final Element element) {
        final NodeList list = element.getChildNodes();
        for (int n = 0; n < list.getLength(); n++) {
            final Node child = list.item(n);
            if (child.getNodeType() == Node.ELEMENT_NODE) {
                final HTMLElement htmlElement = (HTMLElement) child;
                if (htmlElement instanceof HTMLInputElement) {
                    ((HTMLInputElement) htmlElement).setDisabled(true);
                } else if (htmlElement instanceof HTMLTextAreaElement) {
                    ((HTMLTextAreaElement) htmlElement).setReadOnly(true);
                } else if (htmlElement instanceof HTMLSelectElement) {
                    ((HTMLSelectElement) htmlElement).setDisabled(true);
                }
                checkElement(htmlElement);
            }
        }
    }
}

page1.html: (replace ... in img src)
<html dir="ltr"><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body contenteditable="true"><h1><font face="Segoe UI" size="6">Test 2</font></h1><h2><font face="Segoe UI">Bild</font></h2><p><font face="Segoe UI"><br></font></p><p style="margin-top: 0"><img src="���"/></p></body></html>

page2.html: (replace ... in img src)
<html dir="ltr"><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body contenteditable="true"><h1><font face="Segoe UI" size="6">Test 1</font></h1><h2><font face="Segoe UI" size="5">Input</font></h2><p style="margin-top: 0"><input type="text">
<input type="radio"></p><h2><font face="Segoe UI" size="5">Bild</font></h2><p style="margin-top: 0"><img src="..."/></p></body></html>
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Using the following workaround, the SelfDisposer gets called periodically.
The memory usage still increases but much slower.

import com.sun.webkit.Disposer;
import com.sun.webkit.dom.NodeImpl;
import javafx.application.Platform;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jetbrains.annotations.NonNls;
import securiton.uls.utilities.lang.concurrent.NamedExecutorServiceFactory;

import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

/**
 * Ruf, wenn initialisiert, zyklisch die SelfDisposer von {@link NodeImpl} auf, da JavaFX dies nicht selbst tut.
 * Ansonsten w��rde der Speicher, der von WebKit im nativen Teil (JNI) reserviert wurde, nicht freigegeben.
 * <p/>
 * Siehe UMSRELEASE-6045
 * <p/>
 * Created by mathias on 09.03.17.
 */
public final class NodeDisposalWorkaround {
    @NonNls
    private static final Log log = LogFactory.getLog(NodeDisposalWorkaround.class);

    private NodeDisposalWorkaround() {
    }

    private static final class Worker {
        private static final Worker INSTANCE = new Worker();

        private Worker() {
            final ScheduledExecutorService executor =
                    NamedExecutorServiceFactory.createForDaemon().getSingleThreadScheduledExecutor(Worker.class);
            executor.scheduleWithFixedDelay(() -> Platform.runLater(this::disposeAllCollected),
                    1, 1, TimeUnit.SECONDS);
        }

        private void disposeAllCollected() {
            try {
                final Field hashTableField = NodeImpl.class.getDeclaredField("hashTable");
                hashTableField.setAccessible(true);
                final Object hashTable = hashTableField.get(null);

                for (int i = 0; i < Array.getLength(hashTable); i++) {
                    final Disposer.WeakDisposerRecord selfDisposer = (Disposer.WeakDisposerRecord) Array.get(hashTable, i);
                    dispose(selfDisposer);
                }

            } catch (final Exception e) {
                log.warn("Error doing dispose", e);
            }
        }

        private void dispose(final Disposer.WeakDisposerRecord selfDisposer) throws Exception {
            if (selfDisposer == null) {
                return;
            }
            final Field nextField = selfDisposer.getClass().getDeclaredField("next");
            nextField.setAccessible(true);
            final Disposer.WeakDisposerRecord nextDisposer = (Disposer.WeakDisposerRecord) nextField.get(selfDisposer);

            if (selfDisposer.get() == null) {
                selfDisposer.dispose();
            }

            dispose(nextDisposer);
        }

        public static Worker create() {
            return INSTANCE;
        }
    }

    public static void disposeAllCollected() {
        Worker.create();
    }
}



Comments
Changeset: 5865d28f81b1 Author: mbilla Date: 2017-04-23 16:22 +0530 URL: http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/5865d28f81b1
23-04-2017

The .03 version looks great. +1
22-04-2017

webrev: http://cr.openjdk.java.net/~mbilla/8176729/webrev.03/ Corrected Copyright Header for disposer.java and LeakTest comment.
22-04-2017

Thanks [~kcr] for the comments. I Incorporated above comments. webrev: http://cr.openjdk.java.net/~mbilla/8176729/webrev.02/
22-04-2017

The fix itself looks fine to me. However, there are a few issues with the test: 1. The test fails for me if I run all of ":web:test". This seems to be a side effect from previous tests (which run in the same JVM), possibly one of the tests in IrresponsiveScriptTest leaves a strong reference to a DOM node lying around? A possible solution is to capture the current value of hashSize at the beginning of the test (after a GC and sleep) and use that as the expected value. 2. Test failures are more informative if you use assertEquals(expected, observed) rather than assertTrue(expected == observed). In the former case, the observed value is printed out on failure. 3. Your for loop with the GC / sleep does not break out early so is sleeping longer than is likely needed (see the other tests for a better example). 4. I recommend creating more than one DOM node in the test. 5. You should verify the expected number of peers after creation, while you still hold the references to the DOM objects. As long as you need a new webrev, here are two minor things you might correct: 6. NIT: there should be a blank line after the copyright header and before the pacakge statement in the new shim file. 7. NIT: Can you update the copyright year in Disposer.java?
19-04-2017

webrev: http://cr.openjdk.java.net/~mbilla/8176729/webrev.01/ Removed unused imports in LeakTest.java
17-04-2017

webrev: http://cr.openjdk.java.net/~mbilla/8176729/webrev.00/ When instrumented java-wrappers while testing for NodeImplMemoryLeak.java , i could see execution of NodeImpl, but not DOMWindowImpl. Hence I did not included DOMWindowImplShim.java file in above webrev as i could not find a use case to test DOMWindowImpl.
17-04-2017

Executed unit test cases and all PASSED. Currently writing unit test case for this issue.
11-04-2017

Able to reproduce this leak with 8u121 (Windows 7) by using NodeImplMemoryLeak. Currently out of 27 files which implemented DisposerRecord, 25 files are calling Disposer.addRecord. DOMWindowImpl.java and NodeImpl.java are not called Disposer.addRecord. Only difference is DOMWindowImpl and NodeImpl extends Disposer.WeakDisposerRecord instead of implements DisposerRecord
30-03-2017

Tested NodeImplMemoryLeak by adding Disposer.addRecord in DOMWindowImpl and NodeImpl and seems like memory stabilized (after a while) and no spike in memory observed after running 30 mins..
30-03-2017

Attached a preliminary patch. It will need to be fully evaluated and tested. We will also need a unit test.
22-03-2017

I have attached a modified version of the test program (just the java source, since that is all I modified) that has a second button which will run a timeline to toggle between the two web pages every 500 msec (so you don't have to keep pressing the "Toggle" button).
21-03-2017

I can reproduce this bug. As it is a fairly significant leak, I am raising the priority to P3. The problem is that the SelfDisposer record is never added to the Disposer queue.
21-03-2017

Since there is a test case that does not rely on internal APIs, I am reopening this bug. We will take a look at it.
21-03-2017

I think the bug reporter misinterpreted my request. I want to see the results from a run of a test case that only uses publicly exported packages and does not use anything from com.sun.javafx.*. Marking as incomplete again pending such a test case.
17-03-2017

Reply from Submitter ---------------------------------- I was able to start my test application with jdk 9 ea 160 using the option --add-opens=javafx.graphics/com.sun.javafx.tk=ALL-UNNAMED The problem unfortunately remains, the SelfDisposers from NodeImpl still add up. The memory usage still increases albeit much slower (about 50%). --------------------------------------
17-03-2017

Sent email to submitter: ------------------------------------- Test app is using internal interfaces: import com.sun.webkit.Disposer; import com.sun.webkit.dom.NodeImpl; These are not part of the public API and must not be used by an application. I would recommend you to verify the original program on JDK 9 without using above interfaces and let me know the results. ---------------------------------------------------
17-03-2017

I see that the test app is using internal interfaces: import com.sun.webkit.Disposer; import com.sun.webkit.dom.NodeImpl; These are not part of the public API and must not be used by an application. I recommend that the bug reporter try the original program without the workaround on JDK 9.
15-03-2017

Reply from Submitter --------------------------------- I am still able to reproduce the behaviour using JDK 8u152. The memory usage still increases continously. Profiling my test application using YourKit the SelfDisposers from NodeImpl still remain in memory and are strong reachable. I had no luck starting my test application using JDK9ea+160: --- Exception in thread "main" java.lang.IllegalAccessError: NodeImplMemoryLeak (in unnamed module @0x2c13da15) cannot access class com.sun.javafx.tk.Toolkit (in module javafx.graphics) because module javafx.graphics does not export com.sun.javafx.tk to unnamed module @0x2c13da15 at NodeImplMemoryLeak.<init>(NodeImplMemoryLeak.java:54) at NodeImplMemoryLeak.main(NodeImplMemoryLeak.java:41) --- In line 54 I just do "final WebEngine engine = webView.getEngine();". My testcase only directly uses Classes from java. and javafx. packages., so I have no clue what's going wrong here. Thus I cannot verify neither problem nor solution using JDK 9. ---------------------------------------------------------------------------------
15-03-2017

Sent email to submitter: ------------------------------------- Could you please verify this issue in latest early access version of JDK 9 or JDK 8u152. I have found similar issue which has been fixed in these versions. Download links for your reference: JDK 9 - https://jdk9.java.net/download/ JDK 8u152 - https://jdk8.java.net/download.html --------------------------------------------------------------
14-03-2017