[Gtk3] fix for SWTException on Table.remove(...) with focused row #1606

[om-11-2024]

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 https://github.com/eclipse-platform/eclipse.platform.swt/issues/1606

This bug happens in Gtk3.
I haven't tried to reproduce this bug in GTK4.

Java stacktrace:

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:

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);
          ...
        }
      ...
    }
  ...
}

[om-11-2024]

FYI:

• Table.removeAll() is not affected. Internally GTK.gtk_list_store_clear()calls gtk_list_store_remove() in a loop (this is GTK3.24.41 - current GTK3 version for Ubuntu 24.04LTS) .
  As a result, if some row has focus, then during gtk_list_store_clear() the focus moves to different rows and Table.cellDataProc() is invoked.
  Fortunately, Table.removeAll() sets items = new TableItem [4] before calling GTK.gtk_list_store_clear(), as a result there are no cached+disposed TableItems in Table.items when Table.cellDataProc() is executed => SWTError isn't thrown by TableItem.setText().
• Tree is not affected, because Tree.items are never out-of-sync with rows in GtkTreeView. (that's because Tree uses different mapping scheme for items (e.g. it uses ID_COLUMN))

[laeubi]

@om-11-2024 interesting observation, have you checked if a similar problem exits in the Tree widget?

[akurtakov]

Issue is fixed in alternative way https://github.com/eclipse-platform/eclipse.platform.swt/pull/2472

[om-11-2024]

@laeubi

interesting observation, have you checked if a similar problem exits in the Tree widget?

Tree doesn't suffer from this problem.
That's because Tree implements the association of swt Items and gtk rows differently (look at the code that gets TableItem/TreeItem in cellDataProc(...) methods in Table/Tree):

• in Table it's assumed that index of the gtk row == index of TableItem in Table.items
• in Tree each gtk row has unique id that is stored in ID_COLUMNcolumn, this unique id is the index of the associated TreeItem in Tree.items

What causes the bug:

• when the focused row is removed, gtk makes the next row focused and invokes cellDataProc(...) for that next row
• inside cellDataProc(...) the code gets the associated Item while the field items isn't updated yet and contain old disposed Items
  • for Tree everything works fine because index of TreeItem is taken from ID_COLUMN of the row
  • for Table we get a problem because Table.items contains an already removed and disposed TableItem at the index of the row

[om-11-2024]

@akurtakov

Issue is fixed in alternative way #2472

#1606 is fixed. But, as I understand, this still has problems.

Let's see what happens:

1. inside Table.remove() the gtk row is removed by GTK.gtk_list_store_remove(...)
2. inside gtk_list_store_remove(...): because we remove row with focus, gtk makes the next row focused and invokes Table.cellDataProc(...) on this next row
3. inside Table.cellDataProc(...) the code finds the associated TableItem as Table.items[idx_of_gtk_row]
   • was: since Table.items aren't updated yet, the TableItem at the index is the removed TableItem instead of the TableItem for the new row that gets the focus
   • #2472: same as previous, but now we check if TableItem is disposed
   • expected: the TableItem for the new row that get focus should be processed

In other words #2472 fixes #1606 because the disposed TableItem isn't processed in Table.cellDataProc(...).
But the correct TableItem (i.e. the row that gets the focus) is still not processed in Table.cellDataProc(...) - which may cause problems.