Please review by: 7pm March 12 2007
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1961
This change is required to make the base class thread safe. The
problem was "unsafe publication" (see Java Concurrency in Practice).
The problem was that NotificationListenerBase would register itself
as a listener prior to exiting its constructor.
The change touches 11 files, but the logic is not affected; only the
constructor signature and newInstance() methods (see below).
I was unable to find a solution to making the class thread safe which
could be safely made without risking incorrect behavior; clients
instantiating a NotificationListenerBase or any of its subclasses
must call startListening() *explicitly* now.
To prevent code from compiling which would appear to be OK, but not
function corrrectly at runtime:
- The signature of the constructor for NotificationListenerBase has
changed to include a "name" parameter. Named listeners are useful in
general (no names before), but the inclusion of 'name' also serves
the purpose of requring a different signature for the constructor, to
ensure that dependent code is found and fixed. All subclasses have
been modified accordingly.
- To forestall incorrect usage as much as possible, constructors of
NotificationListenerBase, MBeanRegistrationListener and all
subclasses are now 'private', with corresponding public static
newInstance() methods, which make the necessary call to startListening
().
Lloyd
cvs server: Diffing appserv-api/src/java/com/sun/appserv/management/
util/jmx
Index: appserv-api/src/java/com/sun/appserv/management/util/jmx/
MBeanRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/
management/util/jmx/MBeanRegistrationListener.java,v
retrieving revision 1.1
diff -u -w -r1.1 MBeanRegistrationListener.java
--- appserv-api/src/java/com/sun/appserv/management/util/jmx/
MBeanRegistrationListener.java 2 Dec 2006 06:04:06 -0000 1.1
+++ appserv-api/src/java/com/sun/appserv/management/util/jmx/
MBeanRegistrationListener.java 12 Mar 2007 20:38:55 -0000
@@ -59,14 +59,14 @@
@param conn
@param constrain optional fixed or pattern ObjectName
*/
- public
+ protected
MBeanRegistrationListener(
+ final String name,
final MBeanServerConnection conn,
final ObjectName constrain )
throws InstanceNotFoundException, IOException
{
- super( conn,
- JMXUtil.getMBeanServerDelegateObjectName() );
+ super( name, conn,
JMXUtil.getMBeanServerDelegateObjectName() );
mRegUnregFilter = constrain;
mDefaultDomain = conn.getDefaultDomain();
@@ -76,11 +76,13 @@
Calls this( conn, null ).
@param conn
*/
- public
- MBeanRegistrationListener( final MBeanServerConnection conn)
+ protected
+ MBeanRegistrationListener(
+ final String name,
+ final MBeanServerConnection conn)
throws InstanceNotFoundException, IOException
{
- this( conn, (ObjectName)null );
+ this( name, conn, (ObjectName)null );
}
protected abstract void mbeanRegistered( final ObjectName
objectName );
Index: appserv-api/src/java/com/sun/appserv/management/util/jmx/
NotificationListenerBase.java
===================================================================
RCS file: /cvs/glassfish/appserv-api/src/java/com/sun/appserv/
management/util/jmx/NotificationListenerBase.java,v
retrieving revision 1.1
diff -u -w -r1.1 NotificationListenerBase.java
--- appserv-api/src/java/com/sun/appserv/management/util/jmx/
NotificationListenerBase.java 2 Dec 2006 06:04:14 -0000 1.1
+++ appserv-api/src/java/com/sun/appserv/management/util/jmx/
NotificationListenerBase.java 12 Mar 2007 20:38:55 -0000
@@ -54,6 +54,7 @@
public abstract class NotificationListenerBase
implements NotificationListener
{
+ private final String mName;
private final MBeanServerConnection mConn;
/** actual MBean ObjectNames, not patterns */
@@ -66,37 +67,47 @@
private RegistrationListener mDelegateListener;
+ private volatile boolean mSetupListening;
+
/**
Calls this( conn, listenTo, null, null ).
+ <p><b>Instantiating code must call setupListening() in order
to initiate
+ listening</b>
*/
- public
+ protected
NotificationListenerBase(
+ final String name,
final MBeanServerConnection conn,
final ObjectName pattern )
throws InstanceNotFoundException, IOException
{
- this( conn, pattern, null );
+ this( name, conn, pattern, null );
}
/**
- Listen to all MBean(s) which match the pattern
- 'listenTo'
+ Listen to all MBean(s) which match the pattern 'listenTo'.
+ <p><b>Instantiating code must call setupListening() in order
to initiate
+ listening</b>
+ @param listenerName arbitrary name of this listener
@param conn the MBeanServerConnection or MBeanServer
@param pattern an MBean ObjectName, or an ObjectName
pattern
@param filter optional NotificationFilter
*/
- public
+ protected
NotificationListenerBase(
+ final String name,
final MBeanServerConnection conn,
final ObjectName pattern,
final NotificationFilter filter )
throws InstanceNotFoundException, IOException
{
+ mName = name;
mConn = conn;
mPattern = pattern;
mFilter = filter;
mHandback = null;
mDelegateListener = null;
+ mSetupListening = false;
mListenees = Collections.synchronizedSet
( new HashSet<ObjectName>() );
@@ -105,8 +116,6 @@
{
throw new IllegalArgumentException();
}
-
- setupListening( );
}
/**
@@ -128,10 +137,15 @@
}
}
- private void
- setupListening()
+ public synchronized void
+ startListening()
throws InstanceNotFoundException, IOException
{
+ if ( mSetupListening )
+ {
+ throw new IllegalStateException( "setupListening() must
be called exactly once" );
+ }
+
if ( mPattern.isPattern() )
{
// it's crucial we listen for registration/
unregistration events
@@ -162,6 +176,8 @@
listenToMBean( objectName );
}
}
+
+ mSetupListening = true;
}
/**
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
DomainRootImpl.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/DomainRootImpl.java,v
retrieving revision 1.20
diff -u -w -r1.20 DomainRootImpl.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
DomainRootImpl.java 25 Dec 2005 03:38:51 -0000 1.20
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
DomainRootImpl.java 12 Mar 2007 20:38:55 -0000
@@ -42,7 +42,7 @@
try
{
- new LBBootstrapUtil(getObjectName(), mServer);
+ LBBootstrapUtil.newInstance( "DomainRootImpl",
getObjectName(), mServer);
}
catch (Exception ex)
{
cvs server: Diffing admin/mbeanapi-impl/src/java/com/sun/enterprise/
management/config
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
config/ConfigFactory.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/config/ConfigFactory.java,v
retrieving revision 1.16
diff -u -w -r1.16 ConfigFactory.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/config/
ConfigFactory.java 10 Jan 2007 04:22:37 -0000 1.16
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/config/
ConfigFactory.java 12 Mar 2007 20:38:55 -0000
@@ -198,7 +198,11 @@
WaitForUnregistrationListener listener = null;
try
{
- listener = new WaitForUnregistrationListener
( objectName );
+ final String name =
+ "WaitForUnregistrationListener for " +
JMXUtil.toString(objectName);
+
+ listener = new WaitForUnregistrationListener( name,
objectName );
+ listener.startListening();
return listener;
}
catch( Exception e )
@@ -215,10 +219,12 @@
private final CountDownLatch mLatch;
private final boolean mWasUnregistered = false;
- private WaitForUnregistrationListener( final ObjectName
objectName )
+ private WaitForUnregistrationListener(
+ final String name,
+ final ObjectName objectName )
throws InstanceNotFoundException, java.io.IOException
{
- super( getMBeanServer(), null );
+ super( name, getMBeanServer(), null );
mTarget = objectName;
mLatch = new CountDownLatch(1);
mStart = System.currentTimeMillis();
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBaseMBeanRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/LBBaseMBeanRegistrationListener.java,v
retrieving revision 1.6
diff -u -w -r1.6 LBBaseMBeanRegistrationListener.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBaseMBeanRegistrationListener.java 17 Mar 2006 03:34:20
-0000 1.6
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBaseMBeanRegistrationListener.java 12 Mar 2007 20:38:55
-0000
@@ -90,22 +90,17 @@
* Convenience base class for performing registrations
* for various load balancer related monitors.
*/
-public abstract class LBBaseMBeanRegistrationListener
+abstract class LBBaseMBeanRegistrationListener
extends MBeanRegistrationListener {
final ObjectNames objectNames = ObjectNames.getInstance
(JMX_DOMAIN);
- public LBBaseMBeanRegistrationListener(
- final MBeanServer mBeanServer, final ObjectName constraint )
+ protected LBBaseMBeanRegistrationListener(
+ final String name,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer, constraint);
- }
-
- public LBBaseMBeanRegistrationListener(final MBeanServer
mBeanServer)
- throws InstanceNotFoundException, IOException {
-
- this(mBeanServer, (ObjectName)null );
+ super( name, mBeanServer);
}
public void mbeanUnregistered(final ObjectName objectName) {}
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBootstrapUtil.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/LBBootstrapUtil.java,v
retrieving revision 1.3
diff -u -w -r1.3 LBBootstrapUtil.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBootstrapUtil.java 25 Dec 2005 03:40:42 -0000 1.3
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LBBootstrapUtil.java 12 Mar 2007 20:38:55 -0000
@@ -81,11 +81,28 @@
LoadBalancerRegistrationListener lbrl = null;
final private ObjectNames objectNames = ObjectNames.getInstance
(JMX_DOMAIN);
- public LBBootstrapUtil(ObjectName domainRoot, MBeanServer
mBeanServer)
+ private LBBootstrapUtil(
+ final String name,
+ final ObjectName domainRoot,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer, domainRoot, null);
+ super( name, mBeanServer, domainRoot, null );
}
+ public static LBBootstrapUtil
+ newInstance(
+ final String name,
+ final ObjectName domainRoot,
+ final MBeanServer server ) throws
InstanceNotFoundException, IOException
+ {
+ final LBBootstrapUtil listener =
+ new LBBootstrapUtil( name, domainRoot, server );
+
+ listener.startListening();
+ return listener;
+ }
+
+
public void handleNotification(
final Notification notifIn, final Object handback) {
try {
@@ -103,7 +120,8 @@
//element registrations. I store the handle of the root LB
//listener primarily to reuse its ability to enable /
disable monitoring
- lbrl = new LoadBalancerRegistrationListener((MBeanServer)
getConn());
+ lbrl = LoadBalancerRegistrationListener.newInstance(
+ "LBBootstrapUtil root LB listener", (MBeanServer)
getConn());
initRestOfLBMBeanRegistrationListeners();
DomainConfig domainConfig = ProxyFactory.getInstance
(getConn())
@@ -146,9 +164,12 @@
MBeanServer mbeanServer = (MBeanServer)getConn();
try {
- new LoadBalancerClusterRefRegistrationListener
(mbeanServer);
- new LoadBalancerServerRefRegistrationListener(mbeanServer);
- new LoadBalancerApplicationRefRegistrationListener
(mbeanServer);
+ LoadBalancerClusterRefRegistrationListener.newInstance(
+ "LoadBalancerClusterRefRegistrationListener global
listener", mbeanServer);
+ LoadBalancerServerRefRegistrationListener.newInstance(
+ "LoadBalancerServerRefRegistrationListener global
listener", mbeanServer);
+ LoadBalancerApplicationRefRegistrationListener.newInstance(
+ "LoadBalancerApplicationRefRegistrationListener
global listener", mbeanServer);
} catch(Exception e) {
throw new RuntimeException(e);
}
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerApplicationRefRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/
LoadBalancerApplicationRefRegistrationListener.java,v
retrieving revision 1.5
diff -u -w -r1.5 LoadBalancerApplicationRefRegistrationListener.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerApplicationRefRegistrationListener.java 17
Mar 2006 03:34:20 -0000 1.5
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerApplicationRefRegistrationListener.java 12
Mar 2007 20:38:55 -0000
@@ -89,9 +89,23 @@
public class LoadBalancerApplicationRefRegistrationListener
extends LBBaseMBeanRegistrationListener {
- public LoadBalancerApplicationRefRegistrationListener
(MBeanServer mBeanServer)
+ private LoadBalancerApplicationRefRegistrationListener(
+ final String name,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer);
+ super( name, mBeanServer);
+ }
+
+ public static LoadBalancerApplicationRefRegistrationListener
+ newInstance(
+ final String name,
+ final MBeanServer conn ) throws InstanceNotFoundException,
IOException
+ {
+ final LoadBalancerApplicationRefRegistrationListener listener =
+ new LoadBalancerApplicationRefRegistrationListener
( name, conn );
+
+ listener.startListening();
+ return listener;
}
protected void mbeanRegistered(final ObjectName objectName) {
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerClusterRefRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/
LoadBalancerClusterRefRegistrationListener.java,v
retrieving revision 1.5
diff -u -w -r1.5 LoadBalancerClusterRefRegistrationListener.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerClusterRefRegistrationListener.java 9 Mar 2006
20:30:47 -0000 1.5
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerClusterRefRegistrationListener.java 12 Mar 2007
20:38:55 -0000
@@ -93,9 +93,22 @@
public class LoadBalancerClusterRefRegistrationListener
extends LBBaseMBeanRegistrationListener {
- public LoadBalancerClusterRefRegistrationListener(MBeanServer
mBeanServer)
+ private LoadBalancerClusterRefRegistrationListener(
+ final String name,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer);
+ super( name, mBeanServer);
+ }
+
+ public static LoadBalancerClusterRefRegistrationListener
+ newInstance(
+ final String name,
+ final MBeanServer conn ) throws InstanceNotFoundException,
IOException {
+ final LoadBalancerClusterRefRegistrationListener listener =
+ new LoadBalancerClusterRefRegistrationListener( name,
conn );
+
+ listener.startListening();
+ return listener;
}
protected void mbeanRegistered(final ObjectName objectName) {
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/LoadBalancerRegistrationListener.java,v
retrieving revision 1.9
diff -u -w -r1.9 LoadBalancerRegistrationListener.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerRegistrationListener.java 17 Mar 2006 03:34:20
-0000 1.9
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerRegistrationListener.java 12 Mar 2007 20:38:55
-0000
@@ -84,13 +84,25 @@
* @see LoadBalancerApplicationRefRegistrationListener
* @see LoadBalancerRegistrationListener
*/
-public class LoadBalancerRegistrationListener extends
LBBaseMBeanRegistrationListener {
+public final class LoadBalancerRegistrationListener extends
LBBaseMBeanRegistrationListener {
final static String MONITORING_ENABLED = "MonitoringEnabled";
- public LoadBalancerRegistrationListener(MBeanServer mBeanServer)
+ private LoadBalancerRegistrationListener(
+ final String name,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer);
+ super( name, mBeanServer);
+ }
+
+ public static LoadBalancerRegistrationListener
+ newInstance( final String name, final MBeanServer conn )
+ throws InstanceNotFoundException, IOException {
+ final LoadBalancerRegistrationListener listener =
+ new LoadBalancerRegistrationListener( name, conn );
+
+ listener.startListening();
+ return listener;
}
LoadBalancer registerLoadBalancer(String name) {
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerServerRefRegistrationListener.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/
LoadBalancerServerRefRegistrationListener.java,v
retrieving revision 1.6
diff -u -w -r1.6 LoadBalancerServerRefRegistrationListener.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerServerRefRegistrationListener.java 17 Mar 2006
03:34:20 -0000 1.6
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoadBalancerServerRefRegistrationListener.java 12 Mar 2007
20:38:55 -0000
@@ -81,13 +81,28 @@
* @see LoadBalancerApplicationRefRegistrationListener
* @see LoadBalancerRegistrationListener
*/
-public class LoadBalancerServerRefRegistrationListener
+public final class LoadBalancerServerRefRegistrationListener
extends LBBaseMBeanRegistrationListener {
- public LoadBalancerServerRefRegistrationListener(MBeanServer
mBeanServer)
+ private LoadBalancerServerRefRegistrationListener(
+ final String name,
+ final MBeanServer mBeanServer)
throws InstanceNotFoundException, IOException {
- super(mBeanServer);
+ super( name, mBeanServer);
}
+
+ public static LoadBalancerServerRefRegistrationListener
+ newInstance(
+ final String name,
+ final MBeanServer conn )
+ throws InstanceNotFoundException, IOException {
+ final LoadBalancerServerRefRegistrationListener listener =
+ new LoadBalancerServerRefRegistrationListener( name,
conn );
+
+ listener.startListening();
+ return listener;
+ }
+
protected void mbeanRegistered(final ObjectName objectName) {
try {
Index: admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoaderOfOldConfig.java
===================================================================
RCS file: /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/
enterprise/management/support/LoaderOfOldConfig.java,v
retrieving revision 1.9
diff -u -w -r1.9 LoaderOfOldConfig.java
--- admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoaderOfOldConfig.java 9 Jan 2007 23:00:35 -0000 1.9
+++ admin/mbeanapi-impl/src/java/com/sun/enterprise/management/
support/LoaderOfOldConfig.java 12 Mar 2007 20:38:55 -0000
@@ -94,7 +94,8 @@
try
{
- mServerRefListener = new ServerRefListener();
+ mServerRefListener = new ServerRefListener
( "LoaderOfOldConfig" );
+ mServerRefListener.startListening();
}
catch( Exception e )
{
@@ -118,10 +119,10 @@
private final class ServerRefListener
extends MBeanRegistrationListener
{
- public ServerRefListener( )
+ public ServerRefListener( final String name )
throws InstanceNotFoundException, IOException
{
- super( getMBeanServer(),
getListenToServerRefConfigPattern() );
+ super( name, getMBeanServer(),
getListenToServerRefConfigPattern() );
}