admin@glassfish.java.net

code review: com.sun.enterprise.admin.server.core.channel.RMIClient

From: Lloyd Chambers <lloyd.chambers_at_mac.com>
Date: Fri, 2 Feb 2007 11:20:31 -0800

TIMEOUT: 17:00 PST Feb 2, 2007

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

Diffs and whole file follow. To make thread safe:

- static and instance variables 'final' wherever possible

- constructor changed to allow instance variables to be final
(prefetch booleans, allowing assignment once to final variable)

- 'synchronized' and 'volatile' used for mutable state

Lloyd


----------------
MB2:/gf/build/glassfish/appserv-core lloyd$ cvs diff -u 10 -w src/
java/com/sun/enterprise/admin/server/core/channel/RMIClient.java
cvs server: I know nothing about 10
cvs server: I know nothing about -w
Index: src/java/com/sun/enterprise/admin/server/core/channel/
RMIClient.java
===================================================================
RCS file: /cvs/glassfish/appserv-core/src/java/com/sun/enterprise/
admin/server/core/channel/RMIClient.java,v
retrieving revision 1.3
diff -u -r1.3 RMIClient.java
--- src/java/com/sun/enterprise/admin/server/core/channel/
RMIClient.java 25 Dec 2005 04:14:23 -0000 1.3
+++ src/java/com/sun/enterprise/admin/server/core/channel/
RMIClient.java 2 Feb 2007 19:18:16 -0000
@@ -52,16 +52,15 @@
/**
   * RMI client is used to send admin channel messages over RMI.
   */
