diff options
5 files changed, 147 insertions, 2 deletions
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 66a1047331..5c95ac5153 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 @@ -279,7 +279,9 @@ boolean checkData (TableItem item) { int signal_id = OS.g_signal_lookup (OS.row_changed, GTK.gtk_tree_model_get_type ()); OS.g_signal_handlers_block_matched (modelHandle, mask, signal_id, 0, 0, 0, handle); currentItem = item; + item.settingData = true; sendEvent (SWT.SetData, event); + item.settingData = false; //widget could be disposed at this point currentItem = null; if (isDisposed ()) return false; @@ -2673,6 +2675,7 @@ public void remove (int start, int end) { removeAll(); return; } + checkSetDataInProcessBeforeRemoval(start, end + 1); 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); @@ -2773,6 +2776,7 @@ public void remove (int [] indices) { */ public void removeAll () { checkWidget(); + checkSetDataInProcessBeforeRemoval(0, items.length); int index = itemCount - 1; while (index >= 0) { TableItem item = items [index]; @@ -4225,4 +4229,32 @@ Point resizeCalculationsGTK3 (long widget, int width, int height) { return sizes; } +/** + * Check the table item range [start, end) for items that are in process of + * sending {@code SWT#SetData} event. If such items exist, throw an exception. + * + * Does nothing if the given range contains no indices, + * or if we are below GTK 3.22.0 or are using GTK 4. + * + * @param start index of first item to check + * @param end index after the last item to check + */ +void checkSetDataInProcessBeforeRemoval(int start, int end) { + /* + * Bug 182598 - assertion failed in gtktreestore.c + * + * To prevent a crash in GTK, we ensure we are not setting data on the tree items we are about to remove. + * Removing an item while its data is being set will invalidate it, which will cause a crash. + * + * We therefore throw an exception to prevent the crash. + */ + for (int i = start; i < end; i++) { + TableItem item = items[i]; + if (item != null && item.settingData) { + String message = "Cannot remove a table item while its data is being set. " + + "At item " + i + " in range [" + start + ", " + end + ")."; + throw new SWTException(message); + } + } +} } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java index 1316029d95..ec101454c6 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java @@ -41,7 +41,7 @@ public class TableItem extends Item { Font font; Font[] cellFont; String [] strings; - boolean cached, grayed; + boolean cached, grayed, settingData; /** * Constructs a new instance of this class given its parent diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java index e1f11716cc..918daa2710 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java @@ -329,7 +329,9 @@ boolean checkData (TreeItem item) { int signal_id = OS.g_signal_lookup (OS.row_changed, GTK.gtk_tree_model_get_type ()); OS.g_signal_handlers_block_matched (modelHandle, mask, signal_id, 0, 0, 0, handle); currentItem = item; + item.settingData = true; sendEvent (SWT.SetData, event); + item.settingData = false; currentItem = null; //widget could be disposed at this point if (isDisposed ()) return false; @@ -2827,6 +2829,7 @@ void remove (long parentIter, int start, int end) { if (!(0 <= start && start <= end && end < itemCount)) { error (SWT.ERROR_INVALID_RANGE); } + checkSetDataInProcessBeforeRemoval(start, end + 1); 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); @@ -2863,6 +2866,7 @@ void remove (long parentIter, int start, int end) { */ public void removeAll () { checkWidget (); + checkSetDataInProcessBeforeRemoval(0, items.length); for (int i=0; i<items.length; i++) { TreeItem item = items [i]; if (item != null && !item.isDisposed ()) item.release (false); @@ -4190,4 +4194,32 @@ Point resizeCalculationsGTK3 (long widget, int width, int height) { return sizes; } +/** + * Check the tree item range [start, end) for items that are in process of + * sending {@code SWT#SetData} event. If such items exist, throw an exception. + * + * Does nothing if the given range contains no indices, + * or if we are below GTK 3.22.0 or are using GTK 4. + * + * @param start index of first item to check + * @param end index after the last item to check + */ +void checkSetDataInProcessBeforeRemoval(int start, int end) { + /* + * Bug 182598 - assertion failed in gtktreestore.c + * + * To prevent a crash in GTK, we ensure we are not setting data on the tree items we are about to remove. + * Removing an item while its data is being set will invalidate it, which will cause a crash. + * + * We therefore throw an exception to prevent the crash. + */ + for (int i = start; i < end; i++) { + TreeItem item = items[i]; + if (item != null && item.settingData) { + String message = "Cannot remove a tree item while its data is being set. " + + "At item " + i + " in range [" + start + ", " + end + ")."; + throw new SWTException(message); + } + } +} } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java index 7c9d627b64..f984669b5f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java @@ -42,7 +42,7 @@ public class TreeItem extends Item { Font font; Font[] cellFont; String [] strings; - boolean cached, grayed, isExpanded, updated; + boolean cached, grayed, isExpanded, updated, settingData; static final int EXPANDER_EXTRA_PADDING = 4; /** diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug182598_GTK_crash_set_item_count_in_SetData_callback.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug182598_GTK_crash_set_item_count_in_SetData_callback.java new file mode 100644 index 0000000000..aefbd4a928 --- /dev/null +++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug182598_GTK_crash_set_item_count_in_SetData_callback.java @@ -0,0 +1,81 @@ +/******************************************************************************* + * Copyright (c) 2018 Simeon Andreev and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Simeon Andreev - initial API and implementation + *******************************************************************************/ +package org.eclipse.swt.tests.gtk.snippets; +import org.eclipse.swt.SWT; +import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.events.SelectionListener; +import org.eclipse.swt.layout.FillLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Tree; +import org.eclipse.swt.widgets.TreeItem; + +public class Bug182598_GTK_crash_set_item_count_in_SetData_callback { + + // If we want to have a Java dump on crash, we need to set a different number of tree items on SWT.SetData callback. + // I.e. =false -> reproduce https://bugs.eclipse.org/bugs/show_bug.cgi?id=182598 + // =true -> reproduce https://bugs.eclipse.org/bugs/show_bug.cgi?id=545919 + private static final boolean CRASH_WITH_DUMP = false; + + public static void main(String[] args) { + + Display display = new Display(); + Shell shell = new Shell(display); + shell.setSize(300, 200); + shell.setLayout(new FillLayout()); + + final Tree tree = new Tree(shell, SWT.VIRTUAL); + tree.addListener(SWT.SetData, event -> { + int itemCount = CRASH_WITH_DUMP ? 0 : 1; + // JVM will crash now without the patch for bug 182598 + tree.setItemCount(itemCount); + }); + + // NB: Sometimes this N must be changed to crash reliably - in range from 5 to 20 + int N = 15; + for (int i = 0; i < N; ++i) { + TreeItem item = new TreeItem(tree, SWT.NONE); + int c = i + 1; + item.setText("item " + c); + if (i >= 5) { + item.setText("another item " + c); + } + item.setData("something " + c); + tree.setSelection(item); + } + + Button button = new Button(shell, SWT.NONE); + button.setText("Tree.clearAll()"); + button.addSelectionListener(new SelectionListener() { + @Override + public void widgetSelected(SelectionEvent e) { + tree.clearAll(true); + } + + @Override + public void widgetDefaultSelected(SelectionEvent e) { + } + }); + + shell.open(); + + while (!shell.isDisposed()) { + if (!display.readAndDispatch()) { + display.sleep(); + } + } + display.dispose(); + } +}
\ No newline at end of file |