Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLeo Ufimtsev2018-04-04 13:41:20 +0000
committerLeo Ufimtsev2018-04-04 19:11:40 +0000
commit6b73a19165f47cd767b060aa616bc877b5fc2d99 (patch)
tree2e9bc7522364a0d4b671f9e377a11c6544e77459
parent6821d31b9c58db30d37d7b468419a72377bda431 (diff)
downloadeclipse.platform.swt-6b73a19165f47cd767b060aa616bc877b5fc2d99.tar.gz
eclipse.platform.swt-6b73a19165f47cd767b060aa616bc877b5fc2d99.tar.xz
eclipse.platform.swt-6b73a19165f47cd767b060aa616bc877b5fc2d99.zip
Bug 530678 [webkit2] UI hangs after navigating through Javadoc hover,
when clicking an icon or link to an image. TL;DR: Fix for bug. Details: Clicking on a javadoc like below, (after 2 or 3 times), breaks javadoc. /** <a href="https://bugs.eclipse.org/bugs/attachment.cgi?id=272647">Screenshot</a> */ It un-breaks if you manually kill Webkit* processes. I.e, Webkit process deadlocks if Browser is disposed in the middle of a callback. The fix is to delay disposal until callback is completed. This is achived by leaving a dangling reference until next display loop is executed. This also fixes the bug: 494158 - [Webkit1][Webkit2] JVM crash when javadoc hover has <embed> tags. In addition, I'm adding a jUnit to verify that the fix hasn't introduced memory leaks. (E.g if object is not unreferenced, then one can observe a memory leak. So by extension this fix does not introduce a memory leak since the test passes). (Note, it's not added to be ran automatically becaues it takes 5+ minutes to run). Tests & verification: (gtk3.22, Webkit2.18). - jUnit: AllTests - Child Eclipse opens external links (multiple times) in javadoc correctly. - Various Browser Snippets. - Test_Memory_leak.test_Browser(). https://bugs.eclipse.org/bugs/show_bug.cgi?id=530678 Change-Id: I2a0f0f7e2b44bce67a1212bfa4205561942e59de Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>
-rw-r--r--bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java23
-rw-r--r--tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/memoryleak/Test_Memory_Leak.java64
2 files changed, 85 insertions, 2 deletions
diff --git a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
index f146139aff..498038db89 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT WebKit/gtk/org/eclipse/swt/browser/WebKit.java
@@ -1057,6 +1057,8 @@ public void create (Composite parent, int style) {
// As of Webkitgtk 2.18, webkitgtk2 crashes if the first instance of webview is not referenced when JVM shuts down.
// There is a exit handler that tries to dereference the first instance [which if not referenced]
// leads to a crash. This workaround would benefit from deeper investigation (find root cause etc...).
+ // [edit] Bug 530678. Note, it seems that as of Webkit2.18, webkit auto-disposes itself if parent get's disposed.
+ // While not directly related, see onDispose() for how to deal with disposal of this.
if (WEBKIT2 && !bug522733FirstInstanceCreated && vers[0] == 2 && vers[1] >= 18) {
bug522733FirstInstanceCreated = true;
OS.g_object_ref(webView);
@@ -2396,8 +2398,25 @@ void onDispose (Event e) {
postData = null;
headers = null;
htmlBytes = null;
- } else if (WEBKIT2) {
- webView = 0; // Note, Webkit2 disposes it self on it's own.
+ }
+ if (WEBKIT2 && WebKitGTK.webkit_get_minor_version() >= 18) {
+ // Bug 530678.
+ // * As of Webkit 2.18, (it seems) webkitGtk auto-disposes itself when the parent is disposed.
+ // * This can cause a deadlock inside Webkit process if WebkitGTK widget's parent is disposed during a callback.
+ // This is because webkit process is waiting for it's callback to finish which never completes
+ // because parent's disposal also disposed webkitGTK widget. (Note Webkit process vs WebkitGtk widget).
+ // * To break the deadlock, we unparent webkitGtk temporarily and unref (dispose) it later after callback is done.
+ //
+ // If you change dispose logic, to check that you haven't introduced memory leaks, test via:
+ // org.eclipse.swt.tests.junit.memoryleak.Test_Memory_Leak.test_Browser()
+ OS.g_object_ref (webView);
+ GTK.gtk_container_remove (GTK.gtk_widget_get_parent (webView), webView);
+ long /*int*/ webViewTempRef = webView;
+ browser.getDisplay().asyncExec(() -> {
+ GTK._gtk_widget_destroy(webViewTempRef);
+ OS.g_object_unref (webViewTempRef);
+ });
+ webView = 0;
}
}
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/memoryleak/Test_Memory_Leak.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/memoryleak/Test_Memory_Leak.java
new file mode 100644
index 0000000000..b6a0acf69b
--- /dev/null
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/memoryleak/Test_Memory_Leak.java
@@ -0,0 +1,64 @@
+package org.eclipse.swt.tests.junit.memoryleak;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.browser.Browser;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
+import org.junit.Test;
+
+/**
+ * Test Widgets for memory leaks. Used when updating dispose logic.
+ *
+ * Due to lack of better alternative, the current mechanism creates & disposes a widget in a loop.
+ * If the test makes it to the end without crashing your system, then you have no (major?) leaks.
+ *
+ * Run these on demand if updating dispose logic of a particular widget.
+ *
+ * Note:
+ * - The tests are a bit long, so they're not part of the main test suite.
+ * - Note, JNI != Java memory leaks. JNI leaks are not detected by Java profilers as they occur *outside* of the heap.
+ * Finding JNI leaks is not a trivial matter as typical C memory tools see the JVM as a memory leak itself.
+ * It's possible thou. Have not tried myself but see:
+ * https://stackoverflow.com/questions/33334126/how-to-find-memory-leaks-in-java-jni-c-process
+ * https://gdstechnology.blog.gov.uk/2015/12/11/using-jemalloc-to-get-to-the-bottom-of-a-memory-leak/
+ */
+public class Test_Memory_Leak {
+
+ static int COUNT_PRINT_PER_ROW = 50;
+
+ /**
+ * Create and dispose Browser instances.
+ *
+ * If this test runs at linear speed and passes, then it's fairly safe to say you have no memory leaks.
+ * A typical run will take 5 minutes.
+ *
+ * If you have a memory leak in the dispoal logic, then the loop eventually slows down and the test crashes.
+ * You would see 'memory pressure' errors. The Java process of the jUnit would grow significantly (100's of mbs)
+ * On my machine with Intel i7 & 16 GB of ram, this occurs at the ~420th iteration.
+ * (although with my testing, without memory leaks, it grows a little bit (by 100mb by end of test)).
+ */
+ @Test
+ public void test_Browser() {
+ Display display = new Display ();
+ Shell shell = new Shell(display);
+ shell.setLayout(new FillLayout());
+ shell.open ();
+
+ Browser browser;
+ int count = 50_000;
+
+ for (int i = 1; i <= count; i++) {
+ browser = new Browser(shell, SWT.None);
+ browser.setUrl("http://www.google.com");
+ while (display.readAndDispatch()) {
+ // This loop is needed because some disposal is delayed and done asynchronously in main loop.
+ // This loop typically performs ~12 iterations.
+ }
+ if (i != count) browser.dispose();
+ if (i % (COUNT_PRINT_PER_ROW) == 0) System.out.println();
+ System.out.print(i+ " ");
+ }
+ System.out.println();
+ }
+}

Back to the top