Changing RMIClient carries extreme risk
Have you done multi-machine tests with plenty of instances and a cluster
or two and repeated starts and stops of everything? If not, then I
recommend doing it, asking QE to do it, or not committing the changes.
Lloyd Chambers wrote:
> 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_null_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);
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>