Sorry about the waiting time, SWT currently has few active maintainers.
I'm generally interested in JVM crashes, and plan to have a look at this eventually. Unfortunately the waiting time could be around a month.

Hello, just a ping on this issue: We are still often encountering this crash on an almost daily basis... :/

SWT is currently low on active contributors.
It would be nice if you can debug the problem yourself and prepare a fix.
Otherwise, alas, you'll have to wait more, and there's no ETA.

Otherwise, alas, you'll have to wait more, and there's no ETA.

Of course, I understand, FOSS is what it is...

It would be nice if you can debug the problem yourself and prepare a fix.

Actually we ended up tossing the problematic pieces of code (in this one and #697) into a Display.asyncExec callback, so they can be done at a time when Gtk is hopefully no longer confused. It's dumb, but appears to be working so far.

I found a way to reproduce all the time in a @flatpak runtime, where also all workarounds fail.

It would be useful if you describe how exactly do you reproduce.

Sorry, I thought that it won't be that helpful because you need to build a package to test.

Here it is https://github.com/Mailaender/flathub/compare/chemclipse Check the README for the commands to build it.

Click Demo on the green intro screen to trigger the crash.

Use

runtime-version: 20.08

and rebuild to see how the application looked before the crash was introduced.

Oh, I didn't expect a whole other application. Indeed I don't have the time to debug it.
Can you please post your hs_err_pid.log crash reports?

https://gist.github.com/Mailaender/24f56492ded2586edfecf2e4df6d5232

You can also indirectly trigger this with

public class MyLabelProvider extends LabelProvider implements ILabelProvider {
	
	@Override
	public Image getImage(Object element) {
		// return something other then null
	}
}

This is sometimes difficult to reproduce. I had my best chances when there was only one unexpanded item in the tree viewer.

I can't reproduce this in a development environment so this might be already accidentally fixed?

We still have the Display.asyncExec workaround in place, so I can't comment on it, as we haven't encountered it since.
It's possible it got fixed, but I can't verify it due to lack of time, sorry.

There are occasions where the Display.asyncExec either does not work or only lessens the problem.

I just copied your snippet into my 2023-03 project and it crashed. It does not when I try it here.

The instructions at https://www.eclipse.org/swt/git.php are somehow outdated or not very precise, but this should work:

Install git-lfs. Git clone this repository. Only import the repositories (plus snippets) and rename or copy

• bundles/org.eclipse.swt/.classpath_gtk → .classpath
• examples/org.eclipse.swt.snippets/.classpath_gtk → .classpath

I just ignored the API baseline error and got a test environment where the problem is not reproducible or hopefully already fixed.

Updated consecutively up to Eclipse 2023-12 and the crash is still there.

While trying to work around it, I found another way to crash more reliable:

public class MyLabelProvider extends LabelProvider implements ILabelProvider {

	@Override
	public Image getImage(Object element) {
		try {
			Thread.sleep(100);
		} catch(InterruptedException e) {
			logger.warn(e);
		}
		new Image(display, 16, 16))
	}
}

I have no real fix yet, and the TreeViewer API does not allow for a simple override of the setImage function. However, I found out that only the first setImage will crash, all subsequent won't. So setImage(null) will make all following calls safe.

I also don't understand why this crash does not happen with the Eclipse IDE. The PDE Project Explorer and Java Package Explorer as well as the Git Repositories all use trees with icons.

@ericwill why does TreeItem.setImage() recreate renderer for a single column? Should not all columns be affected?

@SyntevoAlex could you please suggest a way to detect GTK warnings from Junit tests?

GTK has been desperately trying to warn us about this problem, but we always ignore its printout to standard error:

My Application Name:4909): GLib-CRITICAL **: 14:43:25.864: g_hash_table_foreach: assertion 'version == hash_table->version' failed