-public class RMIClient implements Runnable {
-
- private File stubFile;
- private File seedFile;
- private long stubFileTs = 0;
- private byte[] key;
- private RemoteAdminChannel stub;
- private boolean autoRefresh;
- private long autoRefreshInterval;
- private Thread autoRefreshThread;
+public final class RMIClient implements Runnable {
+ private final File stubFile;
+ private final File seedFile;
+ private volatile long stubFileTs = 0;
+ private volatile byte[] key;
+ private volatile RemoteAdminChannel stub;
+ private volatile boolean autoRefresh;
+ private final long autoRefreshInterval;
+ private final Thread autoRefreshThread;
      /**
       * Create a new RMI client using specified stub file and use
the specified
@@ -84,19 +83,19 @@
          if (this.stubFile.exists()) {
              stub = readStub();
          }
- if (AdminChannel.getClientAutoRefreshEnabled()) {
- startAutoRefreshThread(
- AdminChannel.getClientAutoRefreshInterval());
- }
+
+ autoRefresh = AdminChannel.getClientAutoRefreshEnabled();
+ autoRefreshInterval = autoRefresh ?
AdminChannel.getClientAutoRefreshInterval() : 0;
+ autoRefreshThread = autoRefresh ? startAutoRefreshThread
() : null;
      }
      /**
       * Constructor for local mode
       */
      public RMIClient(boolean isDebug, String stubFile, String
seedFile) {
- if (! isDebug) {
- logger.setLevel(java.util.logging.Level.SEVERE);
- }
+ if (! isDebug) {
+ logger.setLevel(java.util.logging.Level.SEVERE);
+ }
          if (stubFile == null || seedFile == null) {
              throw new IllegalArgumentException
(CLIENT_NULLARGS_ERRMSG);
          }
@@ -105,24 +104,25 @@
          if (this.stubFile.exists()) {
              stub = readStub();
          }
+
+ autoRefresh = false;
+ autoRefreshInterval = 0;
+ autoRefreshThread = null;
      }
      /**
       * Start auto refresh thread. The auto refresh thread refreshes
the remote
       * stub if the stub file has changed on the disk.
       */
- void startAutoRefreshThread(long interval) {
- if (interval <= 0) {
+ private Thread startAutoRefreshThread() {
+ if ( autoRefreshInterval <= 0 || ! autoRefresh ) {
              throw new IllegalArgumentException
(INVALID_AUTO_REFRESH_INTERVAL);
          }
- autoRefresh = true;
- autoRefreshInterval = interval;
- if (autoRefreshThread != null && autoRefreshThread.isAlive()) {
- return;
- } else {
- autoRefreshThread = new Thread(this);
- autoRefreshThread.start();
- }
+
+ final Thread theThread = new Thread(this);
+ theThread.start();
+
+ return theThread;
      }
      /**
@@ -386,7 +386,7 @@
      /**
       * Check status of server stub file and refresh if needed.
       */
- private boolean checkServerStatus() {
+ private synchronized boolean checkServerStatus() {
          boolean gotNew = false;
          if (stubFile.exists()) {
              long ts = stubFile.lastModified();
@@ -563,7 +563,7 @@
      }
         // i18n StringManager
- private static StringManager localStrings =
+ private static final StringManager localStrings =
                 StringManager.getManager( RMIClient.class );
      private static final String CLIENT_NULLARGS_ERRCODE =
@@ -584,5 +584,5 @@
      static final String FILE_READ_ERROR = "channel.file_read_error";
      static final String SEED_FILE_OLDER = "channel.seed_file_older";
- static Logger logger = Logger.getLogger
(AdminConstants.kLoggerName);
+ static final Logger logger = Logger.getLogger
(AdminConstants.kLoggerName);
}
MB2:/gf/build/glassfish/appserv-core lloyd$

--------------------

public final class RMIClient implements Runnable {
     private final File stubFile;
     private final File seedFile;
     private volatile long stubFileTs = 0;
     private volatile byte[] key;
     private volatile RemoteAdminChannel stub;
     private volatile boolean autoRefresh;
     private final long autoRefreshInterval;
     private final Thread autoRefreshThread;

     /**
      * Create a new RMI client using specified stub file and use the
specified
      * byte array (seed) for all calls to the remote object. To
obtain an
      * instance of RMIClient please use the public method
      * <code>AdminChannel.getRMIClient()</code> which ensure that
all calls to
      * a particular server instance go through the same object instance
      * (of RMIClient).
      * @param stubFile name of the RMI stub file
      * @param seedFile name of the shared secret file
      * @throws IllegalArgumentException if either stubFile or
seedFile is null.
      */
     RMIClient(String stubFile, String seedFile) {
         if (stubFile == null || seedFile == null) {
             warn(CLIENT_NULLARGS_ERRCODE);
             throw new IllegalArgumentException(CLIENT_NULLARGS_ERRMSG);
         }
         this.stubFile = new File(stubFile);
         this.seedFile = new File(seedFile);
         if (this.stubFile.exists()) {
             stub = readStub();
         }

         autoRefresh = AdminChannel.getClientAutoRefreshEnabled();
         autoRefreshInterval = autoRefresh ?
AdminChannel.getClientAutoRefreshInterval() : 0;
         autoRefreshThread = autoRefresh ? startAutoRefreshThread
() : null;
     }

     /**
      * Constructor for local mode
      */
     public RMIClient(boolean isDebug, String stubFile, String
seedFile) {
         if (! isDebug) {
                 logger.setLevel(java.util.logging.Level.SEVERE);
         }
         if (stubFile == null || seedFile == null) {
             throw new IllegalArgumentException(CLIENT_NULLARGS_ERRMSG);
         }
         this.stubFile = new File(stubFile);
         this.seedFile = new File(seedFile);
         if (this.stubFile.exists()) {
             stub = readStub();
         }

         autoRefresh = false;
         autoRefreshInterval = 0;
         autoRefreshThread = null;
     }

     /**
      * Start auto refresh thread. The auto refresh thread refreshes
the remote
      * stub if the stub file has changed on the disk.
      */
     private Thread startAutoRefreshThread() {
         if ( autoRefreshInterval <= 0 || ! autoRefresh ) {
             throw new IllegalArgumentException
(INVALID_AUTO_REFRESH_INTERVAL);
         }

         final Thread theThread = new Thread(this);
         theThread.start();

         return theThread;
     }

     /**
      * Stop auto refresh thread.
      */
     void stopAutoRefreshThread() {
         autoRefresh = false;
     }

     /**
      * Auto refresh this rmi client.
      */
     public void run() {
         while (autoRefresh) {
             try {
                 Thread.sleep(autoRefreshInterval);
             } catch (InterruptedException ie) {
                 warn(AUTO_REFRESH_INTR);
                 autoRefresh = false;
             }
             if (autoRefresh) {
                 checkServerStatus();
             }
         }
     }

     /**
      * Send specified event notification over admin channel
      */
     public AdminEventResult sendNotification(AdminEvent event) {
         // Normal handling, send the event
         boolean doRetry = true;
         AdminEventResult result = null;
         if (stub != null) {
             try {
                 result = stub.sendNotification(key, event);
                 doRetry = false;
             } catch (ServerException re) {
                 if ((re.detail != null) &&
                         (re.detail instanceof
java.lang.IllegalArgumentException
                         || re.detail instanceof
java.lang.SecurityException)) {
                     doRetry = false;
                     warn(EVENT_NOTIFY_ERROR);
                     debug(re.detail);
                 } else {
                     if (re.detail != null) {
                         debug(re.detail);
                     }
                     debug(re);
                 }
             } catch (RemoteException re) {
                 if (re.detail != null) {
                     debug(re.detail);
                 }
                 debug(re);
             }
         }
         if (doRetry) {
             // Normal processing did not work, try to get stub again
and then
             // attempt to send the event
             boolean gotNew = checkServerStatus();
             if (stub != null && gotNew) {
                 try {
                     result = stub.sendNotification(key, event);
                 } catch (RemoteException re) {
                     warn(EVENT_RENOTIFY_ERROR);
                     if (re.detail != null) {
                         debug(re.detail);
                     }
                     debug(re);
                 }
             }
         }
         if (result == null) {
             // Still couldn't communicate, set result appropriately
             result = new AdminEventResult(event.getSequenceNumber());
             result.setResultCode(AdminEventResult.TRANSMISSION_ERROR);
             if (stub == null) {
                 result.addMessage(event.getEffectiveDestination(),
                     "Remote Stub is null");
             }
         }
         return result;
     }

     /** Determines wheter the instance with given name is alive.
      * Really speaking, checks whether the RMI channel is responsive.
      * The method returns immediately with the status without any
retries.
      * @return boolean indicating whether the RMIServer Object
      * that <code> this </code> server instance is alive.
      */
     public boolean isAlive() {
         return isAlive(false);
     }

     /**
      * Determines whether the instance represented by this object is
alive.
      * Unless the parameter refreshStub is true, the responsiveness of
      * instance is checked by using cached stub (if any) and if
there is no
      * cached stub the method returns false.
      * @param refreshStub if true, refresh remote stub if it has
changed
      * @return true if the instance represented by <code>this</code>
object
      * is responding, false otherwise.
      */
     public boolean isAlive(boolean refreshStub) {

         if (refreshStub) {
             boolean gotNew = checkServerStatus();
         }

         boolean isAlive = true;

         if (stub != null) {
             try {
                 stub.pingServer(key);
             }
             catch(RemoteException re) {
                 debug(re);
                 isAlive = false;
             }
         }
         else {
             isAlive = false;
         }
         return ( isAlive );
     }

     /**
      * Get server instance status code. If the instance can not be
contacted
      * over rmi channel then the method reports that instance is not
running.
      * The return value of this method is one of the constants <code>
      * kInstanceStartingCode, kInstanceRunningCode,
kInstanceStoppingCode or
      * kInstanceNotRunningCode</code> from the class <code>
      * com.sun.enterprise.admin.common.Status</code>.
      * @return an int denoting the status of server starting,
running, stopping
      * or not running.
      */
     public int getInstanceStatusCode() {
         boolean gotNew = checkServerStatus();
         int statusCode = Status.kInstanceNotRunningCode;
         if (stub != null) {
             try {
                 statusCode = stub.getServerStatusCode(key);
             } catch (RemoteException re) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 if (re.detail != null) {
                     trace(re.detail);
                 }
                 trace(re);
             } catch (IllegalArgumentException iae) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 trace(iae);
                 // This means that the key did not match. Attempt to
read
                 // the key from the disk again to work around the race
                 // condition when the file is read on the client
before the
                 // server has finished writing (and hence client has
partial
                 // key). Another read updates the shared key on the
client.
                 byte[] newKey = null;
                 try {
                     newKey = readSeed();
                 } catch (IOException ioe) {
                     debug(FILE_READ_ERROR, seedFile.getName());
                     trace(ioe);
                 }
                 if (newKey != null) {
                     key = newKey;
                 }
                 throw iae;
             }
         }
         return statusCode;
     }

     /**
      * Get the port where the conflict occurs.
      *
      * @return A valid port number.0 If there is any error in
processing
      */
     public int getConflictedPort() {
         int conflictedPort = 0;
         boolean gotNew = checkServerStatus();
         if (stub != null) {
             try {
                 conflictedPort = stub.getConflictedPort(key);
             } catch (RemoteException re) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 if (re.detail != null) {
                     trace(re.detail);
                 }
                 trace(re);
             }
         }
         return conflictedPort;
     }

     /**
      * Triggers exit of the server VM. Server VM will client
      * that got the conflicted port number calls this method.
      */
     public void triggerServerExit() {
         boolean gotNew = checkServerStatus();
         if (stub != null) {
             try {
                 stub.triggerServerExit(key);
             } catch (RemoteException re) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 if (re.detail != null) {
                     trace(re.detail);
                 }
                 trace(re);
             }
         }
     }

     /**
      * Is restart needed on the instance. The status is set by admin
server and
      * instance just keeps track of it across admin server restarts.
If the
      * instance has already restarted since admin server set the
status, the
      * instance does not require restart (as expected).
      * @return true if the instance requires restart, false otherwise.
      */
     public boolean isRestartNeeded() {
         boolean restartNeeded = false;
         boolean gotNew = checkServerStatus();
         if (stub != null) {
             try {
                 restartNeeded = stub.isRestartNeeded(key);
             } catch (RemoteException re) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 if (re.detail != null) {
                     trace(re.detail);
                 }
                 trace(re);
             }
         }
         return restartNeeded;
     }

     /**
      * Set restart needed status for a server instance. If the
instance is not
      * running or if there is an error in communicating, the
instance restart
      * status will not be changed. The status set by this method is
preserved
      * in the instance across admin server restarts.
      * @param restartNeeded true if instance restart is needed,
false otherwise.
      */
     public void setRestartNeeded(boolean restartNeeded) {
         boolean gotNew = checkServerStatus();
         if (stub != null) {
             try {
                 stub.setRestartNeeded(key, restartNeeded);
             } catch (RemoteException re) {
                 debug(CHANNEL_COMM_ERROR, stubFile.getName());
                 if (re.detail != null) {
                     trace(re.detail);
                 }
                 trace(re);
             }
         }
     }

     /**
      * Check status of server stub file and refresh if needed.
      */
     private synchronized boolean checkServerStatus() {
         boolean gotNew = false;
         if (stubFile.exists()) {
             long ts = stubFile.lastModified();
             if (ts > stubFileTs) {
                 if (stubFile.canRead()) {
                     RemoteAdminChannel obj = readStub();
                     if (obj != null) {
                         gotNew = true;
                         stub = obj;
                     }
                 } else {
                     warn(FILE_READ_ERROR, stubFile.getName());
                 }
             }
         } else {
             if (stub != null) {
                 stub = null;
             }
         }
         return gotNew;
     }

     /**
      * Read stub from stub file
      */
     private RemoteAdminChannel readStub() {
         RemoteAdminChannel obj = null;
         FileInputStream fis = null;
         try {
             stubFileTs = stubFile.lastModified();
             fis = new FileInputStream(stubFile);
             ObjectInputStream ois = new ObjectInputStream(fis);
             obj = (RemoteAdminChannel)ois.readObject();
         } catch (IOException ioe) {
             warn(CLIENT_INIT_ERROR);
             debug(ioe);
         } catch (ClassNotFoundException cnfe) {
             warn(CLIENT_INIT_ERROR);
             debug(cnfe);
         } finally {
             if (fis != null) {
                 try {
                     fis.close();
                 } catch (IOException ioe) {
                 }
             }
         }
         try {
             key = readSeed();
         } catch (IOException ioe) {
             warn(CLIENT_INIT_ERROR);
             debug(ioe);
             obj = null;
         }
         if (obj == null) {
             stubFileTs = 0;
         }
         return obj;
     }

     /**
      * Read shared secret from file.
      */
     private byte[] readSeed() throws IOException {
         byte[] seed = null;
         if (seedFile.exists() && seedFile.canRead()) {
             // Seed file is updated after creating stub file and
therefore
             // it should only be read only if it has been modified
since stub
             // file was modified. NOTE: This relies on the fact that
the
             // server always (on every startup) writes seed file
after stubfile.
             long seedFileTs = seedFile.lastModified();
             if (seedFileTs >= stubFileTs) {
                 FileInputStream fis = null;
                 try {
                     fis = new FileInputStream(seedFile);
                     seed = new byte[AdminChannel.SEED_LENGTH];
                     fis.read(seed);
                 } catch (IOException ioe) {
                     warn(AdminChannel.KEY_READ_ERROR);
                     debug(ioe);
                     seed = null;
                 } finally {
                     if (fis != null) {
                         try {
                             fis.close();
                         } catch (IOException ioe) {
                         }
                     }
                 }
             } else {
                 debug(SEED_FILE_OLDER,
                         new Long[] {new Long(seedFileTs), new Long
(stubFileTs)});
             }
         } else {
             warn(AdminChannel.KEY_READ_ERROR);
             debug(FILE_READ_ERROR, seedFile.getName());
         }
         if (seed == null) {
                        String msg = localStrings.getString
( "admin.server.core.channel.unable_initializing_key", seedFile );
             throw new IOException( msg );
         }
         return seed;
     }

     /**
      * Has the server instance restarted since specified timestamp.
If instance
      * is not running, the method will return false. The return
value is
      * accurate within the timespan as specified in
getClientRefreshInterval()
      * of AdminChannel -- meaning that if the time period since
instance restart
      * is less than the specified timespan then the method may
return false
      * instead of true.
      * @param ts Timestamp to check for
      * @return true if the instance has restarted since specified
timestamp,
      * false otherwise.
      */
     public boolean hasRestartedSince(long ts) {
         return hasRestartedSince(ts, false);
     }

     /**
      * Has the server instance restarted since specified timestamp.
If instance
      * is not running, the method will return false. If refreshStub
is true
      * then the method will attempt to read newer stub (if any) and
will report
      * status using that. If refreshStub is false then the stub is
not re-read
      * and return value is dependent on the cached stub (if any).
      * @param ts Timestamp to check for
      * @param refreshStub if true, refresh remote stub if it has
changed
      * @return true if the instance has restarted since specified
timestamp,
      * false otherwise.
      */
     public boolean hasRestartedSince(long ts, boolean refreshStub) {
         if (refreshStub) {
             boolean gotNew = checkServerStatus();
         }
         boolean restarted = false;
         if (stubFile != null) {
             if (stubFileTs > ts) {
                 restarted = true;
             }
         }
         return restarted;
     }

     //
     // logger methods copied from AdminChannel
     //

     static void trace(Throwable t) {
         logger.log(Level.FINEST, t.getMessage(), t);
     }

     static void warn(String s) {
         logger.warning(s);
     }

     static void warn(String msgkey, String obj1) {
         logger.log(Level.WARNING, msgkey, obj1);
     }

     static void debug(String s) {
         logger.fine(s);
     }

     static void debug(String msgkey, String obj1) {
         logger.log(Level.FINE, msgkey, obj1);
     }

     static void debug(String msgkey, Object[] objarr) {
         logger.log(Level.FINE, msgkey, objarr);
     }

     static void debug(Throwable t) {
         logger.log(Level.FINE, t.getMessage(), t);
     }

        // i18n StringManager
        private static final StringManager localStrings =
                StringManager.getManager( RMIClient.class );

     private static final String CLIENT_NULLARGS_ERRCODE =
             "channel.client_nullargs";
     private static final String CLIENT_NULLARGS_ERRMSG =
                        localStrings.getString
( "admin.server.core.channel.attempt_initializing_channel_client_with_nu
ll_arguments" );
     private static final String CLIENT_INIT_ERROR =
"channel.client_init_error";
     private static final String EVENT_NOTIFY_ERROR =
             "channel.event_notify_error";
     private static final String EVENT_RENOTIFY_ERROR =
             "channel.event_renotify_error";
     private static final String AUTO_REFRESH_INTR =
"channel.auto_refresh_intr";
     private static final String CHANNEL_COMM_ERROR =
"channel.comm_error";
     private static final String INVALID_AUTO_REFRESH_INTERVAL =
             localStrings.getString(
                      
"admin.server.core.channel.invalid_auto_refresh_interval");

     static final String FILE_READ_ERROR = "channel.file_read_error";
     static final String SEED_FILE_OLDER = "channel.seed_file_older";

     static final Logger logger = Logger.getLogger
(AdminConstants.kLoggerName);
}