[Gtk3] fix for SWTException on Table.remove(...) with focused row #1606 #1608
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ipse-platform#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 eclipse-platform#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); ... } ... } ... } ```
|
FYI:
|
basilevs
reviewed
Feb 13, 2025
| var gtkMajorVersion = GTK.GTK_VERSION >>> 16; | ||
| isGtk3 = (gtkMajorVersion == 3); | ||
| } | ||
| Assume.assumeTrue("Test is only for Gtk3.", isGtk3); |
There was a problem hiding this comment.
Will this test fail on other platforms? If not, why should not it run there?
There was a problem hiding this comment.
The platform (as returned by org.eclipse.swt.SWT.getPlatform() ) is "gtk".
All the test in tests/gtk/... are executed only on "gtk" platform.
This test is in tests/gtk/... because it tests a bug in the swt implementation for gtk.
For now swt uses gtk3 + switching to gtk4 is in progress.
In gtk4 GtkTreeView is deprecated, which means that the code for Table for gtk4 will be rewritten.
That's why this test and this fix are only for gtk3.
There was a problem hiding this comment.
Additionally.
In GTK3 this bug involves GtkCellAccessible from the so called Accessibility toolkit (ATK) used by GTK3.
In GTK4 accessibility implemented differently and ATK isn't used anymore.
As a result, even if SWT would continue to use deprecated GtkTreeView in GTK4, this bug would not happen there.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for
SWTExceptiononTable.remove(...)with a focused row.Cause:
gtk_list_store_remove(...)for a focused row invokesTable.cellDataProc(...)with not-yet-updatedTable.items.Fix: remove
TableItemfromTable.itemsbeforegtk_list_store_remove(...).Fixes #1606
This bug happens in Gtk3.
I haven't tried to reproduce this bug in GTK4.
Java stacktrace:
The main cause of the error is that:
gtk_list_store_remove(...), then cell data functionTable.cellDataProc(...)is called for a row after the removed oneTable.cellDataProc(...)Table.itemsisn't yet updated , thereforeTable.cellDataProc(...)operates withTableItem, which is already disposed but not yet removed fromTable.itemsInside
gtk_list_store_remove(...)the row is removed from theGtkTreeModel, and only thenGtkTreeViewcallbacks are invoked. It means that whenTable.cellDataProc(...)is invoked, the row has already been removed in Gtk3.The fix: remove
TableItemfromTable.itemsbeforegtk_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(...)invokesTable.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
GtkCellAccessibleinvokes cell data functionTable.cellDataProc(...)- just like it happens when a standard cell inGtkTreeViewgets rendered.Here is the stacktrace in gtk3 code:
The code that leads to execution of cell data function for a row with focus is in gtktreeview.c: