From b72319110e765c35b9a990d203963c667be011fc Mon Sep 17 00:00:00 2001 From: mo Date: Thu, 21 Nov 2024 18:21:18 +0500 Subject: [PATCH] [Gtk3] fix for SWTException on Table.remove(...) with focused row #1606 Fix for SWTException on Table.remove(...) with focused row. This bug happens only in Gtk3. Cause: `gtk_list_store_remove(...)` for a focused row invokes `Table.cellDataProc(...)` with not-yet-updated `Table.items`. Fix: remove `TableItem` from `Table.items` before `gtk_list_store_remove(...)`. Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/1606: The exception only happens in Gtk3. Java stacktrace: ```java org.eclipse.swt.SWTException: Widget is disposed at org.eclipse.swt.SWT.error(SWT.java:4922) at org.eclipse.swt.SWT.error(SWT.java:4837) at org.eclipse.swt.SWT.error(SWT.java:4808) at org.eclipse.swt.widgets.Widget.error(Widget.java:597) at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:512) at org.eclipse.swt.widgets.TableItem.setText(TableItem.java:1363) at org.eclipse.swt.tests.gtk.Test_Gtk3_Table_Remove_Focused_Row.lambda$3(Test_Gtk3_Table_Remove_Focused_Row.java:68) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5857) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1617) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1643) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1626) at org.eclipse.swt.widgets.Table.checkData(Table.java:289) at org.eclipse.swt.widgets.Table.cellDataProc(Table.java:227) at org.eclipse.swt.widgets.Display.cellDataProc(Display.java:995) at org.eclipse.swt.internal.gtk.GTK.gtk_list_store_remove(Native Method) at org.eclipse.swt.widgets.Table.remove(Table.java:2668) ... ``` The main cause of the error is that: 1. when a row with focus is deleted with `gtk_list_store_remove(...)`, then cell data function `Table.cellDataProc(...)` is called for a row after the removed one 2. but inside `Table.cellDataProc(...)` `Table.items` isn't yet updated , therefore `Table.cellDataProc(...)` operates with `TableItem`, which is already disposed but not yet removed from `Table.items` Inside `gtk_list_store_remove(...)` the row is removed from the `GtkTreeModel`, and only then `GtkTreeView` callbacks are invoked. It means that when `Table.cellDataProc(...)` is invoked, the row has already been removed in Gtk3. The fix: remove `TableItem` from `Table.items` before `gtk_list_store_remove(...)` instead of after. Important: I haven't tried to reproduce this bug in GTK4. Os: Ubuntu 24.04.1 LTS Gtk: 3.24.41 Swt: 4.967.8 ================================================= Why `gtk_list_store_remove(...)` invokes `Table.cellDataProc(...)` only for a row with focus and not for ordinary rows? The reason is that in gtk3 when a row with focus is deleted, the next row receives focus, which among other things creates `GtkCellAccessible` (a "cell with focus" for assistive technology in gtk3). Creation of `GtkCellAccessible` invokes cell data function `Table.cellDataProc(...)` - just like it happens when a standard cell in `GtkTreeView` gets rendered. Here is the stacktrace in gtk3 code: ```c apply_cell_attributes() at gtkcellarea.c:1257 g_hash_table_foreach() at ghash.c:2117 gtk_cell_area_real_apply_attributes() at gtkcellarea.c:1286 gtk_cell_area_box_apply_attributes() at gtkcellareabox.c:1310 _gtk_marshal_VOID__OBJECT_BOXED_BOOLEAN_BOOLEANv() at gtkmarshalers.c:5447 g_type_class_meta_marshalv() at gclosure.c:1062 _g_closure_invoke_va() at gclosure.c:897 signal_emit_valist_unlocked() at gsignal.c:3424 g_signal_emit_valist() at gsignal.c:3263 g_signal_emit() at gsignal.c:3583 gtk_cell_area_apply_attributes() at gtkcellarea.c:2373 gtk_tree_view_column_cell_set_cell_data() at gtktreeviewcolumn.c:2821 set_cell_data() at gtktreeviewaccessible.c:347 create_cell() at gtktreeviewaccessible.c:439 _gtk_tree_view_accessible_add_state() at gtktreeviewaccessible.c:2053 gtk_tree_view_real_set_cursor() at gtktreeview.c:13377 gtk_tree_view_row_deleted() at gtktreeview.c:9430 g_cclosure_marshal_VOID__BOXED() at gmarshal.c:1628 g_closure_invoke() at gclosure.c:834 signal_emit_unlocked_R() at gsignal.c:3888 signal_emit_valist_unlocked() at gsignal.c:3520 g_signal_emit_valist() at gsignal.c:3263 g_signal_emit() at gsignal.c:3583 gtk_tree_model_row_deleted() at gtktreemodel.c:1914 gtk_list_store_remove() at gtkliststore.c:1219 Java_org_eclipse_swt_internal_gtk_GTK_gtk_1list_1store_1remove() ``` The code that leads to execution of cell data function for a row with focus is in gtktreeview.c: ``` static void gtk_tree_view_row_deleted (GtkTreeModel *model, GtkTreePath *path, gpointer data) { ... /* If the cursor row got deleted, move the cursor to the next row */ if (tree_view->priv->cursor_node && (tree_view->priv->cursor_node == node || (node->children && (tree_view->priv->cursor_tree == node->children || _gtk_rbtree_contains (node->children, tree_view->priv->cursor_tree))))) { ... cursor_changed = TRUE; } ... if (cursor_changed) { if (cursor_node) { ... gtk_tree_view_real_set_cursor (tree_view, cursor_path, CLEAR_AND_SELECT | CURSOR_INVALID); ... } ... } ... } ``` --- .../gtk/org/eclipse/swt/widgets/Table.java | 25 +++-- .../Test_Gtk3_Table_Remove_Focused_Row.java | 93 +++++++++++++++++++ 2 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 tests/org.eclipse.swt.tests.gtk/JUnit Tests/org/eclipse/swt/tests/gtk/Test_Gtk3_Table_Remove_Focused_Row.java diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java index d2d99021f65..cd473c68c03 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java @@ -1124,11 +1124,11 @@ void destroyItem (TableItem item) { } if (index == itemCount) return; long selection = GTK.gtk_tree_view_get_selection (handle); + System.arraycopy (items, index + 1, items, index, --itemCount - index); + items [itemCount] = null; OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); GTK.gtk_list_store_remove (modelHandle, item.handle); OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - System.arraycopy (items, index + 1, items, index, --itemCount - index); - items [itemCount] = null; if (itemCount == 0) resetCustomDraw (); } @@ -2664,11 +2664,11 @@ public void remove (int index) { } if (!disposed) { long selection = GTK.gtk_tree_view_get_selection (handle); + System.arraycopy (items, index + 1, items, index, --itemCount - index); + items [itemCount] = null; OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); GTK.gtk_list_store_remove (modelHandle, iter); OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - System.arraycopy (items, index + 1, items, index, --itemCount - index); - items [itemCount] = null; } OS.g_free (iter); } @@ -2703,20 +2703,17 @@ public void remove (int start, int end) { long selection = GTK.gtk_tree_view_get_selection (handle); long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); if (iter == 0) error (SWT.ERROR_NO_HANDLES); - int index = -1; - for (index = start; index <= end; index++) { - if (index == start) GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index); - TableItem item = items [index]; + GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, start); + for (int index = start; index <= end; index++) { + TableItem item = items [start]; if (item != null && !item.isDisposed ()) item.release (false); + System.arraycopy (items, start + 1, items, start, --itemCount - start); + items [itemCount] = null; OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); GTK.gtk_list_store_remove (modelHandle, iter); OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); } OS.g_free (iter); - index = end + 1; - System.arraycopy (items, index, items, start, itemCount - index); - for (int i=itemCount-(index-start); i>> 16; + isGtk3 = (gtkMajorVersion == 3); + } + Assume.assumeTrue("Test is only for Gtk3.", isGtk3); + } + + @Before + public void setUp() { + shell = new Shell(); + shell.setMinimumSize(400, 400); + shell.setLayout(new FillLayout()); + } + + @After + public void tearDown() { + if (table != null) { + table.dispose(); + } + if (shell != null) { + shell.dispose(); + } + } + + @Test + public void test_remove_focused_row_remove_one() { + test_remove_focused_row_remove_method_arg(() -> table.remove(0)); + } + + @Test + public void test_remove_focused_row_remove_array() { + test_remove_focused_row_remove_method_arg(() -> table.remove(new int[] { 0 })); + } + + @Test + public void test_remove_focused_row_remove_interval() { + test_remove_focused_row_remove_method_arg(() -> table.remove(0, 0)); + } + + private void test_remove_focused_row_remove_method_arg(Runnable removeRow0) { + table = new Table(shell, SWT.VIRTUAL); + table.setItemCount(2); + table.addListener(SWT.SetData, event -> { + var item = (TableItem) event.item; + item.setText("Item #" + System.identityHashCode(item) + " " + item.toString()); + }); + + shell.pack(); + shell.open(); + + processUiEvents(); + + // set focus on row[0] + table.setFocus(); + + processUiEvents(); + + table.clear(0); + removeRow0.run(); + + processUiEvents(); + } + + private void processUiEvents() { + while (table.getDisplay().readAndDispatch()) { + // continue to next event + } + } + +}