6733294: MBeans tab - UI issues with writable attributes
authordfuchs
Fri, 08 Aug 2008 14:24:31 +0200
changeset 1017 8d24d37ceed8
parent 1015 98e761716380
child 1019 b07cf7c26db9
6733294: MBeans tab - UI issues with writable attributes Reviewed-by: emcmanus
jdk/make/netbeans/jconsole/build.properties
jdk/make/netbeans/jconsole/build.xml
jdk/src/share/classes/sun/tools/jconsole/inspector/TableSorter.java
jdk/src/share/classes/sun/tools/jconsole/inspector/XMBeanAttributes.java
jdk/src/share/classes/sun/tools/jconsole/inspector/XPlotter.java
jdk/src/share/classes/sun/tools/jconsole/inspector/XSheet.java
jdk/src/share/classes/sun/tools/jconsole/inspector/XTable.java
jdk/src/share/classes/sun/tools/jconsole/inspector/XTextFieldEditor.java
--- a/jdk/make/netbeans/jconsole/build.properties	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/make/netbeans/jconsole/build.properties	Fri Aug 08 14:24:31 2008 +0200
@@ -44,3 +44,4 @@
 build.release = ${build.jdk.version}-opensource
 build.number = b00
 jconsole.version = ${build.release}-${user.name}-${build.number}
+jconsole.args = -debug
--- a/jdk/make/netbeans/jconsole/build.xml	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/make/netbeans/jconsole/build.xml	Fri Aug 08 14:24:31 2008 +0200
@@ -30,9 +30,9 @@
 -->
 
 <project name="jconsole" default="build" basedir=".">
-    
+
     <import file="../common/shared.xml"/>
-    
+
     <target name="-pre-compile">
         <copy
             file="${root}/src/share/classes/sun/tools/jconsole/Version-template.java"
@@ -42,7 +42,7 @@
             token="@@jconsole_version@@"
             value="${jconsole.version}"/>
     </target>
-    
+
     <target name="-post-compile">
         <mkdir dir="${dist.dir}/lib"/>
         <jar destfile="${dist.dir}/lib/jconsole.jar"
@@ -55,18 +55,19 @@
             </fileset>
         </jar>
     </target>
-    
+
     <target name="run" depends="-init,build">
         <property name="jvm.args" value=""/>
         <java classname="sun.tools.jconsole.JConsole"
               fork="true"
               classpath="${classes.dir}:${bootstrap.jdk}/lib/tools.jar">
             <jvmarg line="${jvm.args}"/>
+            <arg line="${jconsole.args}"/>
         </java>
     </target>
-    
+
     <target name="clean" depends="-init,shared.clean">
         <delete file="${dist.dir}/lib/jconsole.jar"/>
     </target>
-    
+
 </project>
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/TableSorter.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/TableSorter.java	Fri Aug 08 14:24:31 2008 +0200
@@ -25,71 +25,78 @@
 
 package sun.tools.jconsole.inspector;
 
-import java.util.*;
-import java.awt.event.*;
-import javax.swing.table.*;
-import javax.swing.event.*;
 
 // Imports for picking up mouse events from the JTable.
 
-import java.awt.event.MouseAdapter;
 import java.awt.event.MouseEvent;
-import java.awt.event.InputEvent;
+import java.awt.event.MouseListener;
+import java.util.Vector;
 import javax.swing.JTable;
+import javax.swing.event.TableModelEvent;
+import javax.swing.event.TableModelListener;
+import javax.swing.table.DefaultTableModel;
 import javax.swing.table.JTableHeader;
 import javax.swing.table.TableColumnModel;
