[SWINGX-1529] IndexOutOfBoundsException when deleting last rows in a JXTreeTable Created: 26/Oct/12  Updated: 08/Dec/12  Resolved: 08/Dec/12

Status: Resolved
Project: swingx
Component/s: TreeTable
Affects Version/s: 1.6.3, 1.6.4
Fix Version/s: 1.6.5

Type: Bug Priority: Major
Reporter: rkeen Assignee: kleopatra
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Tested with Java 6 and SwingX 1.6.3, but the source code for 1.6.4 appears to be the same in the affected area.


Attachments: Java Source File TreeTableException.java    
Tags: TreeTableModelAdapter, jxtreetable

 Description   

To reproduce:

  1. Create a JXTreeTable with the root hidden and 3 child nodes
  2. Remove the first and last child by removing them from the model and then calling TreeModelSupport.fireChildRemoved for each individually in display order.

After the second rowsDeleted event is handled by the display table's row sorter, you will get the following exception:

Exception in thread "AWT-EventQueue-0" java.lang.IndexOutOfBoundsException: Invalid range
	at javax.swing.DefaultRowSorter.rowsDeleted(DefaultRowSorter.java:863)
	at org.jdesktop.swingx.sort.DefaultSortController.rowsDeleted(DefaultSortController.java:398)
	at javax.swing.JTable.notifySorter(JTable.java:4262)
	at javax.swing.JTable.sortedTableChanged(JTable.java:4106)
	at javax.swing.JTable.tableChanged(JTable.java:4383)
	at org.jdesktop.swingx.JXTable.tableChanged(JXTable.java:1529)
	at javax.swing.table.AbstractTableModel.fireTableChanged(AbstractTableModel.java:280)
	at javax.swing.table.AbstractTableModel.fireTableRowsDeleted(AbstractTableModel.java:245)
	at org.jdesktop.swingx.JXTreeTable$TreeTableModelAdapter$6.run(JXTreeTable.java:2475)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:646)
	at java.awt.EventQueue.access$000(EventQueue.java:84)
	at java.awt.EventQueue$1.run(EventQueue.java:607)
	at java.awt.EventQueue$1.run(EventQueue.java:605)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.AccessControlContext$1.doIntersectionPrivilege(AccessControlContext.java:87)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:616)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

This happens because the first child removed event is processed after both children are removed from the tree within JXTreeTable.TreeTableModelAdapter since the fireTableRowsDeleted is called within a SwingUtilities.invokeLater. On the first call, the modelRowCount is updated which will then be 1. On the second call, the firstRow and endRow parameters will be 1 and the IndexOutOfBoundsException will be thrown.

The only workaround I've found for this is the following:

// Add new child
modelSupport.fireChildAdded(...);

SwingUtilities.invokeLater(new Runnable()
{
    @Override
    public void run()
    {
        // Remove target children and call fireChildRemoved for each individually
        SwingUtilities.invokeLater(new Runnable()
        {
            @Override
            public void run()
            {
                // Remove new child
                modelSupport.fireChildRemoved(...);
            }
        });
    }
});

If the new child you add and remove has no display text, this isn't noticeable to the user unless the row it takes up causes the table to display a scroll bar which then flashes momentarily.

Ideally, the handling would be synchronous, although the comment in JXTreeTable.TreeTableModelAdapter.getTreeModelListener indicates that this caused problems. The next best solution would be to change the logic in delayedFireTableDataChanged to only call fireTableRowsDeleted if the min index is less than the total rows and snap max to the total rows if it is more. I haven't tested either of these, so I'm not sure if they will work, but it seems like they could.



 Comments   
Comment by kleopatra [ 27/Oct/12 ]

hmm ... a bit surprised that the rowSorter is in the stackTrace: afair, sorting/filtering is disabled completely so it shouldn't be involved?

