JDK-4730055 : getSelectedRows return non-existing rows in JTable
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.swing
  • Affected Version: 1.3.1,6
  • Priority: P3
  • Status: Closed
  • Resolution: Duplicate
  • OS: generic,windows_nt
  • CPU: generic,x86
  • Submitted: 2002-08-12
  • Updated: 2006-01-23
  • Resolved: 2006-01-23
Related Reports
Duplicate :  
Description
Name: jl125535			Date: 08/12/2002


FULL PRODUCT VERSION :
java version "1.3.1"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.3.1-b24)
Java HotSpot(TM) Client VM (build 1.3.1-b24, mixed mode)

java version "1.4.0"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.0-b92)
Java HotSpot(TM) Client VM (build 1.4.0-b92, mixed mode)

java version "1.4.1-rc"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.1-rc-b15)
Java HotSpot(TM) Client VM (build 1.4.1-rc-b15, mixed mode)


FULL OPERATING SYSTEM VERSION :
Windows NT 4.0, SP6

A DESCRIPTION OF THE PROBLEM :
When using getSelectedRows() or getSelectedRow() on a JTable
instance, inside a TableModelListener, the method return
row(s) that have been removed from the model. This is
certainly because the TableModelListener that changes the
selection model of the JTable is called after our
TableModelListener.
As a consequence, trying to get the values from these rows
results in an ArrayOutOfBoundException.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1. Create a JTable with its TableModel.
2. Add a TableModelListener to the TableModel. Implement the
tableChanged() method of this listener so that it calls
getSelectedRows() on the table and tries to get a value from
one of the returned rows of the table model.

EXPECTED VERSUS ACTUAL BEHAVIOR :
I expect the getSelectedRows() and getSelectedRow() method
to return the selected rows (or row). A selected row must be
a valid row, i.e. a row that exists in the table model.
Instead, I get a row that has just been removed from the
table model, and which thus can't be selected, since it
doesn't exist.

ERROR MESSAGES/STACK TRACES THAT OCCUR :
java.lang.ArrayIndexOutOfBoundsException: 1 >= 1
	at java.util.Vector.elementAt(Vector.java:417)
	at javax.swing.table.DefaultTableModel.getValueAt(DefaultTableModel.java:658)
	at jbn.test.TableBug.showTheBug(TableBug.java:65)
	at jbn.test.TableBug.access$0(TableBug.java:9)
	at jbn.test.TableBug$1.tableChanged(TableBug.java:36)
	at javax.swing.table.AbstractTableModel.fireTableChanged(AbstractTableModel.java:262)
	at
javax.swing.table.AbstractTableModel.fireTableRowsDeleted(AbstractTableModel.java:227)
	at javax.swing.table.DefaultTableModel.removeRow(DefaultTableModel.java:590)
	at jbn.test.TableBug$3.actionPerformed(TableBug.java:51)
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1450)
	at
javax.swing.AbstractButton$ForwardActionEvents.actionPerformed(AbstractButton.java:1504)
	at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:378)
	at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:250)
	at
javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:216)
	at java.awt.Component.processMouseEvent(Component.java:3715)
	at java.awt.Component.processEvent(Component.java:3544)
	at java.awt.Container.processEvent(Container.java:1164)
	at java.awt.Component.dispatchEventImpl(Component.java:2593)
	at java.awt.Container.dispatchEventImpl(Container.java:1213)
	at java.awt.Component.dispatchEvent(Component.java:2497)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:2451)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:2216)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:2125)
	at java.awt.Container.dispatchEventImpl(Container.java:1200)
	at java.awt.Window.dispatchEventImpl(Window.java:926)
	at java.awt.Component.dispatchEvent(Component.java:2497)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:339)
	at
java.awt.EventDispatchThread.pumpOneEventForHierarchy(EventDispatchThread.java:131)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:98)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:85)

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
package jbn.test;

import javax.swing.*;
import javax.swing.table.*;
import javax.swing.event.*;
import java.awt.*;
import java.awt.event.*;

public class TableBug extends JFrame {

  private JTable table;
  private DefaultTableModel tableModel;