+import sun.tools.jconsole.JConsole;
 
 @SuppressWarnings("serial")
 public class TableSorter extends DefaultTableModel implements MouseListener {
     private boolean ascending = true;
     private TableColumnModel columnModel;
     private JTable tableView;
-    private Vector<TableModelListener> listenerList;
+    private Vector<TableModelListener> evtListenerList;
     private int sortColumn = 0;
 
     private int[] invertedIndex;
 
     public TableSorter() {
         super();
-        listenerList = new Vector<TableModelListener>();
+        evtListenerList = new Vector<TableModelListener>();
     }
 
     public TableSorter(Object[] columnNames, int numRows) {
         super(columnNames,numRows);
-        listenerList = new Vector<TableModelListener>();
+        evtListenerList = new Vector<TableModelListener>();
     }
 
+    @Override
     public void newDataAvailable(TableModelEvent e) {
         super.newDataAvailable(e);
         invertedIndex = new int[getRowCount()];
-        for (int i=0;i<invertedIndex.length;i++) {
+        for (int i = 0; i < invertedIndex.length; i++) {
             invertedIndex[i] = i;
         }
-        sort(this.sortColumn);
+        sort(this.sortColumn, this.ascending);
     }
 
+    @Override
     public void addTableModelListener(TableModelListener l) {
-        listenerList.add(l);
+        evtListenerList.add(l);
         super.addTableModelListener(l);
     }
 
+    @Override
     public void removeTableModelListener(TableModelListener l) {
-        listenerList.remove(l);
+        evtListenerList.remove(l);
         super.removeTableModelListener(l);
     }
 
     private void removeListeners() {
-        for(TableModelListener tnl : listenerList)
+        for(TableModelListener tnl : evtListenerList)
             super.removeTableModelListener(tnl);
     }
 
     private void restoreListeners() {
-        for(TableModelListener tnl : listenerList)
+        for(TableModelListener tnl : evtListenerList)
             super.addTableModelListener(tnl);
     }
 
     @SuppressWarnings("unchecked")
-    public int compare(Object o1, Object o2) {
+    private int compare(Object o1, Object o2) {
+        // take care of the case where both o1 & o2 are null. Needed to keep
+        // the method symetric. Without this quickSort gives surprising results.
+        if (o1 == o2)
+            return 0;
         if (o1==null)
             return 1;
         if (o2==null)
@@ -104,18 +111,40 @@
         }
     }
 
-    public void sort(int column) {
+    private void sort(int column, boolean isAscending) {
+        final XMBeanAttributes attrs =
+                (tableView instanceof XMBeanAttributes)
+                ?(XMBeanAttributes) tableView
+                :null;
+
+        // We cannot sort rows when a cell is being
+        // edited - so we're going to cancel cell editing here if needed.
+        // This might happen when the user is editing a row, and clicks on
+        // another row without validating. In that case there are two events
+        // that compete: one is the validation of the value that was previously
+        // edited, the other is the mouse click that opens the new editor.
+        //
+        // When we reach here the previous value is already validated, and the
+        // old editor is closed, but the new editor might have opened.
+        // It's this new editor that wil be cancelled here, if needed.
+        //
+        if (attrs != null && attrs.isEditing())
+            attrs.cancelCellEditing();
+
         // remove registered listeners
         removeListeners();
         // do the sort
-        //n2sort(column);
-        quickSort(0,getRowCount()-1,column);
+
+        if (JConsole.isDebug()) {
+            System.err.println("sorting table against column="+column
+                    +" ascending="+isAscending);
+        }
+        quickSort(0,getRowCount()-1,column,isAscending);
         // restore registered listeners
         restoreListeners();
-        this.sortColumn = column;
+
         // update row heights in XMBeanAttributes (required by expandable cells)
-        if (tableView instanceof XMBeanAttributes) {
-            XMBeanAttributes attrs = (XMBeanAttributes) tableView;
+        if (attrs != null) {
             for (int i = 0; i < getRowCount(); i++) {
                 Vector data = (Vector) dataVector.elementAt(i);
                 attrs.updateRowHeight(data.elementAt(1), i);
@@ -123,21 +152,21 @@
         }
     }
 
-    private synchronized boolean compareS(Object s1, Object s2) {
-        if (ascending)
+    private boolean compareS(Object s1, Object s2, boolean isAscending) {
+        if (isAscending)
             return (compare(s1,s2) > 0);
         else
             return (compare(s1,s2) < 0);
     }
 
-    private synchronized boolean compareG(Object s1, Object s2) {
-        if (ascending)
+    private boolean compareG(Object s1, Object s2, boolean isAscending) {
+        if (isAscending)
             return (compare(s1,s2) < 0);
         else
             return (compare(s1,s2) > 0);
     }
 
-    private synchronized void quickSort(int lo0,int hi0, int key) {
+    private void quickSort(int lo0,int hi0, int key, boolean isAscending) {
         int lo = lo0;
         int hi = hi0;
         Object mid;
@@ -153,14 +182,14 @@
                          * from the left Index.
                          */
                         while( ( lo < hi0 ) &&
-                               ( compareS(mid,getValueAt(lo,key)) ))
+                               ( compareS(mid,getValueAt(lo,key), isAscending) ))
                             ++lo;
 
                         /* find an element that is smaller than or equal to
                          * the partition element starting from the right Index.
                          */
                         while( ( hi > lo0 ) &&
-                               ( compareG(mid,getValueAt(hi,key)) ))
+                               ( compareG(mid,getValueAt(hi,key), isAscending) ))
                             --hi;
 
                         // if the indexes have not crossed, swap
@@ -177,27 +206,17 @@
                                  * must now sort the left partition.
                                  */
                 if( lo0 < hi )
-                    quickSort(lo0, hi , key);
+                    quickSort(lo0, hi , key, isAscending);
 
                                 /* If the left index has not reached the right
                                  * side of array
                                  * must now sort the right partition.
                                  */
                 if( lo <= hi0 )
-                    quickSort(lo, hi0 , key);
+                    quickSort(lo, hi0 , key, isAscending);
             }
     }
 
-    public void n2sort(int column) {
-        for (int i = 0; i < getRowCount(); i++) {
-            for (int j = i+1; j < getRowCount(); j++) {
-                if (compare(getValueAt(i,column),getValueAt(j,column)) == -1) {
-                    swap(i, j, column);
-                }
-            }
-        }
-    }
-
     private Vector getRow(int row) {
         return (Vector) dataVector.elementAt(row);
     }
@@ -207,7 +226,7 @@
         dataVector.setElementAt(data,row);
     }
 
-    public void swap(int i, int j, int column) {
+    private void swap(int i, int j, int column) {
         Vector data = getRow(i);
         setRow(getRow(j),i);
         setRow(data,j);
@@ -223,11 +242,12 @@
 
     public void sortByColumn(int column, boolean ascending) {
         this.ascending = ascending;
-        sort(column);
+        this.sortColumn = column;
+        sort(column,ascending);
     }
 
-    public int[] getInvertedIndex() {
-        return invertedIndex;
+    public int getIndexOfRow(int row) {
+        return invertedIndex[row];
     }
 
     // Add a mouse listener to the Table to trigger a table sort
@@ -243,6 +263,14 @@
         int viewColumn = columnModel.getColumnIndexAtX(e.getX());
         int column = tableView.convertColumnIndexToModel(viewColumn);
         if (e.getClickCount() == 1 && column != -1) {
+            if (tableView instanceof XTable) {
+                XTable attrs = (XTable) tableView;
+                // inform the table view that the rows are going to be sorted
+                // against the values in a given column. This gives the
+                // chance to the table view to close its editor - if needed.
+                //
+                attrs.sortRequested(column);
+            }
             tableView.invalidate();
             sortByColumn(column);
             tableView.validate();
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/XMBeanAttributes.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/XMBeanAttributes.java	Fri Aug 08 14:24:31 2008 +0200
@@ -25,31 +25,49 @@
 
 package sun.tools.jconsole.inspector;
 
-import javax.swing.*;
-import javax.swing.event.*;
-import javax.swing.table.*;
-import javax.swing.tree.*;
-import java.awt.BorderLayout;
-import java.awt.Color;
-import java.awt.GridLayout;
-import java.awt.FlowLayout;
+
 import java.awt.Component;
 import java.awt.EventQueue;
-import java.awt.event.*;
-import java.awt.Insets;
 import java.awt.Dimension;
-import java.util.*;
-import java.io.*;
+import java.awt.event.MouseAdapter;
+import java.awt.event.MouseEvent;
+import java.io.IOException;
 
 import java.lang.reflect.Array;
 
-import javax.management.*;
+import java.util.EventObject;
+import java.util.HashMap;
+import java.util.WeakHashMap;
+
+import java.util.concurrent.ExecutionException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.management.JMException;
+import javax.management.MBeanInfo;
+import javax.management.MBeanAttributeInfo;
+import javax.management.AttributeList;
+import javax.management.Attribute;
 import javax.management.openmbean.CompositeData;
 import javax.management.openmbean.TabularData;
 
+import javax.swing.JComponent;
+import javax.swing.JOptionPane;
+import javax.swing.JTable;
+import javax.swing.JTextField;
+import javax.swing.SwingWorker;
+import javax.swing.event.ChangeEvent;
+import javax.swing.event.TableModelEvent;
+import javax.swing.event.TableModelListener;
+import javax.swing.table.DefaultTableCellRenderer;
+import javax.swing.table.DefaultTableModel;
+import javax.swing.table.TableCellEditor;
+import javax.swing.table.TableCellRenderer;
+import javax.swing.table.TableColumn;
+import javax.swing.table.TableColumnModel;
+import javax.swing.table.TableModel;
+
 import sun.tools.jconsole.Resources;
 import sun.tools.jconsole.MBeansTab;
-import sun.tools.jconsole.Plotter;
 import sun.tools.jconsole.JConsole;
 import sun.tools.jconsole.ProxyClient.SnapshotMBeanServerConnection;
 
@@ -61,12 +79,14 @@
   COMPULSORY to not call the JMX world in synchronized blocks */
 @SuppressWarnings("serial")
 public class XMBeanAttributes extends XTable {
+
+    final Logger LOGGER =
+            Logger.getLogger(XMBeanAttributes.class.getPackage().getName());
+
     private final static String[] columnNames =
     {Resources.getText("Name"),
      Resources.getText("Value")};
 
-    private boolean editable = true;
-
     private XMBean mbean;
     private MBeanInfo mbeanInfo;
     private MBeanAttributeInfo[] attributesInfo;
@@ -75,9 +95,8 @@
     private HashMap<String, Object> viewableAttributes;
     private WeakHashMap<XMBean, HashMap<String, ZoomedCell>> viewersCache =
             new WeakHashMap<XMBean, HashMap<String, ZoomedCell>>();
-    private TableModelListener attributesListener;
+    private final TableModelListener attributesListener;
     private MBeansTab mbeansTab;
-    private XTable table;
     private TableCellEditor valueCellEditor = new ValueCellEditor();
     private int rowMinHeight = -1;
     private AttributesMouseListener mouseListener = new AttributesMouseListener();
@@ -89,8 +108,8 @@
         super();
         this.mbeansTab = mbeansTab;
         ((DefaultTableModel)getModel()).setColumnIdentifiers(columnNames);
-        getModel().addTableModelListener(attributesListener =
-                                         new AttributesListener(this));
+        attributesListener = new AttributesListener(this);
+        getModel().addTableModelListener(attributesListener);
         getColumnModel().getColumn(NAME_COLUMN).setPreferredWidth(40);
 
         addMouseListener(mouseListener);
@@ -99,6 +118,7 @@
         addKeyListener(new Utils.CopyKeyAdapter());
     }
 
+    @Override
     public synchronized Component prepareRenderer(TableCellRenderer renderer,
                                                   int row, int column) {
         //In case we have a repaint thread that is in the process of
@@ -124,6 +144,7 @@
                 setRowHeight(row, rowMinHeight);
     }
 
+    @Override
     public synchronized TableCellRenderer getCellRenderer(int row,
             int column) {
         //In case we have a repaint thread that is in the process of
@@ -169,26 +190,40 @@
     }
 
     public void cancelCellEditing() {
-        TableCellEditor editor = getCellEditor();
-        if (editor != null) {
-            editor.cancelCellEditing();
+        if (LOGGER.isLoggable(Level.FINER)) {
+            LOGGER.finer("Cancel Editing Row: "+getEditingRow());
+        }
+        final TableCellEditor tableCellEditor = getCellEditor();
+        if (tableCellEditor != null) {
+            tableCellEditor.cancelCellEditing();
         }
     }
 
     public void stopCellEditing() {
-        TableCellEditor editor = getCellEditor();
-        if (editor != null) {
-            editor.stopCellEditing();
+        if (LOGGER.isLoggable(Level.FINER)) {
+            LOGGER.finer("Stop Editing Row: "+getEditingRow());
+        }
+        final TableCellEditor tableCellEditor = getCellEditor();
+        if (tableCellEditor != null) {
+            tableCellEditor.stopCellEditing();
         }
     }
 
-    public final boolean editCellAt(int row, int column, EventObject e) {
+    @Override
+    public final boolean editCellAt(final int row, final int column, EventObject e) {
+        if (LOGGER.isLoggable(Level.FINER)) {
+            LOGGER.finer("editCellAt(row="+row+", col="+column+
+                    ", e="+e+")");
+        }
+        if (JConsole.isDebug()) {
+            System.err.println("edit: "+getValueName(row)+"="+getValue(row));
+        }
         boolean retVal = super.editCellAt(row, column, e);
         if (retVal) {
-            TableCellEditor editor =
+            final TableCellEditor tableCellEditor =
                     getColumnModel().getColumn(column).getCellEditor();
-            if (editor == valueCellEditor) {
-                ((JComponent) editor).requestFocus();
+            if (tableCellEditor == valueCellEditor) {
+                ((JComponent) tableCellEditor).requestFocus();
             }
         }
         return retVal;
@@ -213,6 +248,10 @@
     public void setValueAt(Object value, int row, int column) {
         if (!isCellError(row, column) && isColumnEditable(column) &&
             isWritable(row) && Utils.isEditableType(getClassName(row))) {
+            if (JConsole.isDebug()) {
+                System.err.println("validating [row="+row+", column="+column+
+                        "]: "+getValueName(row)+"="+value);
+            }
             super.setValueAt(value, row, column);
         }
     }
@@ -256,12 +295,14 @@
         }
     }
 
-
     public Object getValue(int row) {
-        return ((DefaultTableModel) getModel()).getValueAt(row, VALUE_COLUMN);
+        final Object val = ((DefaultTableModel) getModel())
+                .getValueAt(row, VALUE_COLUMN);
+        return val;
     }
 
     //tool tip only for editable column
+    @Override
     public String getToolTip(int row, int column) {
         if (isCellError(row, column)) {
             return (String) unavailableAttributes.get(getValueName(row));
@@ -302,6 +343,7 @@
      * Override JTable method in order to make any call to this method
      * atomic with TableModel elements.
      */
+    @Override
     public synchronized int getRowCount() {
         return super.getRowCount();
     }
@@ -332,24 +374,67 @@
         return isViewable;
     }
 
-    public void loadAttributes(final XMBean mbean, MBeanInfo mbeanInfo) {
+    // Call this in EDT
+    public void loadAttributes(final XMBean mbean, final MBeanInfo mbeanInfo) {
+
+        final SwingWorker<Runnable,Void> load =
+                new SwingWorker<Runnable,Void>() {
+            @Override
+            protected Runnable doInBackground() throws Exception {
+                return doLoadAttributes(mbean,mbeanInfo);
+            }
+
+            @Override
+            protected void done() {
+                try {
+                    final Runnable updateUI = get();
+                    if (updateUI != null) updateUI.run();
+                } catch (RuntimeException x) {
+                    throw x;
+                } catch (ExecutionException x) {
+                    if(JConsole.isDebug()) {
+                       System.err.println(
+                               "Exception raised while loading attributes: "
+                               +x.getCause());
+                       x.printStackTrace();
+                    }
+                } catch (InterruptedException x) {
+                    if(JConsole.isDebug()) {
+                       System.err.println(
+                            "Interrupted while loading attributes: "+x);
+                       x.printStackTrace();
+                    }
+                }
+            }
+
+        };
+        mbeansTab.workerAdd(load);
+    }
+
+    // Don't call this in EDT, but execute returned Runnable inside
+    // EDT - typically in the done() method of a SwingWorker
+    // This method can return null.
+    private Runnable doLoadAttributes(final XMBean mbean, MBeanInfo infoOrNull)
+        throws JMException, IOException {
         // To avoid deadlock with events coming from the JMX side,
         // we retrieve all JMX stuff in a non synchronized block.
 
-        if(mbean == null) return;
+        if(mbean == null) return null;
+        final MBeanInfo curMBeanInfo =
+                (infoOrNull==null)?mbean.getMBeanInfo():infoOrNull;
 
-        final MBeanAttributeInfo[] attributesInfo = mbeanInfo.getAttributes();
-        final HashMap<String, Object> attributes =
-            new HashMap<String, Object>(attributesInfo.length);
-        final HashMap<String, Object> unavailableAttributes =
-            new HashMap<String, Object>(attributesInfo.length);
-        final HashMap<String, Object> viewableAttributes =
-            new HashMap<String, Object>(attributesInfo.length);
+        final MBeanAttributeInfo[] attrsInfo = curMBeanInfo.getAttributes();
+        final HashMap<String, Object> attrs =
+            new HashMap<String, Object>(attrsInfo.length);
+        final HashMap<String, Object> unavailableAttrs =
+            new HashMap<String, Object>(attrsInfo.length);
+        final HashMap<String, Object> viewableAttrs =
+            new HashMap<String, Object>(attrsInfo.length);
         AttributeList list = null;
 
         try {
-            list = mbean.getAttributes(attributesInfo);
-        } catch (Exception e) {
+            list = mbean.getAttributes(attrsInfo);
+        }catch(Exception e) {
             if (JConsole.isDebug()) {
                 System.err.println("Error calling getAttributes() on MBean \"" +
                                    mbean.getObjectName() + "\". JConsole will " +
@@ -359,18 +444,18 @@
             }
             list = new AttributeList();
             //Can't load all attributes, do it one after each other.
-            for(int i = 0; i < attributesInfo.length; i++) {
+            for(int i = 0; i < attrsInfo.length; i++) {
                 String name = null;
                 try {
-                    name = attributesInfo[i].getName();
+                    name = attrsInfo[i].getName();
                     Object value =
-                        mbean.getMBeanServerConnection().getAttribute(mbean.getObjectName(), name);
+                        mbean.getMBeanServerConnection().
+                        getAttribute(mbean.getObjectName(), name);
                     list.add(new Attribute(name, value));
                 }catch(Exception ex) {
-                    if(attributesInfo[i].isReadable()) {
-                        unavailableAttributes.put(name,
-                                                  Utils.getActualException(ex).
-                                                  toString());
+                    if(attrsInfo[i].isReadable()) {
+                        unavailableAttrs.put(name,
+                                Utils.getActualException(ex).toString());
                     }
                 }
             }
@@ -380,22 +465,22 @@
             for (int i=0;i<att_length;i++) {
                 Attribute attribute = (Attribute) list.get(i);
                 if(isViewable(attribute)) {
-                    viewableAttributes.put(attribute.getName(),
+                    viewableAttrs.put(attribute.getName(),
                                            attribute.getValue());
                 }
                 else
-                    attributes.put(attribute.getName(),attribute.getValue());
+                    attrs.put(attribute.getName(),attribute.getValue());
 
             }
             // if not all attributes are accessible,
             // check them one after the other.
-            if (att_length < attributesInfo.length) {
-                for (int i=0;i<attributesInfo.length;i++) {
-                    MBeanAttributeInfo attributeInfo = attributesInfo[i];
-                    if (!attributes.containsKey(attributeInfo.getName()) &&
-                        !viewableAttributes.containsKey(attributeInfo.
+            if (att_length < attrsInfo.length) {
+                for (int i=0;i<attrsInfo.length;i++) {
+                    MBeanAttributeInfo attributeInfo = attrsInfo[i];
+                    if (!attrs.containsKey(attributeInfo.getName()) &&
+                        !viewableAttrs.containsKey(attributeInfo.
                                                         getName()) &&
-                        !unavailableAttributes.containsKey(attributeInfo.
+                        !unavailableAttrs.containsKey(attributeInfo.
                                                            getName())) {
                         if (attributeInfo.isReadable()) {
                             // getAttributes didn't help resolving the
@@ -408,16 +493,13 @@
                                     mbean.getObjectName(), attributeInfo.getName());
                                 //What happens if now it is ok?
                                 // Be pragmatic, add it to readable...
-                                attributes.put(attributeInfo.getName(),
+                                attrs.put(attributeInfo.getName(),
                                                v);
                             }catch(Exception e) {
                                 //Put the exception that will be displayed
                                 // in tooltip
-                                unavailableAttributes.put(attributeInfo.
-                                                          getName(),
-                                                          Utils.
-                                                          getActualException(e)
-                                                          .toString());
+                                unavailableAttrs.put(attributeInfo.getName(),
+                                        Utils.getActualException(e).toString());
                             }
                         }
                     }
@@ -426,10 +508,10 @@
         }
         catch(Exception e) {
             //sets all attributes unavailable except the writable ones
-            for (int i=0;i<attributesInfo.length;i++) {
-                MBeanAttributeInfo attributeInfo = attributesInfo[i];
+            for (int i=0;i<attrsInfo.length;i++) {
+                MBeanAttributeInfo attributeInfo = attrsInfo[i];
                 if (attributeInfo.isReadable()) {
-                    unavailableAttributes.put(attributeInfo.getName(),
+                    unavailableAttrs.put(attributeInfo.getName(),
                                               Utils.getActualException(e).
                                               toString());
                 }
@@ -438,40 +520,36 @@
         //end of retrieval
 
         //one update at a time
-        synchronized(this) {
+        return new Runnable() {
+            public void run() {
+                synchronized (XMBeanAttributes.this) {
+                    XMBeanAttributes.this.mbean = mbean;
+                    XMBeanAttributes.this.mbeanInfo = curMBeanInfo;
+                    XMBeanAttributes.this.attributesInfo = attrsInfo;
+                    XMBeanAttributes.this.attributes = attrs;
+                    XMBeanAttributes.this.unavailableAttributes = unavailableAttrs;
+                    XMBeanAttributes.this.viewableAttributes = viewableAttrs;
 
-            this.mbean = mbean;
-            this.mbeanInfo = mbeanInfo;
-            this.attributesInfo = attributesInfo;
-            this.attributes = attributes;
-            this.unavailableAttributes = unavailableAttributes;
-            this.viewableAttributes = viewableAttributes;
-
-            EventQueue.invokeLater(new Runnable() {
-                public void run() {
                     DefaultTableModel tableModel =
                             (DefaultTableModel) getModel();
 
-                    // don't listen to these events
-                    tableModel.removeTableModelListener(attributesListener);
-
                     // add attribute information
-                    emptyTable();
+                    emptyTable(tableModel);
 
                     addTableData(tableModel,
-                                 mbean,
-                                 attributesInfo,
-                                 attributes,
-                                 unavailableAttributes,
-                                 viewableAttributes);
+                            mbean,
+                            attrsInfo,
+                            attrs,
+                            unavailableAttrs,
+                            viewableAttrs);
 
                     // update the model with the new data
                     tableModel.newDataAvailable(new TableModelEvent(tableModel));
                     // re-register for change events
                     tableModel.addTableModelListener(attributesListener);
                 }
-            });
-        }
+            }
+        };
     }
 
     void collapse(String attributeName, final Component c) {
@@ -534,22 +612,80 @@
         return cell;
     }
 
-     public void refreshAttributes() {
-         SnapshotMBeanServerConnection mbsc = mbeansTab.getSnapshotMBeanServerConnection();
-         mbsc.flush();
-         stopCellEditing();
-         loadAttributes(mbean, mbeanInfo);
+    // This is called by XSheet when the "refresh" button is pressed.
+    // In this case we will commit any pending attribute values by
+    // calling 'stopCellEditing'.
+    //
+    public void refreshAttributes() {
+         refreshAttributes(true);
+    }
+
+    // refreshAttributes(false) is called by tableChanged().
+    // in this case we must not call stopCellEditing, because it's already
+    // been called - e.g.
+    // lostFocus/mousePressed -> stopCellEditing -> setValueAt -> tableChanged
+    //                        -> refreshAttributes(false)
+    //
+    // Can be called in EDT - as long as the implementation of
+    // mbeansTab.getCachedMBeanServerConnection() and mbsc.flush() doesn't
+    // change
+    //
+    private void refreshAttributes(final boolean stopCellEditing) {
+         SwingWorker<Void,Void> sw = new SwingWorker<Void,Void>() {
+
+            @Override
+            protected Void doInBackground() throws Exception {
+                SnapshotMBeanServerConnection mbsc =
+                mbeansTab.getSnapshotMBeanServerConnection();
+                mbsc.flush();
+                return null;
+            }
+
+            @Override
+            protected void done() {
+                try {
+                    get();
+                    if (stopCellEditing) stopCellEditing();
+                    loadAttributes(mbean, mbeanInfo);
+                } catch (Exception x) {
+                    if (JConsole.isDebug()) {
+                        x.printStackTrace();
+                    }
+                }
+            }
+         };
+         mbeansTab.workerAdd(sw);
      }
+    // We need to call stop editing here - otherwise edits are lost
+    // when resizing the table.
+    //
+    @Override
+    public void columnMarginChanged(ChangeEvent e) {
+        if (isEditing()) stopCellEditing();
+        super.columnMarginChanged(e);
+    }
+
+    // We need to call stop editing here - otherwise the edited value
+    // is transferred to the wrong row...
+    //
+    @Override
+    void sortRequested(int column) {
+        if (isEditing()) stopCellEditing();
+        super.sortRequested(column);
+    }
 
 
-     public void emptyTable() {
-         synchronized(this) {
-             ((DefaultTableModel) getModel()).
-                 removeTableModelListener(attributesListener);
-             super.emptyTable();
-         }
+    @Override
+    public synchronized void emptyTable() {
+         emptyTable((DefaultTableModel)getModel());
      }
 
+    // Call this in synchronized block.
+    private void emptyTable(DefaultTableModel model) {
+         model.removeTableModelListener(attributesListener);
+         super.emptyTable();
+    }
+
     private boolean isViewable(Attribute attribute) {
         Object data = attribute.getValue();
         return XDataViewer.isViewableValue(data);
@@ -659,6 +795,7 @@
 
     class AttributesMouseListener extends MouseAdapter {
 
+        @Override
         public void mousePressed(MouseEvent e) {
             if(e.getButton() == MouseEvent.BUTTON1) {
                 if(e.getClickCount() >= 2) {
@@ -674,8 +811,10 @@
         }
     }
 
+    @SuppressWarnings("serial")
     class ValueCellEditor extends XTextFieldEditor {
         // implements javax.swing.table.TableCellEditor
+        @Override
         public Component getTableCellEditorComponent(JTable table,
                                                      Object value,
                                                      boolean isSelected,
@@ -727,6 +866,7 @@
         }
     }
 
+    @SuppressWarnings("serial")
     class MaximizedCellRenderer extends  DefaultTableCellRenderer {
         Component comp;
         MaximizedCellRenderer(Component comp) {
@@ -736,6 +876,7 @@
                 comp.setPreferredSize(new Dimension((int) d.getWidth(), 200));
             }
         }
+        @Override
         public Component getTableCellRendererComponent(JTable table,
                                                        Object value,
                                                        boolean isSelected,
@@ -818,6 +959,7 @@
             return minHeight;
         }
 
+        @Override
         public String toString() {
 
             if(value == null) return null;
@@ -854,45 +996,82 @@
             this.component = component;
         }
 
+        // Call this in EDT
         public void tableChanged(final TableModelEvent e) {
-            final TableModel model = (TableModel)e.getSource();
             // only post changes to the draggable column
             if (isColumnEditable(e.getColumn())) {
-                mbeansTab.workerAdd(new Runnable() {
-                        public void run() {
-                            try {
-                                Object tableValue =
-                                    model.getValueAt(e.getFirstRow(),
-                                                     e.getColumn());
-                                // if it's a String, try construct new value
-                                // using the defined type.
-                                if (tableValue instanceof String) {
-                                    tableValue =
-                                        Utils.createObjectFromString(getClassName(e.getFirstRow()), // type
-                                                                     (String)tableValue);// value
-                                }
-                                String attributeName =
-                                    getValueName(e.getFirstRow());
-                                Attribute attribute =
-                                    new Attribute(attributeName,tableValue);
-                                mbean.setAttribute(attribute);
-                            }
-                            catch (Throwable ex) {
-                                if (JConsole.isDebug()) {
-                                    ex.printStackTrace();
-                                }
-                                ex = Utils.getActualException(ex);
+                final TableModel model = (TableModel)e.getSource();
+                Object tableValue = model.getValueAt(e.getFirstRow(),
+                                                 e.getColumn());
+
+                if (LOGGER.isLoggable(Level.FINER)) {
+                    LOGGER.finer("tableChanged: firstRow="+e.getFirstRow()+
+                        ", lastRow="+e.getLastRow()+", column="+e.getColumn()+
+                        ", value="+tableValue);
+                }
+                // if it's a String, try construct new value
+                // using the defined type.
+                if (tableValue instanceof String) {
+                    try {
+                        tableValue =
+                            Utils.createObjectFromString(getClassName(e.getFirstRow()), // type
+                            (String)tableValue);// value
+                    } catch (Throwable ex) {
+                        popupAndLog(ex,"tableChanged",
+                                "Problem setting attribute");
+                    }
+                }
+                final String attributeName = getValueName(e.getFirstRow());
+                final Attribute attribute =
+                      new Attribute(attributeName,tableValue);
+                setAttribute(attribute, "tableChanged");
+            }
+        }
 
-                                String message = (ex.getMessage() != null) ? ex.getMessage() : ex.toString();
-                                EventQueue.invokeLater(new ThreadDialog(component,
-                                                                        message+"\n",
-                                                                        Resources.getText("Problem setting attribute"),
-                                                                        JOptionPane.ERROR_MESSAGE));
-                            }
-                            refreshAttributes();
+        // Call this in EDT
+        private void setAttribute(final Attribute attribute, final String method) {
+            final SwingWorker<Void,Void> setAttribute =
+                    new SwingWorker<Void,Void>() {
+                @Override
+                protected Void doInBackground() throws Exception {
+                    try {
+                        if (JConsole.isDebug()) {
+                            System.err.println("setAttribute("+
+                                    attribute.getName()+
+                                "="+attribute.getValue()+")");
                         }
-                    });
-            }
+                        mbean.setAttribute(attribute);
+                    } catch (Throwable ex) {
+                        popupAndLog(ex,method,"Problem setting attribute");
+                    }
+                    return null;
+                }
+                @Override
+                protected void done() {
+                    try {
+                        get();
+                    } catch (Exception x) {
+                        if (JConsole.isDebug())
+                            x.printStackTrace();
+                    }
+                    refreshAttributes(false);
+                }
+
+            };
+            mbeansTab.workerAdd(setAttribute);
+        }
+
+        // Call this outside EDT
+        private void popupAndLog(Throwable ex, String method, String key) {
+            ex = Utils.getActualException(ex);
+            if (JConsole.isDebug()) ex.printStackTrace();
+
+            String message = (ex.getMessage() != null) ? ex.getMessage()
+                    : ex.toString();
+            EventQueue.invokeLater(
+                    new ThreadDialog(component, message+"\n",
+                                     Resources.getText(key),
+                                     JOptionPane.ERROR_MESSAGE));
         }
     }
 }
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/XPlotter.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/XPlotter.java	Fri Aug 08 14:24:31 2008 +0200
@@ -37,6 +37,7 @@
         super(unit);
         this.table = table;
     }
+    @Override
     public void addValues(long time, long... values) {
         super.addValues(time, values);
         table.repaint();
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/XSheet.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/XSheet.java	Fri Aug 08 14:24:31 2008 +0200
@@ -25,18 +25,39 @@
 
 package sun.tools.jconsole.inspector;
 
-import java.awt.*;
-import java.awt.event.*;
-import java.io.*;
-import javax.management.*;
-import javax.swing.*;
-import javax.swing.border.*;
-import javax.swing.tree.*;
+
+import java.awt.BorderLayout;
+import java.awt.Color;
+import java.awt.Component;
+import java.awt.Dimension;
+import java.awt.event.ActionEvent;
+import java.awt.event.ActionListener;
+import java.io.IOException;
+
+import javax.management.IntrospectionException;
+import javax.management.NotificationListener;
+import javax.management.MBeanInfo;
+import javax.management.InstanceNotFoundException;
+import javax.management.ReflectionException;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanOperationInfo;
+import javax.management.MBeanNotificationInfo;
+import javax.management.Notification;
+import javax.swing.BorderFactory;
+import javax.swing.JButton;
+import javax.swing.JOptionPane;
+import javax.swing.JPanel;
+import javax.swing.JScrollPane;
+import javax.swing.JTextArea;
+import javax.swing.SwingWorker;
+import javax.swing.border.LineBorder;
+import javax.swing.tree.DefaultMutableTreeNode;
+import javax.swing.tree.DefaultTreeModel;
+
 import sun.tools.jconsole.*;
 import sun.tools.jconsole.inspector.XNodeInfo.Type;
 
 import static sun.tools.jconsole.Resources.*;
-import static sun.tools.jconsole.Utilities.*;
 
 @SuppressWarnings("serial")
 public class XSheet extends JPanel
@@ -344,34 +365,41 @@
             return;
         }
         mbean = (XMBean) uo.getData();
-        SwingWorker<Void, Void> sw = new SwingWorker<Void, Void>() {
+        final XMBean xmb = mbean;
+        SwingWorker<MBeanInfo,Void> sw = new SwingWorker<MBeanInfo,Void>() {
             @Override
-            public Void doInBackground() throws InstanceNotFoundException,
+            public MBeanInfo doInBackground() throws InstanceNotFoundException,
                     IntrospectionException, ReflectionException, IOException {
-                mbeanAttributes.loadAttributes(mbean, mbean.getMBeanInfo());
-                return null;
+                MBeanInfo mbi = xmb.getMBeanInfo();
+                return mbi;
             }
             @Override
             protected void done() {
                 try {
-                    get();
-                    if (!isSelectedNode(node, currentNode)) {
-                        return;
+                    MBeanInfo mbi = get();
+                    if (mbi != null && mbi.getAttributes() != null &&
+                            mbi.getAttributes().length > 0) {
+
+                        mbeanAttributes.loadAttributes(xmb, mbi);
+
+                        if (!isSelectedNode(node, currentNode)) {
+                            return;
+                        }
+                        invalidate();
+                        mainPanel.removeAll();
+                        JPanel borderPanel = new JPanel(new BorderLayout());
+                        borderPanel.setBorder(BorderFactory.createTitledBorder(
+                                Resources.getText("Attribute values")));
+                        borderPanel.add(new JScrollPane(mbeanAttributes));
+                        mainPanel.add(borderPanel, BorderLayout.CENTER);
+                        // add the refresh button to the south panel
+                        southPanel.removeAll();
+                        southPanel.add(refreshButton, BorderLayout.SOUTH);
+                        southPanel.setVisible(true);
+                        refreshButton.setEnabled(true);
+                        validate();
+                        repaint();
                     }
-                    invalidate();
-                    mainPanel.removeAll();
-                    JPanel borderPanel = new JPanel(new BorderLayout());
-                    borderPanel.setBorder(BorderFactory.createTitledBorder(
-                            Resources.getText("Attribute values")));
-                    borderPanel.add(new JScrollPane(mbeanAttributes));
-                    mainPanel.add(borderPanel, BorderLayout.CENTER);
-                    // add the refresh button to the south panel
-                    southPanel.removeAll();
-                    southPanel.add(refreshButton, BorderLayout.SOUTH);
-                    southPanel.setVisible(true);
-                    refreshButton.setEnabled(true);
-                    validate();
-                    repaint();
                 } catch (Exception e) {
                     Throwable t = Utils.getActualException(e);
                     if (JConsole.isDebug()) {
@@ -704,13 +732,7 @@
             JButton button = (JButton) e.getSource();
             // Refresh button
             if (button == refreshButton) {
-                new SwingWorker<Void, Void>() {
-                    @Override
-                    public Void doInBackground() {
-                        refreshAttributes();
-                        return null;
-                    }
-                }.execute();
+                refreshAttributes();
                 return;
             }
             // Clear button
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/XTable.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/XTable.java	Fri Aug 08 14:24:31 2008 +0200
@@ -25,10 +25,13 @@
 
 package sun.tools.jconsole.inspector;
 
-import javax.swing.*;
-import javax.swing.table.*;
-import java.awt.*;
-import java.io.*;
+import java.awt.Color;
+import java.awt.Component;
+import java.awt.Font;
+import javax.swing.JTable;
+import javax.swing.table.DefaultTableCellRenderer;
+import javax.swing.table.DefaultTableModel;
+import javax.swing.table.TableCellRenderer;
 
 public abstract class XTable extends JTable {
     static final int NAME_COLUMN = 0;
@@ -38,8 +41,9 @@
 
     public XTable () {
         super();
-        TableSorter sorter;
-        setModel(sorter = new TableSorter());
+        @SuppressWarnings("serial")
+        final TableSorter sorter = new TableSorter();
+        setModel(sorter);
         sorter.addMouseListenerToHeaderInTable(this);
         setRowSelectionAllowed(false);
         setColumnSelectionAllowed(false);
@@ -55,6 +59,14 @@
     }
 
     /**
+     * Called by TableSorter if a mouse event requests to sort the rows.
+     * @param column the column against which the rows are sorted
+     */
+    void sortRequested(int column) {
+        // This is a hook for subclasses
+    }
+
+    /**
      * This returns the select index as the table was at initialization
      */
     public int getSelectedIndex() {
@@ -67,7 +79,7 @@
     public int convertRowToIndex(int row) {
         if (row == -1) return row;
         if (getModel() instanceof TableSorter) {
-            return (((TableSorter) getModel()).getInvertedIndex()[row]);
+            return ((TableSorter) getModel()).getIndexOfRow(row);
         } else {
             return row;
         }
@@ -97,6 +109,7 @@
     //JTable re-implementation
 
     //attribute can be editable even if unavailable
+    @Override
     public boolean isCellEditable(int row, int col) {
         return ((isTableEditable() && isColumnEditable(col)
                  &&  isWritable(row)
@@ -118,6 +131,7 @@
      * This method sets read write rows to be blue, and other rows to be their
      * default rendered colour.
      */
+    @Override
     public TableCellRenderer getCellRenderer(int row, int column) {
         DefaultTableCellRenderer tcr =
             (DefaultTableCellRenderer) super.getCellRenderer(row,column);
@@ -146,6 +160,7 @@
         return tcr;
     }
 
+    @Override
     public Component prepareRenderer(TableCellRenderer renderer,
                                      int row, int column) {
         Component comp = super.prepareRenderer(renderer, row, column);
--- a/jdk/src/share/classes/sun/tools/jconsole/inspector/XTextFieldEditor.java	Thu Aug 07 16:25:45 2008 +0200
+++ b/jdk/src/share/classes/sun/tools/jconsole/inspector/XTextFieldEditor.java	Fri Aug 08 14:24:31 2008 +0200
@@ -26,22 +26,30 @@
 package sun.tools.jconsole.inspector;
 
 import java.awt.Component;
+import java.awt.event.ActionEvent;
+import java.awt.event.FocusAdapter;
+import java.awt.event.FocusEvent;
+import java.awt.event.FocusListener;
 import java.util.EventObject;
-import java.awt.event.*;
-import java.awt.dnd.DragSourceDropEvent;
-import javax.swing.*;
-import javax.swing.event.*;
-import javax.swing.table.*;
+import javax.swing.JMenuItem;
+import javax.swing.JTable;
+import javax.swing.JTextField;
+import javax.swing.event.CellEditorListener;
+import javax.swing.event.ChangeEvent;
+import javax.swing.event.EventListenerList;
+import javax.swing.table.TableCellEditor;
 
 @SuppressWarnings("serial")
 public class XTextFieldEditor extends XTextField implements TableCellEditor {
 
-    protected EventListenerList listenerList = new EventListenerList();
+    protected EventListenerList evtListenerList = new EventListenerList();
     protected ChangeEvent changeEvent = new ChangeEvent(this);
 
     private FocusListener editorFocusListener = new FocusAdapter() {
+        @Override
         public void focusLost(FocusEvent e) {
-            fireEditingStopped();
+            // fireEditingStopped();
+            // must not call fireEditingStopped() here!
         }
     };
 
@@ -51,6 +59,7 @@
     }
 
     //edition stopped ou JMenuItem selection & JTextField selection
+    @Override
     public void  actionPerformed(ActionEvent e) {
         super.actionPerformed(e);
         if ((e.getSource() instanceof JMenuItem) ||
@@ -67,16 +76,16 @@
     //TableCellEditor implementation
 
     public void addCellEditorListener(CellEditorListener listener) {
-        listenerList.add(CellEditorListener.class,listener);
+        evtListenerList.add(CellEditorListener.class,listener);
     }
 
     public void removeCellEditorListener(CellEditorListener listener) {
-        listenerList.remove(CellEditorListener.class, listener);
+        evtListenerList.remove(CellEditorListener.class, listener);
     }
 
     protected void fireEditingStopped() {
         CellEditorListener listener;
-        Object[] listeners = listenerList.getListenerList();
+        Object[] listeners = evtListenerList.getListenerList();
         for (int i=0;i< listeners.length;i++) {
             if (listeners[i] == CellEditorListener.class) {
                 listener = (CellEditorListener) listeners[i+1];
@@ -87,7 +96,7 @@
 
     protected void fireEditingCanceled() {
         CellEditorListener listener;
-        Object[] listeners = listenerList.getListenerList();
+        Object[] listeners = evtListenerList.getListenerList();
         for (int i=0;i< listeners.length;i++) {
             if (listeners[i] == CellEditorListener.class) {
                 listener = (CellEditorListener) listeners[i+1];