Anyway, could you please attach a small runnable code example so that I can reproduce the error (details matter

Thanks
Jeanette

Comment by rkeen [ 29/Oct/12 ]

I should have thought of this earlier, but it turns out that the problem presents itself when using a JTable (or other derivative) as a row header for a JXTreeTable. This is similar to the approach used on the Java Tips Blog for adding a fixed column to a table.

If I remove the row header, I don't get the exception. The workaround is to derive my own subclass of JTable for the row header, override tableChanged, and add a try...catch for IndexOutOfBoundsException, but it would be preferable not to do this.

@Override
public void tableChanged(TableModelEvent e)
{
    try
    {
        super.tableChanged(e);
    }
    catch(IndexOutOfBoundsException ex)
    {
        // Ignore
    }
}
Comment by rkeen [ 29/Oct/12 ]

To reproduce the exception, compile and run the attached file (TreeTableException.java) and click the "Remove Some Children" button which will remove the first and last items in the tree table and cause the IndexOutOfBoundsException.

Comment by kleopatra [ 29/Oct/12 ]

thanks for example (will look at it later this week)

Instead of your tweak in tableChanged you can simply disable sorting on the rowHeader table (by default autoCreateRowSorter is true, that's not needed being a rowHeader on an unsortable treeTable)

rowHeader.setAutoCreateRowSorter(false);
Comment by rkeen [ 29/Oct/12 ]

That workaround is better, but still requires that you subclass JXTable since both setSortable(true) and setAutoCreateRowSorter(true) are called from JXTable.init(). I ended up subclassing JXTable for my row header and overriding setRowSorter to do nothing. If you just call setAutoCreateRowSorter(false), it doesn't reset the previously created sorter. It's better to call rowHeader.setRowSorter(null) after setting the model. Doing so in the sample I attached avoids the exception, but you end up with the last row still visible in the row header. You can get rid of this by calling rowHeader.repaint() after removing the children, but this isn't a great solution as the updater of the model may not have a reference to the view.

Perhaps there should be a constructor parameter for JXTable to disable sorting by default.

Comment by kleopatra [ 30/Oct/12 ]

Great digging and great bug report!

We have several issues:

1) the asynchronous notification of the tableModel (relative to those from the treeModel) causes havoc on a rowSorter which sees that tableModel. So a table showing the model must not have a rowSorter

2) controlling J/X/Table auto-sorting is a bit hard, once it had been enabled. Both table variants require two method calls

// prevent the table from touching the rowSorter
table.setAutoCreateRowSorter(false)
// remove the rowSorter
table.setRowSorter(null);

It feels harder in JXTable, because auto-sorting is on by default. But the underlying behaviour is exactly the same: with autoCreate the table takes control, with !autoCreate its completely up to the developer to take control, including removing the sorter. Not sure if it's worth adding api to combine to two methods?

3) the asynchronous notifications also confuse the repaint mechanism of a table: that last deleted row doesn't get visually removed (except by a direct repaint), same for core and xTable. In your example:

JTable rowHeader = new JTable();
rowHeader.setFillsViewportHeight(true);
//... same further config

The real culprit - as you already found is the asynchronous notification. At the end of the day, that's hacking. Afair, there is nothing we can do about it - solves more problems than it poses. Hmm ..

Thanks
Jeanette

Comment by kleopatra [ 07/Dec/12 ]

still can't think of anything we could do here - there's no way around the asynchronous notification (except a complete revamp and throwing out the JTree as renderer altogether - which is a long-standing task, but ...

So tend to close this as won't fix - objections?

Thanks
Jeanette

Comment by rkeen [ 07/Dec/12 ]

While I'd love to see it fixed, at least there is a decent workaround and the issue is documented here, so I don't have any real objections to closing it.

Comment by kleopatra [ 08/Dec/12 ]

closing because we can't do much about it
added api doc with a link to this issue, to warn against re-using the adapted tableModel elsewhere

Generated at Tue Jun 02 20:20:58 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.