Community
Participate
Working Groups
Created attachment 286339 [details] Video showing reproduction of the problem. The code in question is found in Combo.eventWindow() and Text.eventWindow(): @Override long eventWindow () { ... long window = gtk_widget_get_window (entryHandle); // Find the internal GDK_INPUT_ONLY window long children = GDK.gdk_window_get_children (window); if (children != 0) { do { window = OS.g_list_data (children); } while ((children = OS.g_list_next (children)) != 0); } OS.g_list_free (children); return window; } "children" can be (and is) re-assigned in the while loop, and so the original handle is never freed. To reproduce: 1. Set a conditional breakpoint at org.eclipse.swt.widgets.Combo.eventWindow(), on line "OS.g_list_free (children);", print "children" in the condition and return false. Alternatively add a println() if you have the source code (a normal breakpoint might result in a window manager freeze when trying to change the combo value). 2. Debug snippet: public class TestComboMemoryLeak { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setSize(600, 400); shell.setLayout(new FillLayout()); Composite composite = new Composite(shell, SWT.NONE); composite.setLayout(new FillLayout(SWT.VERTICAL)); Combo c = new Combo(composite, SWT.NONE); c.add("1"); c.add("2"); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } } 3. Change the combo value. 4. Observe "children" is 0 when the free call is reached. This leaks native memory; the free call must be done (see e.g. documentation of the method that "gdk_window_get_children()" delegates to: https://developer.gnome.org/glib/stable/glib-Doubly-Linked-Lists.html#g-list-copy-deep ). Respectively, valgrind reports e.g.: ==84048== 432 (216 direct, 216 indirect) bytes in 9 blocks are definitely lost in loss record 12,728 of 13,476 ==84048== at 0x4C2B017: malloc (vg_replace_malloc.c:380) ==84048== by 0x5391A68D: g_malloc (in /usr/lib64/libglib-2.0.so.0.5600.1) ==84048== by 0x53931C8D: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.5600.1) ==84048== by 0x53911293: g_list_copy_deep (in /usr/lib64/libglib-2.0.so.0.5600.1) ==84048== by 0x4FFFDAD2: Java_org_eclipse_swt_internal_gtk_GDK_gdk_1window_1get_1children (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-pi3-gtk-4944r15.so) ==84048== by 0x1140672F: ??? ==84048== by 0x11400A4F: ??? ==84048== by 0x11400A4F: ??? ==84048== by 0x11400CBF: ??? ==84048== by 0x11BCA74B: ??? ==84048== 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) ==84048== by 0x676A185: jni_invoke_nonstatic(JNIEnv_*, JavaValue*, _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, Thread*) (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==84048== by 0x6770FBC: jni_CallLongMethodV (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==84048== by 0x4FD6A5AB: callback (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-gtk-4944r15.so) ==84048== by 0x4FD6F862: fn9_3 (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-gtk-4944r15.so) ==84048== by 0x50461D24: ??? (in /usr/lib64/libgtk-3.so.0.2200.30) ==84048== by 0x53688987: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==84048== by 0x5369B05C: ??? (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==84048== by 0x536A2CDB: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==84048== by 0x536A32DE: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.5600.1) ==84048== by 0x505AC56B: ??? (in /usr/lib64/libgtk-3.so.0.2200.30) ==84048== by 0x5045EF9B: ??? (in /usr/lib64/libgtk-3.so.0.2200.30) ==84048== by 0x50460E75: gtk_main_do_event (in /usr/lib64/libgtk-3.so.0.2200.30) ==84048== by 0x50018C00: Java_org_eclipse_swt_internal_gtk3_GTK3_gtk_1main_1do_1event (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-pi3-gtk-4944r15.so) ==84048== by 0x18FA9983: ??? ==84048== by 0x11BBF5BB: ??? ==84048== 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) ==84048== by 0x676A185: jni_invoke_nonstatic(JNIEnv_*, JavaValue*, _jobject*, JNICallType, _jmethodID*, JNI_ArgumentPusher*, Thread*) (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==84048== by 0x6770FBC: jni_CallLongMethodV (in /usr/lib/jvm/java-11-openjdk-11.0.11.0.9-1.el7_9.x86_64/lib/server/libjvm.so) ==84048== by 0x4FD6A5AB: callback (in /home/sandreev/git/eclipse/eclipse.platform.swt.binaries/bundles/org.eclipse.swt.gtk.linux.x86_64/libswt-gtk-4944r15.so)
Nice job finding that!
Note: I think there's similar code in many other places in SWT.
(In reply to Alexandr Miloslavskiy from comment #1) > Nice job finding that! Thank to you for the tooling. Alexandr, would *you* be able to provide a patch for 4.20, or should *we* try to fix that? We are currently trying to find a leak in 4.15 platform we know exists somewhere, this bug is just a side effect of our tests :-)
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/180362
I see that you decided to make patch yourself. I see that you linked Bug 570331 and Bug 561444, probably meaning that this is where the bug come from, but that's not actually correct. The code was merely rearranged in these patches. Before the patch, eventWindow() called paintWindow(). In fact, those patches removed the bug from paintWindow().
OK, I see. I didn't check the full commits, only the lines for the eventWindow() method.
I glanced at all other uses of 'g_list_free()' and found just one other bug in 'Shell.showWidget()'. Simeon, care to defeat that one as well?
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/180362 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=c469e04cc1407f93b2827da2f3dbc0770c85dbc7
Bug 564795 added the leak for Shell (4.19). Bug 540841 added the leak for Text (4.12). Bug 509514 added the leak for Combo (4.7). So the leak is since 4.7 :-).
Haha, it's dangerous to change code in SWT! In my understanding, the leak in Shell is in fact since Bug 393681. Didn't check the others. Shameless advertising: our DeepGit is quite good at tracking where the code came from.
