admin@glassfish.java.net

Re: code review: LBDeregistrationUtil thread-safety fix

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Thu, 15 Feb 2007 16:38:05 -0800

What's this?!?
+ //_at_GuardedBy("this") JDK 1.6


Shouldn't these messages be in a properties file?

+ throw new IllegalStateException( "can't add same listener
twice or change it" );
+ }




Lloyd L Chambers wrote:
> 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);
> }
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>