  public TableBug() {
    super("Table bug");
    setDefaultCloseOperation(EXIT_ON_CLOSE);

    String[][] data = new String[2][2];
    data[0][0] = "hello";
    data[0][1] = "world";
    data[1][0] = "here's a";
    data[1][1] = "bug";

    tableModel = new DefaultTableModel(
      data,
      new String[] {"column 1", "column 2"});

    JLabel label = new JLabel(
      "<HTML>To show the bug, select the last row of the table " +
      "and press the button.<BR>" +
      "This will throw an exception because getSelectedRows() " +
      "return rows that are not <BR>" +
      "in the table model anymore.</HTML>");

    table = new JTable(tableModel);

    tableModel.addTableModelListener(new TableModelListener() {
      public void tableChanged(TableModelEvent e) {
        showTheBug();
      }
    });

    table.getSelectionModel().addListSelectionListener(
        new ListSelectionListener() {
      public void valueChanged(ListSelectionEvent e) {
        showTheBug();
      }
    });

    JButton button = new JButton("Show the bug");
    button.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent e) {
        int lastRow = tableModel.getRowCount() - 1;
        if (lastRow >= 0) {
          tableModel.removeRow(lastRow);
        }
      }
    });

    getContentPane().add(label, BorderLayout.NORTH);
    getContentPane().add(new JScrollPane(table), BorderLayout.CENTER);
    getContentPane().add(button, BorderLayout.SOUTH);

    pack();
  }

  private void showTheBug() {
    int[] selectedRows = table.getSelectedRows();
    Object firstObjectOfSelectedRow =
        tableModel.getValueAt(selectedRows[selectedRows.length - 1], 0);
  }

  public static void main(String[] args) {
    TableBug t = new TableBug();
    t.setVisible(true);
  }
}
---------- END SOURCE ----------

CUSTOMER WORKAROUND :
For each returned row, of getSelectedRows(), verify that
it's < tableModel.getRowCount().

Or

Subclass JTable and override getSelectedRow() and
getSelectedRows() to remove the non-existing rows from the
returned value.
(Review ID: 160136) 
======================================================================
Suggested by java.net member leouser:

A DESCRIPTION OF THE FIX :
BUG ID: 4730055 getSelectedRows return non-existing rows in JTable
FILES AFFECTED: javax.swing.event.package, javax.swing.table.AbstractTableModel
JDK VERSION
jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin

Discusion(embeded in test case as well):
/**
 * BUG ID: 4730055 getSelectedRows return non-existing rows in JTable.
 * This is an extremely vexing bug.  The essence of the problem is that
 * the listeners need to be notifed after the component they are working
 * with has been updated by the same event.  This is fundamentally impossible
 * to ensure with what exists today.  If you add the JTable last to the list
 * what guarantees are there that it in turn will get called first?  There
 * are none.  If you add it first, what guarantees are there that it will
 * be called last?  There are none.  The specification of addTableModelListener
 * is:
 * Adds a listener to the list that is notified each time a change to the data model occurs.
 * It doesn't state when it will occur.  Which leads us back to this quandary:
 * How can code be notified of the event after the component it is working with
 * has been updated by the same event?
 *
 * My thinking is that the TableModelListener does not offer the user enough
 * hooks.  We have only: tableChanged.  What is needed is a method called:
 * afterTableChanged(TableModelEvent e).  This method will guarantee in its
 * javadoc that all the components who are listening have been notified in
 * the tableChanged method has been called.  Where should it go?  It would
 * be great to toss it on the TableModelListener but we can't do this because
 * this would lead to the breaking of clients who implement this method.  They
 * suddenly won't compile.  Hence I believe the answer is:
 * ExtendedTableModelListener.  It is an interface that extends TabelModelListener
 * and adds the afterTableChanged method.  It will also allow us to register it
 * with the addTableModelListener method.  I considered doing a separate class
 * but that would entail adding a new method to TableModel and breaking all
 * the clients. Another odd idea was to add an addTableModelListener with some
 * form of order priority attached to it.  This doesn't fit in well with
 * the scheme of things and still really offers no guarantees when a TabelModelListener
 * will be called.  So you have priority 200, well the JTable has a priorty of 500,
 * guess what? the problem reoccurs.
 *
 * IMPACT: classes that subclass TableModel should ensure when firing a notification
 * that they notify any ExtendedTableModelListeners registered with them.
 *
 * ANTI-RATIONALE:
 * 1. This adds a new interface to the javax.swing.event package and this
 * interface exits to add only one method, complicating the package for one
 * method.  This shouldn't break any existing clients though.
 *
 * TESTING STRATEGY:
 * 1. Create 2 Guis one with the old broken way, one with the new way.  Interact
 * with these guis and see how they perform differently.  To show the bug
 * in the non-fixed gui select the last row, hit Show the Bug and watch the
 * exception get tossed.  Do the same in the Fixed version and watch the bug
 * not get tossed.
 *
 * HOOKAMANIA?
 * Given that we are adding an interface for one hook, is there a need for
 * another hook, maybe beforeTableChanged(TableModelEvent e)?  This one
 * seems dubious at this point.  I guess it could be useful for listeners
 * that need to takes some data from the component they are working with
 * before it is altered by the event. maybe. :D
 *
 * FILES AFFECTED: javax.swing.event.package, javax.swing.table.AbstractTableModel
 *
 * JDK VERSION
 * jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
 *
 * test ran succesfully on a SUSE 7.3 Linux distribution
 *
 * Brian Harry
 * ###@###.###
 * Jan 21, 2006
 */

