Skip to content

Commit

Permalink
[Gtk3] fix for SWTException on Table.remove(...) with focused row ecl…
Browse files Browse the repository at this point in the history 
…ipse-platform#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 eclipse-platform#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);
          ...
        }
      ...
    }
  ...
}
```
  • Loading branch information
@om-11-2024
om-11-2024 committed Nov 21, 2024
1 parent e6588c2 commit 9d5b707
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 14 deletions.
25 changes: 11 additions & 14 deletions bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ();
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<itemCount; i++) items [i] = null;
itemCount = itemCount - (index - start);
}

/**
Expand Down Expand Up @@ -2764,11 +2761,11 @@ public void remove (int [] indices) {
GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index);
}
if (!disposed) {
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;
}
last = index;
}
Expand Down
93 changes: 93 additions & 0 deletions ...t.tests.gtk/JUnit Tests/org/eclipse/swt/tests/gtk/Test_Gtk3_Table_Remove_Focused_Row.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.eclipse.swt.tests.gtk;

import org.eclipse.swt.SWT;
import org.eclipse.swt.internal.gtk.GTK;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class Test_Gtk3_Table_Remove_Focused_Row {

private Shell shell;
private Table table;

@BeforeClass
public static void setUpClass() {
var isGtk3 = false;
if ("gtk".equals(SWT.getPlatform())) {
@SuppressWarnings("restriction")
var gtkMajorVersion = GTK.GTK_VERSION >>> 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
}
}

}

0 comments on commit 9d5b707

Please sign in to comment.