Skip to content

Fix for #678 #1712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+212 −34
Closed

Fix for #678 #1712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 64 additions & 16 deletions bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package org.eclipse.swt.widgets;


import java.util.*;

import org.eclipse.swt.*;
import org.eclipse.swt.events.*;
import org.eclipse.swt.graphics.*;
Expand Down Expand Up @@ -98,6 +100,7 @@ public class Table extends Composite {
int headerHeight;
boolean boundsChangedSinceLastDraw, headerVisible, wasScrolled;
boolean rowActivated;
SetDataTask setDataTask = new SetDataTask();

private long headerCSSProvider;

Expand Down Expand Up @@ -220,26 +223,27 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long
}
}
if (modelIndex == -1) return 0;
boolean setData = false;
boolean updated = false;
if ((style & SWT.VIRTUAL) != 0) {
if (!item.cached) {
lastIndexOf = index[0];
setData = checkData (item);
setDataTask.enqueueItem (item);
}
if (item.updated) {
updated = true;
item.updated = false;
}
}
long [] ptr = new long [1];
if (setData) {
ptr [0] = 0;
if (isPixbuf) {
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1);
OS.g_object_set (cell, OS.gicon, ptr [0], 0);
if (ptr [0] != 0) OS.g_object_unref (ptr [0]);
} else {
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_TEXT, ptr, -1);
if (ptr [0] != 0) {
OS.g_object_set (cell, OS.text, ptr [0], 0);
OS.g_free (ptr [0]);
}
ptr [0] = 0;
if (isPixbuf) {
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1);
OS.g_object_set (cell, OS.gicon, ptr [0], 0);
if (ptr [0] != 0) OS.g_object_unref (ptr [0]);
} else {
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_TEXT, ptr, -1);
if (ptr [0] != 0) {
OS.g_object_set (cell, OS.text, ptr [0], 0);
OS.g_free (ptr [0]);
}
}
if (customDraw) {
Expand All @@ -266,7 +270,7 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long
}
}
}
if (setData) {
if (updated) {
ignoreCell = cell;
setScrollWidth (tree_column, item);
ignoreCell = 0;
Expand Down Expand Up @@ -4249,4 +4253,48 @@ public void dispose() {
headerCSSProvider = 0;
}
}

class SetDataTask implements Runnable {
boolean scheduled;
LinkedHashSet<TableItem> itemsQueue = new LinkedHashSet<> ();

void enqueueItem (TableItem item) {
itemsQueue.add (item);
ensureExecutionScheduled ();
}

void ensureExecutionScheduled () {
if (!scheduled && !isDisposed ()) {
display.asyncExec (this);
scheduled = true;
}
}

@Override
public void run () {
scheduled = false;
if (itemsQueue.isEmpty () || isDisposed ()) {
return;
}
LinkedHashSet<TableItem> items = itemsQueue;
itemsQueue = new LinkedHashSet<> ();
try {
for (Iterator<TableItem> it = items.iterator (); it.hasNext ();) {
TableItem item = it.next ();
it.remove ();
if (!item.cached && !item.isDisposed ()) {
if (checkData (item)) {
item.updated = true;
}
}
}
} catch (Throwable t) {
if (!items.isEmpty ()) {
itemsQueue.addAll (items);
ensureExecutionScheduled ();
}
throw t;
}
}
}
}
2 changes: 1 addition & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class TableItem extends Item {
Font font;
Font[] cellFont;
String [] strings;
boolean cached, grayed, settingData;
boolean cached, grayed, updated, settingData;

/**
* Constructs a new instance of this class given its parent
Expand Down
75 changes: 58 additions & 17 deletions bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public class Tree extends Composite {
Color headerBackground, headerForeground;
boolean boundsChangedSinceLastDraw, wasScrolled;
boolean rowActivated;
SetDataTask setDataTask = new SetDataTask();

private long headerCSSProvider;

Expand Down Expand Up @@ -296,32 +297,28 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long
}
}
if (modelIndex == -1) return 0;
boolean setData = false;
boolean updated = false;
if ((style & SWT.VIRTUAL) != 0) {
if (!item.cached) {
//lastIndexOf = index [0];
setData = checkData (item);
setDataTask.enqueueItem (item);
}
if (item.updated) {
updated = true;
item.updated = false;
}
}
long [] ptr = new long [1];
if (setData) {
if (isPixbuf) {
ptr [0] = 0;
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1);
OS.g_object_set (cell, OS.gicon, ptr [0], 0);
if (ptr [0] != 0) OS.g_object_unref (ptr [0]);
} else {
ptr [0] = 0;
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_TEXT, ptr, -1);
if (ptr [0] != 0) {
OS.g_object_set (cell, OS.text, ptr[0], 0);
OS.g_free (ptr[0]);
}
if (isPixbuf) {
ptr [0] = 0;
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1);
OS.g_object_set (cell, OS.gicon, ptr [0], 0);
if (ptr [0] != 0) OS.g_object_unref (ptr [0]);
} else {
ptr [0] = 0;
GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_TEXT, ptr, -1);
if (ptr [0] != 0) {
OS.g_object_set (cell, OS.text, ptr[0], 0);
OS.g_free (ptr[0]);
}
}
if (customDraw) {
Expand All @@ -348,7 +345,7 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long
}
}
}
if (setData || updated) {
if (updated) {
ignoreCell = cell;
setScrollWidth (tree_column, item);
ignoreCell = 0;
Expand Down Expand Up @@ -4333,4 +4330,48 @@ public void dispose() {
headerCSSProvider = 0;
}
}

class SetDataTask implements Runnable {
boolean scheduled;
LinkedHashSet<TreeItem> itemsQueue = new LinkedHashSet<> ();

void enqueueItem (TreeItem item) {
itemsQueue.add (item);
ensureExecutionScheduled ();
}

void ensureExecutionScheduled () {
if (!scheduled && !isDisposed ()) {
display.asyncExec (this);
scheduled = true;
}
}

@Override
public void run () {
scheduled = false;
if (itemsQueue.isEmpty () || isDisposed ()) {
return;
}
LinkedHashSet<TreeItem> items = itemsQueue;
itemsQueue = new LinkedHashSet<> ();
try {
for (Iterator<TreeItem> it = items.iterator (); it.hasNext ();) {
TreeItem item = it.next ();
it.remove ();
if (!item.cached && !item.isDisposed ()) {
if (checkData (item)) {
item.updated = true;
}
}
}
} catch (Throwable t) {
if (!items.isEmpty ()) {
itemsQueue.addAll (items);
ensureExecutionScheduled ();
}
throw t;
}
}
}
}
89 changes: 89 additions & 0 deletions ...k/ManualTests/org/eclipse/swt/tests/gtk/snippets/Issue678_JvmCrashOnTreeItemSetImage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package org.eclipse.swt.tests.gtk.snippets;

import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeItem;

/**
* Description: when {@link TreeItem#setImage(Image)} is called within an
* {@link SWT#SetData} event handler for a {@link SWT#VIRTUAL} tree, then a JVM
* crash can happen because of use-after-free gtk3 renderer in
* {@code Tree.cellDataProc()}.
* <p>
*
* <pre>
* Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
* C [libgobject-2.0.so.0+0x3b251] g_type_check_instance_is_fundamentally_a+0x11
* C [libswt-pi3-gtk-4958r2.so+0x4b609] Java_org_eclipse_swt_internal_gtk_OS_g_1object_1set__J_3BJJ+0x4a
*
* Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
* J 11988 org.eclipse.swt.internal.gtk.OS.g_object_set(J[BJJ)V (0 bytes)
* J 10921 c1 org.eclipse.swt.widgets.Tree.cellDataProc(JJJJJ)J (486 bytes)
* J 10920 c1 org.eclipse.swt.widgets.Display.cellDataProc(JJJJJ)J (29 bytes)
* v ~StubRoutines::call_stub
* J 11619 org.eclipse.swt.internal.gtk3.GTK3.gtk_main_iteration_do(Z)Z (0 bytes)
* J 11623 c1 org.eclipse.swt.widgets.Display.readAndDispatch()Z (88 bytes)
* </pre>
*
* Tested on GTK 3.24.37 (Fedora 38)
*/
public class Issue678_JvmCrashOnTreeItemSetImage {

private static final int NUM_ITERATIONS = 100;

public static void main (String[] args) {
Display display = new Display ();
Shell shell = new Shell (display);
shell.setSize (400, 300);
shell.setLayout (new FillLayout ());
shell.open ();
Image image = new Image (display, 20, 20);

for (int i = 0; i < NUM_ITERATIONS; i++) {
Tree tree = new Tree (shell, SWT.VIRTUAL);
tree.addListener (SWT.SetData, e -> {
TreeItem item = (TreeItem) e.item;
item.setText (0, "A");

// for some reason sleeping increases probability of crash
try {
Thread.sleep (50);
} catch (InterruptedException ex) {
throw new RuntimeException (ex);
}

item.setImage (image); // <-- this is the critical line!
});
tree.setItemCount (1);
shell.layout ();

waitUntilIdle ();

tree.dispose ();
}

display.dispose ();
}

private static void waitUntilIdle () {
long lastActive = System.currentTimeMillis ();
while (true) {
if (Thread.interrupted ()) {
throw new AssertionError ();
}
if (Display.getCurrent ().readAndDispatch ()) {
lastActive = System.currentTimeMillis ();
} else {
if (lastActive + 10 < System.currentTimeMillis ()) {
return;
}
Thread.yield ();
}
}
}

}
Loading