swingx
  1. swingx
  2. SWINGX-1536

ListSortUI calling method with wrong argument

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.6.1
    • Fix Version/s: 1.6.5-1
    • Component/s: Sort/filter
    • Labels:
      None

      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.

        Activity

        Hide
        kleopatra added a comment -

        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)

        Show
        kleopatra added a comment - 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)
        Hide
        nj1308 added a comment -

        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.

        Show
        nj1308 added a comment - 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.
        Hide
        kleopatra added a comment -

        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.

        Show
        kleopatra added a comment - 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.
        Hide
        nj1308 added a comment -

        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.

        Show
        nj1308 added a comment - 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.
        Hide
        kleopatra added a comment -

        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

        Show
        kleopatra added a comment - 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
        Hide
        nj1308 added a comment -

        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

        Show
        nj1308 added a comment - 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
        Hide
        kleopatra added a comment -

        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)
        Show
        kleopatra added a comment - 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)
        Hide
        nj1308 added a comment -

        Thanks for the quick fix.

        Show
        nj1308 added a comment - Thanks for the quick fix.
        Hide
        kleopatra added a comment -

        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?

        Show
        kleopatra added a comment - 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?
        Hide
        rmk_lp added a comment -

        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;

        Show
        rmk_lp added a comment - 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;
        Hide
        kleopatra added a comment -

        darn ... guilty of not reading the api doc

        Thanks, will fix soon!
        Jeanette

        Show
        kleopatra added a comment - darn ... guilty of not reading the api doc Thanks, will fix soon! Jeanette
        Hide
        rmk_lp added a comment -

        Great, thank you!

        Robert

        Show
        rmk_lp added a comment - Great, thank you! Robert
        Hide
        kleopatra added a comment -
        • reverted incorrect change to removeInterval (note: api is different from insertInterval!)
        • added test and visual check

        Compliments to your colleague - good eyes

        Cheers
        Jeanette

        Show
        kleopatra added a comment - reverted incorrect change to removeInterval (note: api is different from insertInterval!) added test and visual check Compliments to your colleague - good eyes Cheers Jeanette
        Hide
        kleopatra added a comment -

        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

        Show
        kleopatra added a comment - 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

          People

          • Assignee:
            kleopatra
            Reporter:
            nj1308
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1 hour
              1h
              Remaining:
              Remaining Estimate - 1 hour
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified