admin@glassfish.java.net

CODE REVIEW: thread-safety fixes for AMXProxyHandler

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Mon, 30 Apr 2007 20:44:59 -0700

REVIEW TIMEOUT: COB Monday April 29

Although I did not find any correctness issues in terms of behavior
that could go *wrong*, the changes below make the AMXProxyHandler
correct in terms of thread safety.

Also added (incidentally) J2EEUtil.getStateManageableString().

------------

Index: appserv-api/src/java/com/sun/appserv/management/client/handler/
AMXProxyHandler.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/
management/client/handler/AMXProxyHandler.java,v
retrieving revision 1.1
diff -r1.1 AMXProxyHandler.java
77a78
> <p><b>THREAD SAFE</b>
86c87
< "com.sun.appserv.management.util.jmx.AMXProxyHandler";
---
 >            
"com.sun.appserv.management.client.handler.AMXProxyHandler";
746c747
<               private MBeanInfo
---
 >               private synchronized MBeanInfo
752c753
<
---
 >
Index: appserv-api/src/java/com/sun/appserv/management/client/handler/ 
ConverterHandler.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/ 
management/client/handler/ConverterHandler.java,v
retrieving revision 1.1
diff -r1.1 ConverterHandler.java
37a38
 >     <p><b>THREAD SAFE (by itself)</b>
Index: appserv-api/src/java/com/sun/appserv/management/util/j2ee/ 
J2EEUtil.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/ 
management/util/j2ee/J2EEUtil.java,v
retrieving revision 1.1
diff -r1.1 J2EEUtil.java
51a52,53
 > import com.sun.appserv.management.j2ee.StateManageable;
 >
414a417,450
 >     /**
 >         @return String equivalent of a {_at_link StateManageable} state
 >      */
 >         public static String
 >     getStateManageableString( final int state )
 >     {
 >         String s = null;
 >
 >         if ( state == StateManageable.STATE_STARTING )
 >         {
 >             s   = "STATE_STARTING";
 >         }
 >         else if ( state == StateManageable.STATE_RUNNING )
 >         {
 >             s   = "STATE_RUNNING";
 >         }
 >         else if ( state == StateManageable.STATE_STOPPING )
 >         {
 >             s   = "STATE_STOPPING";
 >         }
 >         else if ( state == StateManageable.STATE_STOPPED )
 >         {
 >             s   = "STATE_STOPPED";
 >         }
 >         else if ( state == StateManageable.STATE_FAILED )
 >         {
 >             s   = "STATE_FAILED";
 >         }
 >         else
 >         {
 >             throw new IllegalArgumentException( "" + state );
 >         }
 >         return s;
 >     }
cvs diff: Diffing appserv-api/src/java/com/sun/appserv/management/ 
util/j2ee/stringifier
cvs diff: Diffing appserv-api/src/java/com/sun/appserv/management/ 
util/jmx
Index: appserv-api/src/java/com/sun/appserv/management/util/jmx/ 
MBeanProxyHandler.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/ 
management/util/jmx/MBeanProxyHandler.java,v
retrieving revision 1.1
diff -r1.1 MBeanProxyHandler.java
72a73,75
 >     <p>
 >     <b>THREAD SAFE, but not clear if parent class
 >         javax.management.MBeanServerInvocationHandler is thread- 
safe</b>
84,85c87,88
<       protected AttributeNameMangler          mMangler;
<       private AttributeNameMapper                     mMapper;
---
 >       protected final AttributeNameMangler            mMangler;
 >       private volatile AttributeNameMapper            mMapper;
87,91c90,94
<       private boolean                                          
mCacheMBeanInfo;
<       private MBeanInfo                                        
mCachedMBeanInfo;
<       private boolean                                          
mMBeanInfoIsInvariant   = false;
<       private Logger                                          mLogger;
<       private boolean                                          
mTargetValid;
---
 >       private volatile boolean                         
mCacheMBeanInfo;
 >       private volatile MBeanInfo                       
mCachedMBeanInfo;
 >       private volatile boolean                         
mMBeanInfoIsInvariant   = false;
 >       private Logger                      mLogger;    // protected  
with 'synchronized'
 >       private volatile boolean                        mTargetValid;
94c97
<       protected Output                    mDebug;
---
 >       protected volatile Output              mDebug;
260c263
<               public void
---
 >               public synchronized void
268c271
<               public Logger
---
 >               public synchronized Logger
332c335,341
<                       mMapper = createMapper 
( mbeanInfo.getAttributes(), mMangler);
---
 >             synchronized( this )
 >             {
 >                 if ( mMapper == null )
 >                 {
 >                     mMapper   = createMapper 
( mbeanInfo.getAttributes(), mMangler);
 >                 }
 >             }
363c372
<               protected void
---
 >               protected synchronized void
405c414,417
<                       mCachedMBeanInfo        = getConnection 
().getMBeanInfo( getTargetObjectName() );
---
 >             synchronized( this )
 >             {
 >                 mCachedMBeanInfo = getConnection().getMBeanInfo 
( getTargetObjectName() );
 >             }