Community
Participate
Working Groups
To reproduce, run snippet (with VM arguments ) and observe ever-increasing RES memory consumption: public class TestProgramsLeak { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setSize(600, 400); shell.setLayout(new FillLayout()); shell.setText("Demo Program.getPrograms() leak"); Composite composite = new Composite(shell, SWT.NONE); composite.setLayout(new FillLayout(SWT.VERTICAL)); Text text = new Text(composite, SWT.BORDER | SWT.READ_ONLY | SWT.MULTI | SWT.V_SCROLL); text.setText(""); shell.open(); AtomicBoolean c = new AtomicBoolean(true); Thread t = new Thread(new Runnable() { @Override public void run() { while (c.get()) { if (!display.isDisposed()) { display.asyncExec(new Runnable() { @Override public void run() { if (!display.isDisposed()) { StringBuilder sb = new StringBuilder(); Program[] programs = Program.getPrograms(); sb.append("Programs count: " + programs.length); sb.append(System.lineSeparator()); sb.append("List of programs:"); sb.append(System.lineSeparator()); for (Program program : programs) { sb.append(program.getName()); sb.append(System.lineSeparator()); } text.setText(sb.toString()); } } }); } try { Thread.sleep(250); } catch (InterruptedException e) { } } } }); t.start(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } c.set(false); try { t.join(); } catch (InterruptedException e) { } display.dispose(); } } Extra info from Alexander Miloslavskiy: > Documentation for 'g_app_info_get_all()' [1] indicates "transfer full", > and there is a tooltip saying "The caller owns the data, and is > responsible for free it". Compare that to "transfer container", which is > described as "The caller owns the data container, but not the data > inside it". > > Therefore, items in the list also need to be released. See for example > GTK sources: > > 'gtk\gtkappchooserwidget.c' > Line 751 > all_applications = g_app_info_get_all (); > <...> > g_list_free_full (all_applications, g_object_unref); > > Here, 'g_list_free_full()' is used with 'g_object_unref()' to also call > 'g_object_unref()' for every element of the list. SWT forgets to do that.
valgrind leak report is e.g.: ==1379== 31,210,322 (2,749,200 direct, 28,461,122 indirect) bytes in 13,746 blocks are definitely lost in loss record 2 of 14,057 ==1379== at 0x4C2B017: malloc (vg_replace_malloc.c:380) ==1379== by 0x2A31F68D: g_malloc (in /usr/lib64/libglib-2.0.so.0.5600.1) ==1379== by 0x2A336C8D: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5600.1) ==1379== by 0x2A3371ED: g_slice_alloc0 (in /usr/lib64/libglib-2.0.so.0.5600.1) ==1379== by 0x2A0AF466: g_type_create_instance (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==1379== by 0x2A0931FC: ??? (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==1379== by 0x2A095120: g_object_new_valist (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==1379== by 0x2A095468: g_object_new (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==1379== by 0x29DD14F1: g_desktop_app_info_new_from_filename (in /usr/lib64/libgio-2.0.so.0.5600.1) ==1379== by 0x29DD274C: g_app_info_get_all (in /usr/lib64/libgio-2.0.so.0.5600.1) ==1379== by 0x269E8872: Java_org_eclipse_swt_internal_gtk_OS_g_1app_1info_1get_1all (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-pi3-gtk-4944r22.so) ==1379== by 0x1140670F: ??? ==1379== by 0x11400A4F: ??? ==1379== by 0x11400CBF: ??? ==1379== by 0x11400CBF: ??? ==1379== by 0x11400FB7: ??? ==1379== by 0x11400F72: ??? ==1379== by 0x11BCD703: ??? ==1379== by 0x113F7BC8: ??? ==1379== by 0x66FE052: JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, Thread*) (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==1379== by 0x676910C: jni_invoke_static(JNIEnv_*, JavaValue*, _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, Thread*) [clone .isra.165] (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==1379== by 0x676CF92: jni_CallStaticVoidMethod (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==1379== by 0x52723DE: JavaMain (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/jli/libjli.so) ==1379== by 0x5058E24: start_thread (pthread_create.c:308) ==1379== by 0x577A34C: clone (clone.S:113)
In the description I missed adding the VM arguments I use: "-Xmx128M -Xms128M -XX:+AlwaysPreTouch" (to ensure the heap is not growing, so that the native memory leak is easier to spot.
This doesn't fix the leak: diff --git a/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java b/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java index 1bcba8851d..12c003257a 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java @@ -331,7 +331,14 @@ static Program[] getPrograms(Display display) { } list = OS.g_list_next(list); } - if (applicationList != 0) OS.g_list_free(applicationList); + if (applicationList != 0) { + long list_iterator = list; + while (list_iterator != 0) { + OS.g_object_unref(list); + list = OS.g_list_next(list); + } + OS.g_list_free(applicationList); + } return programs.toArray(new Program[programs.size()]); } And this results in GTK3 critical errors: (SWT:107044): GLib-GObject-CRITICAL **: 16:41:58.040: g_object_unref: assertion 'G_IS_OBJECT (object)' failed (I'm using the same code as the code found here, no idea what goes wrong: https://github.com/GNOME/gtk/blob/master/gtk/gtkappchooserwidget.c#L698) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c index c8129360c5..0c0dc76f75 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c @@ -12685,6 +12685,16 @@ JNIEXPORT void JNICALL OS_NATIVE(g_1list_1free) } #endif +#ifndef NO_g_1list_1free_1full +JNIEXPORT void JNICALL OS_NATIVE(g_1list_1free_1full) + (JNIEnv *env, jclass that, jlong arg0) +{ + OS_NATIVE_ENTER(env, that, g_1list_1free_1full_FUNC); + g_list_free_full((GList *)arg0, g_object_unref); + OS_NATIVE_EXIT(env, that, g_1list_1free_1full_FUNC); +} +#endif + #ifndef NO_g_1list_1last JNIEXPORT jlong JNICALL OS_NATIVE(g_1list_1last) (JNIEnv *env, jclass that, jlong arg0) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c index aba908dbf8..b2291a46ef 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c @@ -1051,6 +1051,7 @@ char * OS_nativeFunctionNames[] = { "g_1list_1append", "g_1list_1data", "g_1list_1free", + "g_1list_1free_1full", "g_1list_1last", "g_1list_1length", "g_1list_1model_1get_1item", diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h index 8fb967b877..d88b3ff5a1 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h @@ -1025,6 +1025,7 @@ typedef enum { g_1list_1append_FUNC, g_1list_1data_FUNC, g_1list_1free_FUNC, + g_1list_1free_1full_FUNC, g_1list_1last_FUNC, g_1list_1length_FUNC, g_1list_1model_1get_1item_FUNC, diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java index 1474291045..1dadd7816f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java @@ -1069,6 +1069,8 @@ public static final native long g_list_append(long list, long data); public static final native long g_list_data(long list); /** @param list cast=(GList *) */ public static final native void g_list_free(long list); +/** @param list cast=(GList *) */ +public static final native void g_list_free_full(long list); /** * @param list cast=(GList *) */ diff --git a/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java b/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java index 1bcba8851d..017f6ff29f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Program/gtk/org/eclipse/swt/program/Program.java @@ -331,7 +331,7 @@ static Program[] getPrograms(Display display) { } list = OS.g_list_next(list); } - if (applicationList != 0) OS.g_list_free(applicationList); + if (applicationList != 0) OS.g_list_free_full(applicationList); return programs.toArray(new Program[programs.size()]); } Maybe this is the "double free" mentioned here, and the GTK+ code I'm using as an example is faulty? https://developer.gnome.org/glib/stable/glib-Doubly-Linked-Lists.html#g-list-free-full
(In reply to Simeon Andreev from comment #3) > This doesn't fix the leak: OK I see my code makes no sense. I've tried variations of iterating over the list though, to no avail. E.g.: if (applicationList != 0) { long list_iterator = list; while (list_iterator != 0) { OS.g_object_unref(list_iterator); list_iterator = OS.g_list_previous(list_iterator); } OS.g_list_free(applicationList); } (also with g_list_next()) The leak is still there.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/180539
The list consists of two things: the linked list items and the data contained inside. You need to 1) unref every data item, which is obtained by 'g_list_data()' 2) free the entire list, as before.
(In reply to Alexandr Miloslavskiy from comment #6) > The list consists of two things: the linked list items and the data > contained inside. > > You need to > 1) unref every data item, which is obtained by 'g_list_data()' > 2) free the entire list, as before. With this code, I see the same error as when calling g_list_free_full(list, g_object_unref) (see patch set 1 of https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/180539): if (applicationList != 0) { long list_iterator = applicationList; while (list_iterator != 0) { long application = OS.g_list_data(list_iterator); if (application != 0) { OS.g_object_unref(application); } list_iterator = OS.g_list_next(list_iterator); } OS.g_list_free(applicationList); } The elements of the list should be of type GAppInfo, at least according to the API the other SWT code calls with list elements. I don't understand the docu though, when it comes to the object hierarchy: https://developer.gnome.org/gio/stable/GAppInfo.html#GAppInfo.object-hierarchy I also don't find the definition of the type in the glib repository, though that is probably due to insufficient C knowledge. Though judging from the error, I assume GObject is not on the types hierarchy. I tried using g_free() instead of g_object_unref(), that results in a crash: *** Error in `/usr/lib/jvm/java-11/bin/java': free(): invalid pointer: 0x00007ffff061ee00 ***
Odd, if I comment out the call to org.eclipse.swt.program.Program.gio_getProgram(Display, long) in the method, the error is gone. The leak can then be fixed either by calling g_list_free_full() or with the code from my previous comment (see the suggestion from Alexander). I'll check what this method does, it must be changing the list in some way.
In particular whatever the problem is, its in this code: long icon = OS.g_app_info_get_icon(application); if (icon != 0) { long icon_name = OS.g_icon_to_string(icon); if (icon_name != 0) { length = C.strlen(icon_name); if (length > 0) { buffer = new byte[length]; C.memmove(buffer, icon_name, length); program.iconPath = new String(Converter.mbcsToWcs(buffer)); } OS.g_free(icon_name); } OS.g_object_unref(icon); } If I comment out "OS.g_object_unref(icon);", the error is gone.
The unref on the icon was added for bug 298612 (a while ago). Browsing through GTK3 code, I find the following matches: gdk/x11/gdkapplaunchcontext-x11.c: icon = g_app_info_get_icon (info); gtk/gtkappchooserbutton.c: icon = g_app_info_get_icon (app); gtk/gtkappchooserwidget.c: icon = g_app_info_get_icon (app); gtk/gtkappchooserwidget.c: icon = g_app_info_get_icon (app); Of these, the only unref I see is in: gdkapplaunchcontext-x11.c So maybe the unref is no longer necessary, at least on recent GLib versions (though I don't know if we can check the GLib version from SWT. If I remove the unref, I don't see errors or the memory leak, when using valgrind on the snippet from the description.
As far as I can tell the missing free has been either around "forever", or GLib changed at some point to need freeing. The code (with the leak) was added for bug 215377.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/180539 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=054088157c7b7805fd1e1f118827afe06228ed38
