Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 1.6.4
    • Fix Version/s: 1.6.5
    • Component/s: Table
    • Labels:
      None

      Description

      I have a JTableNavigator component that calls getRowCount on the table but on occasion gets the wrong value. After some researching I found that it get the value of cachedModelRowCount in DefaultSortController. This strange thing is; this happens without sorting, or more importantly filtering, being active. The situation is fairly complex, using asynchronous loading on tabs, so creating a simple example is quite complex. I've patched it by explicitly clearing the row sorter (setRowSorter(null)). Maybe an additional check if sorting or filtering is actually active should be added before using the cached value?

        Activity

        Hide
        kleopatra added a comment -

        hmm ... wouldn't it be you I would guess it to be an EDT problem

        We should try to somehow reproduce it, or at least narrow it (would hate blind additional checks - that caching on SortController actually is a hack around super misbehaving, so adding yet another hack on top ... )

        Only happens if a sorter is registered but nothing sorted nor filtered? When do you clear the sorter, after/during loading?

        Show
        kleopatra added a comment - hmm ... wouldn't it be you I would guess it to be an EDT problem We should try to somehow reproduce it, or at least narrow it (would hate blind additional checks - that caching on SortController actually is a hack around super misbehaving, so adding yet another hack on top ... ) Only happens if a sorter is registered but nothing sorted nor filtered? When do you clear the sorter, after/during loading?
        Hide
        tbee added a comment -

        Nah, this happily runs all inside the EDT. The easiest way would be to remote desktop show you

        What happens is that the TableNavigator does a getRowCount, the JTable checks if a sorter is active (it is, SwingX's DefaultSortController), forwards the call to the sorter and returns the cached value. Which is 0 instead of the actual number of rows present in the model.

        I extend JXTable with my own JTable class (which makes reproducing the problem even more simple) adding the MSAccess alike data entry functionality... Maybe I should first see if things need to be cleaned up there, chances are it is caused by something I setup; this inheritance has 'survived' years of SwingX versions. Never the less, fact is that getRowCount returns cachedRowCount, which value is incorrect. Somewhere in my usage the cache update logic misses a beat.

        Show
        tbee added a comment - Nah, this happily runs all inside the EDT. The easiest way would be to remote desktop show you What happens is that the TableNavigator does a getRowCount, the JTable checks if a sorter is active (it is, SwingX's DefaultSortController), forwards the call to the sorter and returns the cached value. Which is 0 instead of the actual number of rows present in the model. I extend JXTable with my own JTable class (which makes reproducing the problem even more simple) adding the MSAccess alike data entry functionality... Maybe I should first see if things need to be cleaned up there, chances are it is caused by something I setup; this inheritance has 'survived' years of SwingX versions. Never the less, fact is that getRowCount returns cachedRowCount, which value is incorrect. Somewhere in my usage the cache update logic misses a beat.
        Hide
        kleopatra added a comment -

        (damned internet - lost my last coment

        So here's a short version:

        Sounds like the navigator queries the table before its internal state is completely updated.

        See the core bug mentioned in the DefaultSortController java doc and the forum discussions around the fix in SwingX.

        Could well be that you hit a corner case which isn't fully covered by the logic of the workaround. Hard to say without a test case

        Show
        kleopatra added a comment - (damned internet - lost my last coment So here's a short version: Sounds like the navigator queries the table before its internal state is completely updated. See the core bug mentioned in the DefaultSortController java doc and the forum discussions around the fix in SwingX. Could well be that you hit a corner case which isn't fully covered by the logic of the workaround. Hard to say without a test case
        Hide
        kleopatra added a comment -

        here's a code snippet demonstrating what I mean:

            final DefaultTableModel model = new DefaultTableModel(5, 3);
            final JXTable table = new JXTable(model);
            final JTable core = new JTable(model);
            TableModelListener l = new TableModelListener() {
                
                @Override
                public void tableChanged(TableModelEvent e) {
                    LOG.info("model/X/core: " + model.getRowCount() 
                    // the following two lines are _illegal_
                            + "/" + table.getRowCount()
                            + "/" + core.getRowCount()
                            );
                }
            };
            model.addTableModelListener(l);
            Action action = new AbstractAction() {
        
                @Override
                public void actionPerformed(ActionEvent e) {
                    model.addRow(new Object[] {null, null});
                }
                
            };
            new Timer(100, action).start();
        
        

        As there is no guarantee to the sequence of notification, client code must not access notifier-dependent state of sister listeners. Core table's rowsorter accidentally has the same state (at the price of the core bug). The typical safe way to query sister listener's state is to invoke the access.

        So tend to close this as invalid, lest you convince me that your code isn't violating the rule

        Show
        kleopatra added a comment - here's a code snippet demonstrating what I mean: final DefaultTableModel model = new DefaultTableModel(5, 3); final JXTable table = new JXTable(model); final JTable core = new JTable(model); TableModelListener l = new TableModelListener() { @Override public void tableChanged(TableModelEvent e) { LOG.info( "model/X/core: " + model.getRowCount() // the following two lines are _illegal_ + "/" + table.getRowCount() + "/" + core.getRowCount() ); } }; model.addTableModelListener(l); Action action = new AbstractAction() { @Override public void actionPerformed(ActionEvent e) { model.addRow( new Object [] { null , null }); } }; new Timer(100, action).start(); As there is no guarantee to the sequence of notification, client code must not access notifier-dependent state of sister listeners. Core table's rowsorter accidentally has the same state (at the price of the core bug). The typical safe way to query sister listener's state is to invoke the access. So tend to close this as invalid, lest you convince me that your code isn't violating the rule
        Hide
        tbee added a comment -

        Coincidentally I've been looking into this just now, based on you second comment, and indeed is bar notified using a TableModelListener and then gets the row count. Exactly as you describe.

        A discussion naturally is; how should anyone know that table.getRowCount() is depending on a sister listener (that was introduced somewhere in the not too distant past)? It seems to me that the call to table.getRowCount() or model.getRowCount() is valid for a table model listener to do. Is this caching introducing a dependency to a sister listener that should not be?

        Show
        tbee added a comment - Coincidentally I've been looking into this just now, based on you second comment, and indeed is bar notified using a TableModelListener and then gets the row count. Exactly as you describe. A discussion naturally is; how should anyone know that table.getRowCount() is depending on a sister listener (that was introduced somewhere in the not too distant past)? It seems to me that the call to table.getRowCount() or model.getRowCount() is valid for a table model listener to do. Is this caching introducing a dependency to a sister listener that should not be?
        Hide
        tbee added a comment -

        And naturally the logical follow up; if a table model listener cannot get the row count by calling getRowCount(), where should it get that information from?

        Show
        tbee added a comment - And naturally the logical follow up; if a table model listener cannot get the row count by calling getRowCount(), where should it get that information from?
        Hide
        kleopatra added a comment - - edited

        It seems to me that the call to table.getRowCount() or model.getRowCount() is valid for a table model listener to do

        the latter is always valid, the former only after the table (as sister listener) has updated its internal state completely. It's always valid to invoke the former, that is delay until the current event handling chain is ready. Which isn't new: from day zero the table itself has been listening to its model

        It always was incorrect to access sister state that's potentially not yet updated, f.i. you wouldn't query the selection index in your tableModelListener. Same with model-related stuff.

        Simply wrap the access into a SwingUtilities.invokeLater, that is and always has been the recommended way to handle those sister listener contexts. The new thingy is, that - with the advent of filtering - the rowCount belongs to the properties that are no longer safe to access before the table's internal update is ready.

        Show
        kleopatra added a comment - - edited It seems to me that the call to table.getRowCount() or model.getRowCount() is valid for a table model listener to do the latter is always valid, the former only after the table (as sister listener) has updated its internal state completely. It's always valid to invoke the former, that is delay until the current event handling chain is ready. Which isn't new: from day zero the table itself has been listening to its model It always was incorrect to access sister state that's potentially not yet updated, f.i. you wouldn't query the selection index in your tableModelListener. Same with model-related stuff. Simply wrap the access into a SwingUtilities.invokeLater, that is and always has been the recommended way to handle those sister listener contexts. The new thingy is, that - with the advent of filtering - the rowCount belongs to the properties that are no longer safe to access before the table's internal update is ready.
        Hide
        tbee added a comment -

        You are right, I'll change my code. Thanks for figuring this out.

        Show
        tbee added a comment - You are right, I'll change my code. Thanks for figuring this out.
        Hide
        kleopatra added a comment -

        closing as invalid by consent with reporter

        Thanks for bringing this up, so we can be sure

        Show
        kleopatra added a comment - closing as invalid by consent with reporter Thanks for bringing this up, so we can be sure

          People

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

            Dates

            • Created:
              Updated:
              Resolved: