<< Back to previous view

[SWINGX-1520] row count returns cached value Created: 10/Aug/12  Updated: 16/Aug/12  Resolved: 16/Aug/12

Status: Resolved
Project: swingx
Component/s: Table
Affects Version/s: 1.6.4
Fix Version/s: 1.6.5

Type: Bug Priority: Major
Reporter: tbee Assignee: kleopatra
Resolution: Invalid Votes: 0
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Tags: defaultsortcontroller jxtable
Participants: kleopatra and tbee

 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?



 Comments   
Comment by kleopatra [ 10/Aug/12 02:41 PM ]

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?

Comment by tbee [ 10/Aug/12 03:36 PM ]

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.

Comment by kleopatra [ 10/Aug/12 04:23 PM ]

(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

Comment by kleopatra [ 16/Aug/12 07:49 AM ]

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

Comment by tbee [ 16/Aug/12 08:19 AM ]

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?

Comment by tbee [ 16/Aug/12 08:20 AM ]

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?

Comment by kleopatra [ 16/Aug/12 09:44 AM ]

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.

Comment by tbee [ 16/Aug/12 09:58 AM ]

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

Comment by kleopatra [ 16/Aug/12 10:02 AM ]

closing as invalid by consent with reporter

Thanks for bringing this up, so we can be sure

Generated at Wed Apr 16 05:49:55 UTC 2014 using JIRA 4.0.2#472.