(My Application Name:4909): GLib-GObject-WARNING **: 14:43:25.897: g_object_notify_queue_thaw: property-changed notification for GtkCellRendererText(0x7f06dded6990) is not frozen

@ericwill why does TreeItem.setImage() recreate renderer for a single column? Should not all columns be affected?

You're asking me about code I haven't touched in ~4 years, so I might be wrong. If memory serves correctly the method in question modifies only one column at a time as per SWT API. Also the method in question involves SWT.VIRTUAL trees which are performance sensitive (lazy loaded) so we are not looking to make broad changes as it could incur a performance hit.

You're asking me about code I haven't touched in ~4 years, so I might be wrong. If memory serves correctly the method in question modifies only one column at a time as per SWT API. Also the method in question involves SWT.VIRTUAL trees which are performance sensitive (lazy loaded) so we are not looking to make broad changes as it could incur a performance hit.

It was trying to solve cropping of images by using actual size in gtk_cell_renderer_set_fixed_size(renderer, x, y) call, however, it uses Tree-global values, so was worried about other columns, where this call does not happen and leaves renderer configuration unsynchronized with global setting, potentially leaving invalid cropping for all but one column.

This is fine, neither of us probably have time to write a test anyway 🤷

The enhaced reproducer with a fixed SWT version can now be found in https://github.com/basilevs/bug_678

Thread.sleep() was mentioned above

It tried adding Thread.sleep() to the first code snippet for reproducing the bug, and now the bug to appears at every run of the snippet on my machine.

package swt_tree_crash;

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.Event;
import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeItem;

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

        Tree tree = new Tree(shell, SWT.VIRTUAL);
        tree.addListener(SWT.SetData, new Listener() {
            public void handleEvent(final Event e) {
            	TreeItem item = (TreeItem)e.item;
                item.setText(0, "A");
                try {
                  Thread.sleep(1000);
                }
                catch ( InterruptedException ex ) {
                  throw new RuntimeException(ex);
                }
                item.setImage(new Image(display, 20, 20)); // <-- this is the critical line!
            }
        });
        tree.setItemCount(1);
        
        shell.open();

        while (!shell.isDisposed()) {
            if (!display.readAndDispatch()) {
                display.sleep();
            }
        }
        display.dispose();
    }
}

@om-11-2024 does wrapping setImage() in Display.asyncExec() fix your snippet?

Yes, this fixes the bug in the snippet.

Here is what causes the error in the swt code.

The problem is inside org.eclipse.swt.widgets.Tree.cellDataProc().

Here is the stacktrace to Tree.cellDataProc() in this snippet:

Tree.cellDataProc(long, long, long, long, long)
Display.cellDataProc(long, long, long, long, long) line: 995	
OS.swt_fixed_resize(long, long, int, int) line: not available [native method]	
Tree(Scrollable).resizeHandle(int, int) line: 554	
Tree(Composite).resizeHandle(int, int) line: 1600	
Tree(Control).setBounds(int, int, int, int, boolean, boolean) line: 1122	
Tree(Composite).setBounds(int, int, int, int, boolean, boolean) line: 1658	
Tree.setBounds(int, int, int, int, boolean, boolean) line: 3641	
Tree(Control).setBounds(int, int, int, int) line: 1019	
FillLayout.layout(Composite, boolean) line: 216	
Shell(Composite).updateLayout(boolean) line: 1877	
Shell.setVisible(boolean) line: 3011	
Shell.open() line: 2032	
Main.main(String[]) line: 36

Here is the problem in Tree.cellDataProc() code:

@Override
long cellDataProc (long tree_column, long cell, long tree_model, long iter, long data) {
	...
	boolean setData = false;
	...
	if ((style & SWT.VIRTUAL) != 0) {
		...
		setData = checkData (item); // <= HERE new renderers are created, and the current renderer is disposed + it's memory is freed
		...
	}
	...
	long [] ptr = new long [1];
	if (setData) {
			...
			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); // <= HERE is an attempt to set property for the already disposed current renderer
				OS.g_free (ptr[0]);
			}
		}
	}