UNIFIED DIFF:
--- /home/nstuff/java6/jdk1.6.0/javax/swing/table/AbstractTableModel.java	Thu Dec 15 02:17:52 2005
+++ /home/javarefs/javax/swing/table/AbstractTableModel.java	Sat Jan 21 10:35:54 2006
@@ -271,15 +271,13 @@
      * @see EventListenerList
      */
     public void fireTableChanged(TableModelEvent e) {
-	// Guaranteed to return a non-null array
-	Object[] listeners = listenerList.getListenerList();
-	// Process the listeners last to first, notifying
-	// those that are interested in this event
-	for (int i = listeners.length-2; i>=0; i-=2) {
-	    if (listeners[i]==TableModelListener.class) {
-		((TableModelListener)listeners[i+1]).tableChanged(e);
-	    }
-	}
+	TableModelListener[] tml = listenerList.getListeners(TableModelListener.class);
+        for(TableModelListener t: tml)
+	    t.tableChanged(e);
+       
+        for(TableModelListener t: tml)
+	    if(t instanceof ExtendedTableModelListener)
+                ((ExtendedTableModelListener)t).afterTableChanged(e);
     }
 
     /**


NEW FILE: javax.swing.event.ExtendedTableModelListener.java:
package javax.swing.event;

/**
 * {@code ExtendedTableModelListener} provides an additional hook for notification
 * of a {@code TableModelEvent}.
 */
public interface ExtendedTableModelListener extends TableModelListener{
    
    /**
     * Will be called after other {@code TableModelListener} instances
     * have been notified of the {@code TableModelEvent} through
     * the {@code tableChanged} method.
     *
     * @param e the event
     */
    public void afterTableChanged(TableModelEvent e);

}




JUnit TESTCASE :
import javax.swing.*;
import javax.swing.table.*;
import javax.swing.event.*;
import java.awt.*;
import java.awt.event.*;
import static java.lang.System.out;

/**
 * BUG ID: 4730055 getSelectedRows return non-existing rows in JTable.
 * This is an extremely vexing bug.  The essence of the problem is that
 * the listeners need to be notifed after the component they are working
 * with has been updated by the same event.  This is fundamentally impossible
 * to ensure with what exists today.  If you add the JTable last to the list
 * what guarantees are there that it in turn will get called first?  There
 * are none.  If you add it first, what guarantees are there that it will
 * be called last?  There are none.  The specification of addTableModelListener
 * is:
 * Adds a listener to the list that is notified each time a change to the data model occurs.
 * It doesn't state when it will occur.  Which leads us back to this quandary:
 * How can code be notified of the event after the component it is working with
 * has been updated by the same event?
 *
 * My thinking is that the TableModelListener does not offer the user enough
 * hooks.  We have only: tableChanged.  What is needed is a method called:
 * afterTableChanged(TableModelEvent e).  This method will guarantee in its
 * javadoc that all the components who are listening have been notified in
 * the tableChanged method has been called.  Where should it go?  It would
 * be great to toss it on the TableModelListener but we can't do this because
 * this would lead to the breaking of clients who implement this method.  They
 * suddenly won't compile.  Hence I believe the answer is:
 * ExtendedTableModelListener.  It is an interface that extends TabelModelListener
 * and adds the afterTableChanged method.  It will also allow us to register it
 * with the addTableModelListener method.  I considered doing a separate class
 * but that would entail adding a new method to TableModel and breaking all
 * the clients. Another odd idea was to add an addTableModelListener with some
 * form of order priority attached to it.  This doesn't fit in well with
 * the scheme of things and still really offers no guarantees when a TabelModelListener
 * will be called.  So you have priority 200, well the JTable has a priorty of 500,
 * guess what? the problem reoccurs.
 *
 * IMPACT: classes that subclass TableModel should ensure when firing a notification
 * that they notify any ExtendedTableModelListeners registered with them.
 *
 * ANTI-RATIONALE:
 * 1. This adds a new interface to the javax.swing.event package and this
 * interface exits to add only one method, complicating the package for one
 * method.  This shouldn't break any existing clients though.
 *
 * TESTING STRATEGY:
 * 1. Create 2 Guis one with the old broken way, one with the new way.  Interact
 * with these guis and see how they perform differently.  To show the bug
 * in the non-fixed gui select the last row, hit Show the Bug and watch the
 * exception get tossed.  Do the same in the Fixed version and watch the bug
 * not get tossed.
 *
 * HOOKAMANIA?
 * Given that we are adding an interface for one hook, is there a need for
 * another hook, maybe beforeTableChanged(TableModelEvent e)?  This one
 * seems dubious at this point.  I guess it could be useful for listeners
 * that need to takes some data from the component they are working with
 * before it is altered by the event. maybe. :D
 *
 * FILES AFFECTED: javax.swing.event.package, javax.swing.table.AbstractTableModel
 *
 * JDK VERSION
 * jdk-6-rc-bin-b64-linux-i586-15_dec_2005.bin
 *
 * test ran succesfully on a SUSE 7.3 Linux distribution
 *
 * Brian Harry
 * ###@###.###
 * Jan 21, 2006
 */
