`SWT.SetData` event sent to disposed item during gtk_list_store_remove

[iloveeclipse]

Found that in my log, I was most likely switching between repositories or branches in the history view, and egit tried to recreate shown commits table.

The exception happens during GTK callback on the table. For whatever reason the item for which we received callback event from GTK was already disposed in SWT. This reminds me on https://bugs.eclipse.org/bugs/show_bug.cgi?id=182598, but that was other way around.

I can't reproduce it in debugger, all calls to org.eclipse.swt.widgets.Table.remove(int, int) result in events sent to non disposed items, so I assume there must be some timing/async issue with GTK.

With that, GTK callback shouldn't probably send SWT.SetData event to table listeners for disposed table items. Interestingly, check for if (isDisposed ()) return false; is few lines after sending event?

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4918)
	at org.eclipse.swt.SWT.error(SWT.java:4833)
	at org.eclipse.swt.SWT.error(SWT.java:4804)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:565)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:480)
	at org.eclipse.swt.widgets.Widget.getData(Widget.java:591)
	at org.eclipse.jface.viewers.StructuredViewer.associate(StructuredViewer.java:570)
	at org.eclipse.jface.viewers.AbstractTableViewer$VirtualManager.lambda$0(AbstractTableViewer.java:103)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5794)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1529)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1555)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1538)
	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:949)
	at org.eclipse.swt.internal.gtk.GTK.gtk_list_store_remove(Native Method)
	at org.eclipse.swt.widgets.Table.remove(Table.java:2716)
	at org.eclipse.swt.widgets.Table.setItemCount(Table.java:3652)
	at org.eclipse.jface.viewers.TableViewer.doSetItemCount(TableViewer.java:231)
	at org.eclipse.jface.viewers.AbstractTableViewer.internalVirtualRefreshAll(AbstractTableViewer.java:646)
	at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:620)
	at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:610)
	at org.eclipse.jface.viewers.AbstractTableViewer.lambda$0(AbstractTableViewer.java:572)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1398)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1359)
	at org.eclipse.jface.viewers.AbstractTableViewer.inputChanged(AbstractTableViewer.java:572)
	at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:282)
	at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1632)
	at org.eclipse.egit.ui.internal.history.CommitGraphTable.setInput(CommitGraphTable.java:541)
	at org.eclipse.egit.ui.internal.history.GitHistoryPage$8.run(GitHistoryPage.java:2432)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)

[SyntevoAlex]

To me, this looks very similar to Bug 573932 There, SWT also got confused via 'gtk_list_store_remove()' and 'cellDataProc()', which resulted in accessing deleted items.

I suspect that the proposed change only attempts to fight consequences of a problem elsewhere. Applying it would probably move the problem to a new place, one extra step further from where it happens.

[iloveeclipse]

Are you thinking about something like this before this loop in Table.remove(): https://github.com/eclipse-platform/eclipse.platform.swt/blob/0f47f6b701c87581621bca610d6358587900a9a3/bundles/org.eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/widgets/Table.java#L2712

[SyntevoAlex]

Ah sorry, I forgot to mention that commit message for 5be1cd89 in Bug 573932 has some extra information.

I think that this problem requires some debugging before it can understood enough to attempt a fix. I would advise against blind fixes, because these are likely to add weird code to be supported later and move bug to a new place.

[iloveeclipse]

Sure, feel free to look at it later. I will close my PR then. Thanks for pointer.

[SyntevoAlex]

To try to put Bug 573932 in other words:

1. When gtk_list_store_remove() is called, it might call cellDataProc(). I see that in both old and current Bug.
2. When it happens, SWT Table.items[] indexes disagree with GTK's item indexes, because GTK already removed the item.
3. When SWT tries to use GTK index, bad things happen.

[SyntevoAlex]

I have checked crash database for our product and there are exactly zero reports containing org.eclipse.swt.widgets.Table.remove or org.eclipse.swt.widgets.Tree.remove. It sounds like this is either an Eclipse side problem, or something not used in our product, or requires exotic steps. Either way, unfortunately I won't be able to prioritize this to do myself.

[akurtakov]

Is this one still relevant?