[SWINGX-1536] ListSortUI calling method with wrong argument Created: 05/Dec/12  Updated: 28/Mar/13  Resolved: 22/Feb/13

Status: Resolved
Project: swingx
Component/s: Sort/filter
Affects Version/s: 1.6.1
Fix Version/s: 1.6.5-1

Type: Bug Priority: Blocker
Reporter: nj1308 Assignee: kleopatra
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 1 hour
Time Spent: Not Specified
Original Estimate: 1 hour

Tags: Listsortui

 Description   

This is a straightforward bug and it's obvious within secs of checking.
In ListSortUI class:
private void prepareForChange(ModelChange change) there is a call to modelSelection.insertIndexInterval(change.startModelIndex, change.endModelIndex, true);

The 2nd arg should be a length rather than an end index.

The same bug exists in JTable.SortManager.cacheSelection. I reported to Oracle, but I couldn't get Oracle to fix it.

Thanks.



 Comments   
Comment by kleopatra [ 06/Dec/12 ]

oops ... cough ... c&p is an evil habit

Curious: how/whom did you contact at Oracle and how did they decline to fix it? Because this is really gross - and we can't do anything to fix the same error in JXTable (SortManager is high-security-private)

Comment by nj1308 [ 06/Dec/12 ]

I raised a bug report to Oracle for the JTable bug a few months back. It was accepted, a few months later, when I went back, couldn't find the bug in bug DB anymore. I raised it a second time, and I was told it's going to be an internal review, but nothing came back from that either and the bug is still at large.

Guess, they are waiting for someone with a service contract to fix it, but the bug is a big turn off for any serious use of swing.

As you said it's private member of a private class, I would have done bootpath patch if it were a server bug, but for our webstart gui, this is not possible too.

At least I hope it can be fixed in swingx.

Comment by kleopatra [ 06/Dec/12 ]

do you by any chance have the bug id (or at least the review id)? Might try to activate a private channel - without much hope of succeeding, it's a bit dried up

Will certainly fix for xlist, but can't do anything for xtable, sorry.

Comment by nj1308 [ 06/Dec/12 ]

The bug was initially accepted on 06/28/2012 with Bug ID: 7180549
The 2nd time on 10/26/2012, Review ID: 2367035

Really appreciate your quick response and that would be great if the private channel still works.

Comment by kleopatra [ 06/Dec/12 ]

will try

Regarding the issue itself: just trying to find a failing test case which would pass when the bug is fixed - but failing so far (the end of day for me, need some food).

Maybe you have an example, or at least a description of what evil happens due to it?

Thanks
Jeanette

Comment by nj1308 [ 06/Dec/12 ]

The symptom is typically an ArrayIndexOutOfBoundsException exception when one wants to get the selected rows at a later time.

The conditions for it to happen are:
1. Table filter is set;
2. Table is sorting;
3. Some rows are selected, but the view indexes(or selection indexes) are changing when table rows are updated, added and removed.

The bug does not normally kill immediately, but after a few rounds of it, the error accumulates until you try to get selected rows, it goes out of bound.

It took me 6 hours to find the bug. Not from stacktracing but from reading the code directly right before going crazy. There are similar reports on the internet, but they failed to identify the root cause(http://www.java.net/node/702161?force=979). I guess it's because the accumulating nature of the bug, the seemingly randomness and the inconsistent ListSelectionModel method signature.

Well, now to read the link again, you actually have commented on the problem!
I guess it meant to come back and haunt you again

Yang

Comment by kleopatra [ 07/Dec/12 ]

fix committed as of revision #4267

  • fixed ListSortUI
  • added test (failing before/passing after fix)
  • added test/visuals to JTableIssues to remember (must be handled by core)
Comment by nj1308 [ 07/Dec/12 ]

Thanks for the quick fix.

Comment by kleopatra [ 10/Dec/12 ]

feedback:

"The issue with the AIOOB in the JTable.SortManager.cacheSelection should be easy to fix.
Did you use the http://bugs.sun.com to file the issue? I can't find the issue 7180549 or 2367035 in our bug tracking system."

did you?

Comment by rmk_lp [ 21/Feb/13 ]

Spotted by a colleague of mine:

The fix seems to have introduced a new bug as the second argument of removeIndexInterval according to its javadoc is supposed to be an index and not the length as opposed to insertIndexInterval.

case ListDataEvent.INTERVAL_REMOVED:
modelSelection.removeIndexInterval(change.startModelIndex,
// remove is fixed without test
change.length);
break;
case ListDataEvent.INTERVAL_ADDED:
modelSelection.insertIndexInterval(change.startModelIndex,
// insert is tested
change.length, true);
break;

Comment by kleopatra [ 21/Feb/13 ]

darn ... guilty of not reading the api doc

Thanks, will fix soon!
Jeanette

Comment by rmk_lp [ 21/Feb/13 ]

Great, thank you!

Robert

Comment by kleopatra [ 22/Feb/13 ]
  • reverted incorrect change to removeInterval (note: api is different from insertInterval!)
  • added test and visual check

Compliments to your colleague - good eyes

Cheers
Jeanette

Comment by kleopatra [ 28/Mar/13 ]

fyi: the table bug is fixed in core for jdk8 (and probably backported to 6/7, didn't verify)

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005019

Generated at Fri Mar 06 08:39:41 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.