United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6636370 minor corrections and simplification of code in AppContext
JDK-6636370 : minor corrections and simplification of code in AppContext

Details
Type:
Bug
Submit Date:
2007-12-01
Status:
Closed
Updated Date:
2011-05-18
Project Name:
JDK
Resolved Date:
2011-05-18
Component:
client-libs
OS:
linux
Sub-Component:
java.awt
CPU:
x86
Priority:
P4
Resolution:
Fixed
Affected Versions:
7
Fixed Versions:

Related Reports

Sub Tasks

Description
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;
                                     
2007-12-01
EVALUATION

descripion contains all necessary information
                                     
2007-12-01



Hardware and Software, Engineered to Work Together