Please review.
Begin forwarded message:
From: llc_at_dev.java.net
Date: January 30, 2007 6:24:02 PM PST
To: llc_at_dev.java.net
Subject: [Issue 2273] New - RMIClient is not thread safe
https://glassfish.dev.java.net/issues/show_bug.cgi?id=2273
Issue #|2273
Summary|RMIClient is not thread safe
Component|glassfish
Version|9.1pe
Platform|All
OS/Version|All
URL|
Status|UNCONFIRMED
Status whiteboard|
Keywords|
Resolution|
Issue type|DEFECT
Priority|P3
Subcomponent|admin
Assigned to|llc
Reported by|llc
------- Additional comments from llc_at_dev.java.net Wed Jan 31 02:24:02
+0000 2007 -------
RMIClient is not thread safe. A calling thread creates it, and it
spawns a thread. A later thread could try
to do various things with it, but there are no synchronization
constructs used anywhere. By making
most of the variable 'final', this problem can largely be
eliminated. The few remaining variables can be
made 'volatile'.
Suggested diffs:
MB2:/gf/build/glassfish/appserv-core lloyd$ cvs diff -u -w src/java/
com/sun/enterprise/admin/
server/core/channel/RMIClient.java
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 -w -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 31 Jan 2007
02:22:23 -0000
@@ -52,16 +52,16 @@
/**
* RMI client is used to send admin channel messages over RMI.
*/
-public class RMIClient implements Runnable {
+public final 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;
+ 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,10 +84,10 @@
if (this.stubFile.exists()) {
stub = readStub();
}
- if (AdminChannel.getClientAutoRefreshEnabled()) {
- startAutoRefreshThread(
- AdminChannel.getClientAutoRefreshInterval());
- }
+
+ autoRefresh = AdminChannel.getClientAutoRefreshEnabled();
+ autoRefreshInterval = autoRefresh ?
AdminChannel.getClientAutoRefreshInterval() : 0;
+ autoRefreshThread = autoRefresh ? startAutoRefreshThread
() : null;
}
/**
@@ -105,24 +105,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 +387,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 +564,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 +585,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$