public class TestTable4730055{
    
    JFrame jf1;
    JFrame jf2;
    JTable jt;
    JTable jt2;

    public void createGUI1(){
	jf1 = new JFrame();
	jf1.setTitle("BUG EXISTS FRAME");
	Object[][] data = new Object[10][10];
	Object[] names = new Object[10];
	for(int i = 0; i < 10; i++){
	    names[i] = String.valueOf(i);
	    for(int i2 = 0; i2 < 10; i2++)
		data[i][i2] = i2;

	}
	final DefaultTableModel tm = new DefaultTableModel(data, names);
	jt = new JTable(tm);
	JScrollPane jsp = new JScrollPane(jt);
	jf1.add(jsp);


	tm.addTableModelListener(new TableModelListener(){
		public void tableChanged(TableModelEvent e){
		    showBug(jt);
		}
		
	    });

	jt.getSelectionModel().addListSelectionListener(
	    new ListSelectionListener(){

		    public void valueChanged(ListSelectionEvent e){
			out.println("BOOGA!");
			//Thread.currentThread().dumpStack();
			showBug(jt);
		    }



	});

	Action a = new AbstractAction("Show the bug"){

		public void actionPerformed(ActionEvent ae){
		    int lastRow = tm.getRowCount() -1;
		    out.println("FIRE!!!!!");
		    if(lastRow >= 0)
			tm.removeRow(lastRow);

		}

	    };
	JButton jb = new JButton(a);
	jf1.add(jb, BorderLayout.SOUTH);
	jf1.pack();
	jf1.setVisible(true);
    }

    public void createGUI2(){
	jf2 = new JFrame();
	jf2.setTitle("BUG FIXED FRAME");
	Object[][] data = new Object[10][10];
	Object[] names = new Object[10];
	for(int i = 0; i < 10; i++){
	    names[i] = String.valueOf(i);
	    for(int i2 = 0; i2 < 10; i2++)
		data[i][i2] = i2;

	}
	final DefaultTableModel tm = new DefaultTableModel(data, names);
	jt2 = new JTable(tm);
	JScrollPane jsp = new JScrollPane(jt2);
	jf2.add(jsp);


	tm.addTableModelListener(new ExtendedTableModelListener(){
		public void tableChanged(TableModelEvent e){
		    //showBug();
		}

		public void afterTableChanged(TableModelEvent e){
		    showBug(jt2);
		}
		
	    });

	jt.getSelectionModel().addListSelectionListener(
	    new ListSelectionListener(){

		    public void valueChanged(ListSelectionEvent e){
			out.println("BOOGA!");
			//Thread.currentThread().dumpStack();
			showBug(jt2);
		    }



	});

	Action a = new AbstractAction("Show the bug"){

		public void actionPerformed(ActionEvent ae){
		    int lastRow = tm.getRowCount() -1;
		    out.println("FIRE!!!!!");
		    if(lastRow >= 0)
			tm.removeRow(lastRow);

		}

	    };
	JButton jb = new JButton(a);
	jf2.add(jb, BorderLayout.SOUTH);
	jf2.pack();
        Point p = jf1.getLocation();
	int x = p.x + jf1.getSize().width;
	jf2.setLocation(x, 0);
	jf2.setVisible(true);
    }

    public void showBug(JTable jt){
	int[] rows = jt.getSelectedRows();
	out.println("These rows are selected:");
	for(int i: rows)
	    out.print(i + " " );
	out.println();
	if(rows.length > 0 )
	    jt.getModel().getValueAt(rows[rows.length-1],0);

    }

    public void testTable(){
	Runnable run = new Runnable(){
		public void run(){
		    createGUI1();
		    createGUI2();

		}

	    };
	SwingUtilities.invokeLater(run);

    }

    public static void main(String ... args){

	new TestTable4730055().testTable();

    }


}


FIX FOR BUG NUMBER:
4730055

Comments
EVALUATION Contribution-forum:https://jdk-collaboration.dev.java.net/servlets/ProjectForumMessageView?forumID=1463&messageID=10982
23-01-2006

EVALUATION Swing uses listeners on the table to update its selection model. This developer has written code that tries to fetch the selection before the selection model has been updated. This is not something that we can fix since it is part of the Swing design. Making any modifications to JTable.getSelectedRows() would simply be a hack. There is, however, an RFE that proposes to add an ordering API for adding listeners to Swing components. Once this RFE, 4178930, is implemented, developers will be able to add listeners that get notified after the Swing listeners.
23-01-2006