The problem is that:

1. at first the current renderer (it's address passed to the method in the cell argument) is disposed inside checkData (item)
2. later there is an attempt to set a property to the already disposed current renderer

Here is how the current renderer is released inside Tree.checkData():

1. SWT.SetData is triggered:

   boolean checkData (TreeItem item) {
   	...
   	if ((style & SWT.VIRTUAL) != 0) {
   		...
   		sendEvent (SWT.SetData, event);
   		...
   	}
   	return true;

2. our handler in the snippet invokes TreeItem.setImage()
3. inside TreeItem.setImage() there is an invocation of Tree.createRenderers()

   public void setImage(int index, Image image) {
   	...
   	if (!parent.pixbufSizeSet) {
   		if (image != null) {
   			...
   				if ((parent.style & SWT.VIRTUAL) != 0) {
   					...
   					parent.createRenderers(column, modelIndex, check, parent.style);
   				}
   	...

4. Tree.createRenderers() calls GTK.gtk_tree_view_column_clear (columnHandle), which calls g_object_unref() on the renderer, which results in the renderer disposal + frees it's memory.

Because cell is used as the 1st argument in OS.g_object_set (cell, OS.text, ptr[0], 0).

Initially at the beginning of Tree.cellDataProc() the cell contains the pointer to the renderer.

Later in Tree.cellDataProc() there is a call checkData (item), which calls GTK.gtk_tree_view_column_clear (columnHandle) which disposes all renderers for the column (including the renderer which cell points to).

By the time we get to OS.g_object_set (cell, OS.text, ptr[0], 0); the renderer that cell points to is already disposed.

@om-11-2024 I assumed renderers are reference-counted, so cell object should hold its own reference to renderer, preventing the disposal. Could you send a link to GTK code managing cell object?

Sure, here are some links:

• https://github.com/GNOME/gtk/blob/51e636be0f1f154a3a007892401fc51104f88ca1/gtk/gtktreeviewcolumn.c#L1690
• https://github.com/GNOME/gtk/blob/51e636be0f1f154a3a007892401fc51104f88ca1/gtk/gtkcellarea.c#L1493

And here is native stacktrace for GTK.gtk_tree_view_column_clear() which leads to g_object_unref() call for the renderer.

g_object_unref() at gobject.c:4 381 0x731907fb46d0	
cell_info_free() at gtkcellareabox.c:412 0x731905725e8c	
gtk_cell_area_box_remove() at gtkcellareabox.c:1 121 0x7319057277bb	
gtk_cell_area_remove() at gtkcellarea.c:1 671 0x73190571e8d4	
gtk_cell_area_clear() at gtkcellarea.c:1 502 0x73190571e263	
gtk_cell_layout_clear() at gtkcelllayout.c:415 0x73190572e1eb	
gtk_cell_layout_default_clear() at gtkcelllayout.c:236 0x73190572d9d8	
gtk_cell_layout_clear() at gtkcelllayout.c:415 0x73190572e1eb	
gtk_tree_view_column_clear() at gtktreeviewcolumn.c:1 693 0x731905ac6f2e	
Java_org_eclipse_swt_internal_gtk_GTK_gtk_1tree_1view_1column_1clear() at 0x731906047569	

This stacktrace is also for this snippet

Inside this g_object_unref() the number of references for the renderer reached 0 and it was disposed.

So yes, the renderer is reference-counted, its owner is column.
When GTK.gtk_tree_view_column_clear() is called, then the column executes g_object_unref() for its renderers (because it no longer needs them), their ref-counters reache 0 and the are disposed.

@om-11-2024 do you think Tree.cellDataProc is called from apply_cell_attributes?
Then reserving a reference inside our code would not help, as g_object_thaw_notify still accesses the renderer later.
So #1612 may still be a way to go, unless we want to rework the ownership of renderers.

@om-11-2024 do you think Tree.cellDataProc is called from apply_cell_attributes?

It is. Here is the stacktrace with both java and c frames:

// java
  Tree.cellDataProc(long, long, long, long, long) line: 277	
  Display.cellDataProc(long, long, long, long, long) line: 995	

// native
  apply_cell_attributes() at gtkcellarea.c:1 257 0x7679b4b1da3b	
  g_hash_table_foreach() at ghash.c:2 117 0x7679b5ed0e8b	
  gtk_cell_area_real_apply_attributes() at gtkcellarea.c:1 286 0x7679b4b1db55	
  gtk_cell_area_box_apply_attributes() at gtkcellareabox.c:1 310 0x7679b4b27dc5	
  _gtk_marshal_VOID__OBJECT_BOXED_BOOLEAN_BOOLEANv() at gtkmarshalers.c:5 447 0x7679b4aa82e2	
  g_type_class_meta_marshalv() at gclosure.c:1 062 0x7679e0042f7e	
  _g_closure_invoke_va() at gclosure.c:897 0x7679e0042a82	
  signal_emit_valist_unlocked() at gsignal.c:3 424 0x7679e006581f	
  g_signal_emit_valist() at gsignal.c:3 263 0x7679e006516e	
  g_signal_emit() at gsignal.c:3 583 0x7679e0066b43	
  gtk_cell_area_apply_attributes() at gtkcellarea.c:2 373 0x7679b4b20b52	
  gtk_tree_view_column_cell_set_cell_data() at gtktreeviewcolumn.c:2 821 0x7679b4ec9cfc	
  validate_row() at gtktreeview.c:6 432 0x7679b4eaa845	
  initialize_fixed_height_mode() at gtktreeview.c:6 884 0x7679b4eabf24	
  do_validate_rows() at gtktreeview.c:6 931 0x7679b4eac0ac	
  gtk_tree_view_get_preferred_width() at gtktreeview.c:2 683 0x7679b4e9fc39	
  gtk_widget_query_size_for_orientation() at gtksizerequest.c:181 0x7679b4dccdac	
  gtk_widget_compute_size_for_orientation() at gtksizerequest.c:399 0x7679b4dcd6e6	
  gtk_widget_get_preferred_width() at gtksizerequest.c:492 0x7679b4dcd978	
  gtk_scrolled_window_measure() at gtkscrolledwindow.c:1 849 0x7679b4da5819	
  gtk_css_custom_gadget_get_preferred_size() at gtkcsscustomgadget.c:124 0x7679b4b7d79c	
  gtk_css_gadget_get_preferred_size() at gtkcssgadget.c:683 0x7679b4b84da9	
  gtk_scrolled_window_get_preferred_width() at gtkscrolledwindow.c:4 193 0x7679b4dac6da	
  gtk_widget_query_size_for_orientation() at gtksizerequest.c:181 0x7679b4dccdac	
  gtk_widget_compute_size_for_orientation() at gtksizerequest.c:399 0x7679b4dcd6e6	
  gtk_widget_get_preferred_width_for_height() at gtksizerequest.c:565 0x7679b4dcdbb6	
  _gtk_widget_get_preferred_size_and_baseline() at gtksizerequest.c:708 0x7679b4dcdfe0	
  gtk_widget_get_preferred_size() at gtksizerequest.c:750 0x7679b4dce059	
  0x7679b5452269	// this is `swt_fixed_size_allocate()` from swt
  gtk_widget_size_allocate_with_baseline() at gtkwidget.c:6 179 0x7679b4ed8cfb	
  gtk_widget_size_allocate() at gtkwidget.c:6 260 0x7679b4ed9081	
  swt_fixed_resize() at 0x7679b54527d2	
  Java_org_eclipse_swt_internal_gtk_OS_swt_1fixed_1resize() at 0x7679b544e76c	

// java
OS.swt_fixed_resize(long, long, int, int) line: not available [native method]	
  Tree(Scrollable).resizeHandle(int, int) line: 554	
  Tree(Composite).resizeHandle(int, int) line: 1600	
  Tree(Control).setBounds(int, int, int, int, boolean, boolean) line: 1122	
  Tree(Composite).setBounds(int, int, int, int, boolean, boolean) line: 1658	
  Tree.setBounds(int, int, int, int, boolean, boolean) line: 3641	
  Tree(Control).setBounds(int, int, int, int) line: 1019	
  FillLayout.layout(Composite, boolean) line: 216	
  Shell(Composite).updateLayout(boolean) line: 1877	
  Shell.setVisible(boolean) line: 3011	
  Shell.open() line: 2032	
  Main.main(String[]) line: 36	

Then reserving a reference inside our code would not help, as g_object_thaw_notify still accesses the renderer later.

Agree.

So #1612 may still be a way to go, unless we want to rework the ownership of renderers.

Maybe.
For now assessing #1612 it's hard to me.
The main problem is that this bug is caused by bugfix for 513761480261.
That bugfix added Tree.createRenderers() inside TreeItem.setImage(), which was a mistake because:

• TreeItem.setImage() and TreeItem.setText() are used in SWT.SetData event handlers by design
• SWT.SetData is used in Tree.cellDataProc() (it's a cell data function - i.e. a function which gets invoked on each cell right before its rendering and which sets what to render: text, image, color, etc.) with SWT.VIRTUAL also by design.

I'm not a fan of the fact that #1612 adds asynchronicity to TreeItem.setImage() - imo this makes the code even more complicated. On the other hand, #1612 works and is a better bugfix for 513761 than what we have now.

Indexing options for further discussion:

1. async recreation of renderers
2. move renderer ownership around (TreeColumn?)
3. reconfigure renderer instead of recreating it

The main problem is that this bug is caused by bugfix for 513761.

Could you recheck the bug number? I'm sure the bug is introduced in fix for 480261 instead. Here is the commit.

Yes, it's 480261, not 513761.

I looked into the code of swt and gtk here is what I found out:

• in swt a tree with SWT.VIRTUAL maps to a GtkTreeView with fixed-height-mode=true in gtk
• before the 1st image for any TreeItem is set there is some fixed row height already.
• when the 1st image is set for any TreeItem, then
  -pixbufSizeSet,pixbufHeight andpixbufWidth fields are set for Tree
  • if the height of the image is greater than the current fixed row height, then then the current fixed row height should be increased and the tree redrawn.
• from now on any image set for any item of the tree should have the same size as the 1st image

The fixed row row height for a GtkTreeView is stored in _GtkTreeViewPrivate.fixed_height and is computed in initialize_fixed_height_mode.
The actual row height is stored in _GtkTreeViewPrivate.tree: every row corresponds to a GtkRBNode, and the height is in _GtkRBNode.offset.

So basically what we need to do is: when the 1st image is inserted to the tree, we need to:

• make GtkTreeView recompute its fixed_height field
• make existing rows update their _GtkRBNode.offset fields.

I found 2 ways to do that in gtk code:

1. gtk_tree_view_set_model: i.e. one can create a copy of the current GtkTreeModel and set it to the tree, then everything will be recomputed
2. re-set fixed-height-mode property of the GtkTreeView (this recomputes _GtkTreeViewPrivate.fixed_height) + execute gtk_tree_view_row_changed on each existing row (this recomputes _GtkRBNode.offset for existing rows)

I chose option 2 in #1662 simply because it doesn't resets current selection, focus and scroll position for the tree.

P.S. It turns out that using Display.asyncExec() is fine in this case (actually I had to use it).
Actually GTK itself actively uses asynchronous tasks, e.g. all the drawing and validation happens asynchronously.
So heavy updates of the tree (like in this case) should be executed asynchronously and not inside Tree.cellDataProc().