JDK-6636370 : minor corrections and simplification of code in AppContext
Type:Bug
Component:client-libs
Sub-Component:java.awt
Affected Version:7
Priority:P4
Status:Closed
Resolution:Fixed
OS:linux
CPU:x86
Submitted:2007-12-01
Updated:2011-05-18
Resolved:2011-05-18
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
Working on AppContext code I've noticed that:
1. mainAppContext, isDisposed, and numAppContexts should be made volatile.
2. mostRecentThreadAppContext should be rewritten using ThreadLocal to simplify the code.
Comments
SUGGESTED FIX
src/share/classes/sun/awt/AppContext.java
Print this page
@@ -147,11 +147,11 @@
}
/* The main "system" AppContext, used by everything not otherwise
contained in another AppContext.
*/
- private static AppContext mainAppContext = null;
+ private static volatile AppContext mainAppContext = null;
/*
* The hash map associated with this AppContext. A private delegate
* is used instead of subclassing HashMap so as to avoid all of
* HashMap's potentially risky methods, such as clear(), elements(),
@@ -172,17 +172,16 @@
private PropertyChangeSupport changeSupport = null;
public static final String DISPOSED_PROPERTY_NAME = "disposed";
public static final String GUI_DISPOSED = "guidisposed";
- private boolean isDisposed = false; // true if AppContext is disposed
+ private volatile boolean isDisposed = false; // true if AppContext is disposed
public boolean isDisposed() {
return isDisposed;
}
-
static {
// On the main Thread, we get the ThreadGroup, make a corresponding
// AppContext, and instantiate the Java EventQueue. This way, legacy
// code is unaffected by the move to multiple AppContext ability.
AccessController.doPrivileged(new PrivilegedAction() {
@@ -207,11 +206,11 @@
* incremented at the beginning of the constructor, and decremented
* at the end of dispose(). getAppContext() checks to see if this
* number is 1. If so, it returns the sole AppContext without
* checking Thread.currentThread().
*/
- private static int numAppContexts;
+ private static volatile int numAppContexts;
/*
* The context ClassLoader that was used to create this AppContext.
*/
private final ClassLoader contextClassLoader;
@@ -234,18 +233,19 @@
this.threadGroup = threadGroup;
threadGroup2appContext.put(threadGroup, this);
this.contextClassLoader =
- (ClassLoader) AccessController.doPrivileged(new PrivilegedAction() {
- public Object run() {
+ AccessController.doPrivileged(new PrivilegedAction<ClassLoader>() {
+ public ClassLoader run() {
return Thread.currentThread().getContextClassLoader();
}
});
}
- private static MostRecentThreadAppContext mostRecentThreadAppContext = null;
+ private static final ThreadLocal<AppContext> threadAppContext =
+ new ThreadLocal<AppContext>();
/**
* Returns the appropriate AppContext for the caller,
* as determined by its ThreadGroup. If the main "system" AppContext
* would be returned and there's an AWTSecurityManager installed, it
@@ -258,33 +258,21 @@
*/
public final static AppContext getAppContext() {
if (numAppContexts == 1) // If there's only one system-wide,
return mainAppContext; // return the main system AppContext.
- final Thread currentThread = Thread.currentThread();
-
- AppContext appContext = null;
+ AppContext appContext = threadAppContext.get();
- // Note: this most recent Thread/AppContext caching is thread-hot.
- // A simple test using SwingSet found that 96.8% of lookups
- // were matched using the most recent Thread/AppContext. By
- // instantiating a simple MostRecentThreadAppContext object on
- // cache misses, the cache hits can be processed without
- // synchronization.
-
- MostRecentThreadAppContext recent = mostRecentThreadAppContext;
- if ((recent != null) && (recent.thread == currentThread)) {
- appContext = recent.appContext; // Cache hit
- } else {
- appContext = (AppContext)AccessController.doPrivileged(
- new PrivilegedAction() {
- public Object run() {
+ if (null == appContext) {
+ appContext = AccessController.doPrivileged(new PrivilegedAction<AppContext>()
+ {
+ public AppContext run() {
// Get the current ThreadGroup, and look for it and its
// parents in the hash from ThreadGroup to AppContext --
// it should be found, because we use createNewContext()
// when new AppContext objects are created.
- ThreadGroup currentThreadGroup = currentThread.getThreadGroup();
+ ThreadGroup currentThreadGroup = Thread.currentThread().getThreadGroup();
ThreadGroup threadGroup = currentThreadGroup;
AppContext context = threadGroup2appContext.get(threadGroup);
while (context == null) {
threadGroup = threadGroup.getParent();
if (threadGroup == null) {
@@ -305,12 +293,11 @@
}
// Now we're done, so we cache the latest key/value pair.
// (we do this before checking with any AWTSecurityManager, so if
// this Thread equates with the main AppContext in the cache, it
// still will)
- mostRecentThreadAppContext =
- new MostRecentThreadAppContext(currentThread, context);
+ threadAppContext.set(context);
return context;
}
});
}
@@ -319,13 +306,13 @@
// Before we return the main "system" AppContext, check to
// see if there's an AWTSecurityManager installed. If so,
// allow it to choose the AppContext to return.
SecurityManager securityManager = System.getSecurityManager();
if ((securityManager != null) &&
- (securityManager instanceof AWTSecurityManager)) {
- AWTSecurityManager awtSecMgr =
- (AWTSecurityManager)securityManager;
+ (securityManager instanceof AWTSecurityManager))
+ {
+ AWTSecurityManager awtSecMgr = (AWTSecurityManager)securityManager;
AppContext secAppContext = awtSecMgr.getAppContext();
if (secAppContext != null) {
appContext = secAppContext; // Return what we're told
}
}
@@ -442,11 +429,11 @@
// Next, we sleep 10ms at a time, waiting for all of the active
// Threads in the ThreadGroup to exit.
long startTime = System.currentTimeMillis();
- long endTime = startTime + (long)THREAD_INTERRUPT_TIMEOUT;
+ long endTime = startTime + THREAD_INTERRUPT_TIMEOUT;
while ((this.threadGroup.activeCount() > 0) &&
(System.currentTimeMillis() < endTime)) {
try {
Thread.sleep(10);
} catch (InterruptedException e) { }
@@ -457,11 +444,11 @@
// Next, we sleep 10ms at a time, waiting for all of the active
// Threads in the ThreadGroup to die.
startTime = System.currentTimeMillis();
- endTime = startTime + (long)THREAD_INTERRUPT_TIMEOUT;
+ endTime = startTime + THREAD_INTERRUPT_TIMEOUT;
while ((this.threadGroup.activeCount() > 0) &&
(System.currentTimeMillis() < endTime)) {
try {
Thread.sleep(10);
} catch (InterruptedException e) { }
@@ -476,14 +463,11 @@
threadGroup2appContext.remove(subGroups[subGroup]);
}
}
threadGroup2appContext.remove(this.threadGroup);
- MostRecentThreadAppContext recent = mostRecentThreadAppContext;
- if ((recent != null) && (recent.appContext == this))
- mostRecentThreadAppContext = null;
- // If the "most recent" points to this, clear it for GC
+ threadAppContext.set(null);
// Finally, we destroy the ThreadGroup entirely.
try {
this.threadGroup.destroy();
} catch (IllegalThreadStateException e) {
@@ -662,10 +646,11 @@
/**
* Returns a string representation of this AppContext.
* @since 1.2
*/
+ @Override
public String toString() {
return getClass().getName() + "[threadGroup=" + threadGroup.getName() + "]";
}
/**
@@ -767,19 +752,10 @@
}
return changeSupport.getPropertyChangeListeners(propertyName);
}
}
-final class MostRecentThreadAppContext {
- final Thread thread;
- final AppContext appContext;
- MostRecentThreadAppContext(Thread key, AppContext value) {
- thread = key;
- appContext = value;
- }
-}
-
final class MostRecentKeyValue {
Object key;
Object value;
MostRecentKeyValue(Object k, Object v) {
key = k;
01-12-2007
EVALUATION
descripion contains all necessary information