Skip to content

Commit b48723f

Browse files
om-11-2024om-11-2024
committed
[Gtk3] fix for SWTException on Table.remove(...) with focused row #1606
Fix for `SWTException` on `Table.remove(...)` with a focused row. 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 #1606 This bug happens in Gtk3. I haven't tried to reproduce this bug in GTK4. 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. 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); ... } ... } ... } ```
1 parent e6588c2 commit b48723f

File tree

2 files changed

+104
-14
lines changed

2 files changed

+104
-14
lines changed

‎bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java

+11-14
Original file line numberDiff line numberDiff line change
@@ -1124,11 +1124,11 @@ void destroyItem (TableItem item) {
11241124
}
11251125
if (index == itemCount) return;
11261126
long selection = GTK.gtk_tree_view_get_selection (handle);
1127+
System.arraycopy (items, index + 1, items, index, --itemCount - index);
1128+
items [itemCount] = null;
11271129
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
11281130
GTK.gtk_list_store_remove (modelHandle, item.handle);
11291131
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
1130-
System.arraycopy (items, index + 1, items, index, --itemCount - index);
1131-
items [itemCount] = null;
11321132
if (itemCount == 0) resetCustomDraw ();
11331133
}
11341134

@@ -2664,11 +2664,11 @@ public void remove (int index) {
26642664
}
26652665
if (!disposed) {
26662666
long selection = GTK.gtk_tree_view_get_selection (handle);
2667+
System.arraycopy (items, index + 1, items, index, --itemCount - index);
2668+
items [itemCount] = null;
26672669
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
26682670
GTK.gtk_list_store_remove (modelHandle, iter);
26692671
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
2670-
System.arraycopy (items, index + 1, items, index, --itemCount - index);
2671-
items [itemCount] = null;
26722672
}
26732673
OS.g_free (iter);
26742674
}
@@ -2703,20 +2703,17 @@ public void remove (int start, int end) {
27032703
long selection = GTK.gtk_tree_view_get_selection (handle);
27042704
long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ());
27052705
if (iter == 0) error (SWT.ERROR_NO_HANDLES);
2706-
int index = -1;
2707-
for (index = start; index <= end; index++) {
2708-
if (index == start) GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index);
2709-
TableItem item = items [index];
2706+
GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, start);
2707+
for (int index = start; index <= end; index++) {
2708+
TableItem item = items [start];
27102709
if (item != null && !item.isDisposed ()) item.release (false);
2710+
System.arraycopy (items, start + 1, items, start, --itemCount - start);
2711+
items [itemCount] = null;
27112712
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
27122713
GTK.gtk_list_store_remove (modelHandle, iter);
27132714
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
27142715
}
27152716
OS.g_free (iter);
2716-
index = end + 1;
2717-
System.arraycopy (items, index, items, start, itemCount - index);
2718-
for (int i=itemCount-(index-start); i<itemCount; i++) items [i] = null;
2719-
itemCount = itemCount - (index - start);
27202717
}
27212718

27222719
/**
@@ -2764,11 +2761,11 @@ public void remove (int [] indices) {
27642761
GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index);
27652762
}
27662763
if (!disposed) {
2764+
System.arraycopy (items, index + 1, items, index, --itemCount - index);
2765+
items [itemCount] = null;
27672766
OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
27682767
GTK.gtk_list_store_remove (modelHandle, iter);
27692768
OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED);
2770-
System.arraycopy (items, index + 1, items, index, --itemCount - index);
2771-
items [itemCount] = null;
27722769
}
27732770
last = index;
27742771
}

‎tests/org.eclipse.swt.tests.gtk/JUnit Tests/org/eclipse/swt/tests/gtk/Test_Gtk3_Table_Remove_Focused_Row.java

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package org.eclipse.swt.tests.gtk;
2+
3+
import org.eclipse.swt.SWT;
4+
import org.eclipse.swt.internal.gtk.GTK;
5+
import org.eclipse.swt.layout.FillLayout;
6+
import org.eclipse.swt.widgets.Shell;
7+
import org.eclipse.swt.widgets.Table;
8+
import org.eclipse.swt.widgets.TableItem;
9+
import org.junit.After;
10+
import org.junit.Assume;
11+
import org.junit.Before;
12+
import org.junit.BeforeClass;
13+
import org.junit.Test;
14+
15+
public class Test_Gtk3_Table_Remove_Focused_Row {
16+
17+
private Shell shell;
18+
private Table table;
19+
20+
@BeforeClass
21+
public static void setUpClass() {
22+
var isGtk3 = false;
23+
if ("gtk".equals(SWT.getPlatform())) {
24+
@SuppressWarnings("restriction")
25+
var gtkMajorVersion = GTK.GTK_VERSION >>> 16;
26+
isGtk3 = (gtkMajorVersion == 3);
27+
}
28+
Assume.assumeTrue("Test is only for Gtk3.", isGtk3);
29+
}
30+
31+
@Before
32+
public void setUp() {
33+
shell = new Shell();
34+
shell.setMinimumSize(400, 400);
35+
shell.setLayout(new FillLayout());
36+
}
37+
38+
@After
39+
public void tearDown() {
40+
if (table != null) {
41+
table.dispose();
42+
}
43+
if (shell != null) {
44+
shell.dispose();
45+
}
46+
}
47+
48+
@Test
49+
public void test_remove_focused_row_remove_one() {
50+
test_remove_focused_row_remove_method_arg(() -> table.remove(0));
51+
}
52+
53+
@Test
54+
public void test_remove_focused_row_remove_array() {
55+
test_remove_focused_row_remove_method_arg(() -> table.remove(new int[] { 0 }));
56+
}
57+
58+
@Test
59+
public void test_remove_focused_row_remove_interval() {
60+
test_remove_focused_row_remove_method_arg(() -> table.remove(0, 0));
61+
}
62+
63+
private void test_remove_focused_row_remove_method_arg(Runnable removeRow0) {
64+
table = new Table(shell, SWT.VIRTUAL);
65+
table.setItemCount(2);
66+
table.addListener(SWT.SetData, event -> {
67+
var item = (TableItem) event.item;
68+
item.setText("Item #" + System.identityHashCode(item) + " " + item.toString());
69+
});
70+
71+
shell.pack();
72+
shell.open();
73+
74+
processUiEvents();
75+
76+
// set focus on row[0]
77+
table.setFocus();
78+
79+
processUiEvents();
80+
81+
table.clear(0);
82+
removeRow0.run();
83+
84+
processUiEvents();
85+
}
86+
87+
private void processUiEvents() {
88+
while (table.getDisplay().readAndDispatch()) {
89+
// continue to next event
90+
}
91+
}
92+
93+
}

0 commit comments

Comments
 (0)