Community
Participate
Working Groups
Build Identifier: Version: 3.7.1.v3738a issue is only on Linux platforms (Linux,Solaris, ..etc) not seen in windows platform Tree keep 5 reference for each handle in Display. out of which modelhandle reference is not released when Tree disposed. 1. modehandle add Tree reference to Display.widgetTable through Display.addWidget() 2. when TreeColumn is created in Tree. modelHandle is replaced. 3. Tree reference in Display.widgetTable is not released is causing disposed Tree widgets never garbage collected. 640 if (modelIndex == modelLength) { 641 int /*long*/ oldModel = modelHandle; 642 int /*long*/[] types = getColumnTypes (columnCount + 4); // grow by 4 rows at a time 643 int /*long*/ newModel = OS.gtk_tree_store_newv (types.length, types); 644 if (newModel == 0) error (SWT.ERROR_NO_HANDLES); 645 copyModel (oldModel, FIRST_COLUMN, newModel, FIRST_COLUMN, types, (int /*long*/)0, (int /*long*/)0, modelLength); 646 OS.gtk_tree_view_set_model (handle, newModel); 647 OS.g_object_unref (oldModel); 648 modelHandle = newModel; } Reproducible: Always Steps to Reproduce: 1. Create Tree with 2 or more TreeColumn 2. dispose Tree / recreate Tree 3. check heap dump. disposed Tree is referenced in Display.widgetTable
Created attachment 220143 [details] Patch to fix memory leak in Tree I have tested this fix by modifying Snippet 170, http://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet170.java to create and dispose a few thousand Tree instances. Heap dump analysis confirms that references to all the created instances of Tree are retained in Display's widgetTable array and hence the Tree objects are never garbage collected. This patch avoids the leaks by removing the references to old tree models from Display's widgetTable. I have tested the fix and also ran the Junit tests which were fine (FYI, there were 3 test failures which happened even without this patch). Silenio, can you please review the patch for master and 4.2.1/3.8.1 as well? Thanks!
Hi Arun, The patch is correct, but it is incomplete. 1) It has to be done for Table as well. 2) We need to add the new model to the widgetTable display.addWidget (modelHandle, this); 3) We need to hook the signals in the new model. Take a look at Tree.hookEvents() if (fixAccessibility ()) { OS.g_signal_connect_closure (modelHandle, OS.row_inserted, display.closures [ROW_INSERTED], true); OS.g_signal_connect_closure (modelHandle, OS.row_deleted, display.closures [ROW_DELETED], true); } Maybe add a setModel(int /*long*/ modelHandle) helper that takes care of removing the old model from the widget table and unref it, add the new model the the widget table and hook the signals.
Created attachment 220259 [details] Revised patch Thanks for the review comments Silenio! Please review this revised patch and let me know if I have missed something else. As usual, I have tested the changes and ran the corresponding unit tests.
Patch is good.
Pushed to master, R3_8_maintenance and R4_2_maintenance branches. http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=a3a1752d4cca52a10819bc086957df9ad04a0a87 http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=R3_8_maintenance&id=8c953642f1a586ef686bfee94a22118c15872478 http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=R4_2_maintenance&id=8947df53182404ba6912831b9784d9147ae94f72 Thanks for the review Silenio!
Fixed > 20120827
