| From | Sent On | Attachments |
|---|---|---|
| Lloyd L Chambers | Mar 27, 2007 3:34 pm |
| Subject: | CODE REVIEW: FindBugs and more: GeneratedMonitoringMBeanImpl | |
|---|---|---|
| From: | Lloyd L Chambers (Lloy...@Sun.COM) | |
| Date: | Mar 27, 2007 3:34:56 pm | |
| List: | net.java.dev.glassfish.admin | |
Please review no later than noon on Thursday 3/30.
Fixed an "impossible cast" warning as well as a thread safety issue with 'mbeanInfo'.
Lloyd
RCS file: /cvs/glassfish/admin/monitor/src/java/com/sun/enterprise/ admin/monitor/registry/spi/GeneratedMonitoringMBeanImpl.java,v retrieving revision 1.3 diff -w -u -r1.3 GeneratedMonitoringMBeanImpl.java --- admin/monitor/src/java/com/sun/enterprise/admin/monitor/registry/ spi/GeneratedMonitoringMBeanImpl.java 25 Dec 2005 03:43:33 -0000 1.3 +++ admin/monitor/src/java/com/sun/enterprise/admin/monitor/registry/ spi/GeneratedMonitoringMBeanImpl.java 27 Mar 2007 17:46:54 -0000 @@ -52,11 +52,11 @@ * @author <a href=mailto:shre...@sun.com>Shreedhar Ganapathy</a> */ public class GeneratedMonitoringMBeanImpl implements DynamicMBean { - MBeanInfo mbeanInfo; - Stats resourceInstance; + volatile MBeanInfo mbeanInfo; + final Stats resourceInstance; public static final String LOGGER_NAME="this.is.console"; final Logger logger; - Hashtable attributeMap; + final Hashtable<String,String> attributeMap; //StatsHolder statsHolder = null; /** * constructs a monitoring mbean that manages an underlying Stats resource @@ -64,7 +64,7 @@ public GeneratedMonitoringMBeanImpl(Stats stats) { logger = Logger.getLogger(LOGGER_NAME); this.resourceInstance=stats; - attributeMap=new Hashtable(); + attributeMap=new Hashtable<String,String>(); }
/** @@ -74,10 +74,22 @@ * @param javax.management.j2ee.statistics.Stats * @param javax.management.MBeanInfo */ - MBeanInfo introspect(){ + final MBeanInfo introspect(){ + // !!! 'mbeanInfo' must be 'volatile' !!! + // this is thread-safe, not the double-null-check idiom so long as + // 'mbeanInfo' is 'volatile' + if ( mbeanInfo != null ) { + return mbeanInfo; + } + + synchronized( this ) { + if ( mbeanInfo == null ) { ManagedResourceIntrospector mri = new ManagedResourceIntrospector(this); mbeanInfo = mri.introspect(this.resourceInstance); setUpAttributeMap(); + } + } + return mbeanInfo; } /** @@ -115,9 +127,9 @@ if(str == null){ throw new NullPointerException("An attribute needs to be specified to get value"); } - if(mbeanInfo == null){ + introspect(); - } + if(!isValidAttribute(str)){ throw new AttributeNotFoundException("The requested attribute is not recognized"); } @@ -167,8 +179,8 @@ * @return javax.management.AttributeList */ public javax.management.AttributeList getAttributes(String[] str) { - if(mbeanInfo == null) introspect(); + AttributeList list = new AttributeList(); try{ for(int i=0; i<str.length;i++){ @@ -188,15 +200,12 @@ * @return javax.management.MBeanInfo */ public javax.management.MBeanInfo getMBeanInfo() { - if(mbeanInfo== null){ introspect(); - } return mbeanInfo; }
public Object invoke(String str, Object[] obj, String[] str2) throws javax.management.MBeanException, javax.management.ReflectionException{ - if(mbeanInfo == null) introspect(); Object a =null; Class[] c = new Class[]{};





