atom feed1 message in net.java.dev.glassfish.adminCODE REVIEW: FindBugs and more: Gener...
FromSent OnAttachments
Lloyd L ChambersMar 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[]{};