admin@glassfish.java.net

code review: LBDeregistrationUtil thread-safety fix

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Thu, 15 Feb 2007 16:25:12 -0800

https://glassfish.dev.java.net/issues/show_bug.cgi?id=2137


Index: src/java/com/sun/enterprise/management/support/
LBDeregistrationUtil.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/LBDeregistrationUtil.java,v
retrieving revision 1.6
diff -u -w -1 -0 -r1.6 LBDeregistrationUtil.java
--- src/java/com/sun/enterprise/management/support/
LBDeregistrationUtil.java 17 Mar 2006 03:34:20 -0000 1.6
+++ src/java/com/sun/enterprise/management/support/
LBDeregistrationUtil.java 16 Feb 2007 00:22:24 -0000
@@ -15,81 +15,93 @@
   * at glassfish/bootstrap/legal/CDDLv1.0.txt.
   * If applicable, add the following below the CDDL Header,
   * with the fields enclosed by brackets [] replaced by
   * you own identifying information:
   * "Portions Copyrighted [year] [name of copyright owner]"
   *
   * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
   */
package com.sun.enterprise.management.support;
-import java.util.Hashtable;
+import java.util.HashMap;
import java.util.Map;
+import java.util.Collections;
+
import javax.management.JMException;
import javax.management.MBeanServer;
import javax.management.ObjectName;
-import javax.management.Notification;
import javax.management.NotificationListener;
-import javax.management.AttributeChangeNotification;
import static com.sun.appserv.management.base.AMX.J2EE_TYPE_KEY;
import static com.sun.appserv.management.base.AMX.NAME_KEY;
import static com.sun.appserv.management.base.AMX.JMX_DOMAIN;
import static com.sun.appserv.management.base.XTypes.LOAD_BALANCER;
-import static
com.sun.appserv.management.base.XTypes.LOAD_BALANCER_CONFIG;
import static
com.sun.appserv.management.base.XTypes.LOAD_BALANCER_MONITOR;
import static com.sun.appserv.management.base.XTypes.LB_CONFIG;
-import com.sun.enterprise.management.support.AMXServerLogger;
-import com.sun.enterprise.management.support.AMXMBeanRootLogger;
-
import com.sun.appserv.management.config.LoadBalancerConfig;
import com.sun.appserv.management.config.LBConfig;
import com.sun.appserv.management.client.ProxyFactory;
import com.sun.appserv.management.monitor.LoadBalancerMonitor;
import com.sun.appserv.management.base.Util;
/**
- * Utilities to help the DeregisterationHelpers that are
+ * Utilities to help the DeregistrationHelpers that are
   * invoked in postDeRegister() methods of the config MBeans
   */
public final class LBDeregistrationUtil {
+ private final MBeanServer mbs;
+ private final Map<String, NotificationListener>
mLBConfigListenersMap;
- private MBeanServer mbs = null;
- private Map<String, NotificationListener> mLBConfigListenersMap
= null;
- private static LBDeregistrationUtil mUtil = null;
+ //_at_GuardedBy("LBDeregistrationUtil.class") JDK 1.6
+ private static LBDeregistrationUtil _Instance;

      private LBDeregistrationUtil(MBeanServer mbs) {
- mLBConfigListenersMap = new Hashtable<String,
NotificationListener>();
          this.mbs=mbs;
+ mLBConfigListenersMap =
+ Collections.synchronizedMap(new HashMap<String,
NotificationListener>());
      }

- public static LBDeregistrationUtil getInstance(MBeanServer mbs) {
- if (mUtil == null) mUtil = new LBDeregistrationUtil(mbs);
- return mUtil;
+ public static synchronized LBDeregistrationUtil
+ getInstance( final MBeanServer mbs) {
+ if ( _Instance == null ) {
+ _Instance = new LBDeregistrationUtil(mbs);
+ }
+ else if ( _Instance.mbs != mbs ) {
+ throw new IllegalStateException( "wrong MBeanServer" );
+ }
+ return _Instance;
      }

      public LoadBalancerMonitor fetchLBMonitoringRoot(
- String loadBalancerName) throws JMException {
+ final String loadBalancerName) throws JMException {
- ObjectName loadBalancerMonitorObjName =
+ final ObjectName loadBalancerMonitorObjName =
              new ObjectName(JMX_DOMAIN + ":" + J2EE_TYPE_KEY + "=" +
                  LOAD_BALANCER_MONITOR + "," + NAME_KEY + "=" +
loadBalancerName);
- LoadBalancerMonitor loadBalancerMonitor =
+ final LoadBalancerMonitor loadBalancerMonitor =
              ProxyFactory.getInstance(mbs).getProxy
(loadBalancerMonitorObjName, LoadBalancerMonitor.class);
          return loadBalancerMonitor;
      }

- public void addLBConfigListener(String loadBalancerName,
- NotificationListener nl, LBConfig lbConfig) {
+ /**
+ 'synchronized' to avoid putting the same listener twice.
+ */
+ //_at_GuardedBy("this") JDK 1.6
+ public synchronized void addLBConfigListener(final String
loadBalancerName,
+ final NotificationListener nl, final LBConfig lbConfig) {
+ if ( mLBConfigListenersMap.containsKey(loadBalancerName) ) {
+ throw new IllegalStateException( "can't add same
listener twice or change it" );
+ }
+
          lbConfig.addNotificationListener(nl, null, null);
          mLBConfigListenersMap.put(loadBalancerName, nl);
      }
- public void removeLBConfigListener(String loadBalancerName,
LBConfig lbConfig)
+ public void removeLBConfigListener(final String
loadBalancerName, final LBConfig lbConfig)
          throws JMException {
          lbConfig.removeNotificationListener(
              (NotificationListener)mLBConfigListenersMap.get
(loadBalancerName));
          mLBConfigListenersMap.remove(loadBalancerName);
      }
}