8019853: Break logging and AWT circular dependency

Summary: Break logging and AWT circular dependency, which was at the root cause for 8023258 - Logger.getLogger() after ImageIO.read() returns different logger instance
Reviewed-by: mchung, art

src/share/classes/java/util/logging/LogManager.java

@@ -391,6 +391,9 @@ public class LogManager {
         }
     }
 
+    // LoggerContext maps from AppContext
+    private static WeakHashMap<Object, LoggerContext> contextsMap = null;
+
     // Returns the LoggerContext for the user code (i.e. application or AppContext).
     // Loggers are isolated from each AppContext.
     private LoggerContext getUserContext() {
@@ -399,33 +402,28 @@ public class LogManager {
         SecurityManager sm = System.getSecurityManager();
         JavaAWTAccess javaAwtAccess = SharedSecrets.getJavaAWTAccess();
         if (sm != null && javaAwtAccess != null) {
+            // for each applet, it has its own LoggerContext isolated from others
             synchronized (javaAwtAccess) {
-                // AppContext.getAppContext() returns the system AppContext if called
-                // from a system thread but Logger.getLogger might be called from
-                // an applet code. Instead, find the AppContext of the applet code
-                // from the execution stack.
-                Object ecx = javaAwtAccess.getExecutionContext();
-                if (ecx == null) {
-                    // fall back to thread group seach of AppContext
-                    ecx = javaAwtAccess.getContext();
-                }
+                // find the AppContext of the applet code
+                // will be null if we are in the main app context.
+                final Object ecx = javaAwtAccess.getAppletContext();
                 if (ecx != null) {
-                    context = (LoggerContext)javaAwtAccess.get(ecx, LoggerContext.class);
+                    if (contextsMap == null) {
+                        contextsMap = new WeakHashMap<>();
+                    }
+                    context = contextsMap.get(ecx);
                     if (context == null) {
-                        if (javaAwtAccess.isMainAppContext()) {
-                            context = userContext;
-                        } else {
-                            // Create a new LoggerContext for the applet.
-                            // The new logger context has its requiresDefaultLoggers
-                            // flag set to true - so that these loggers will be
-                            // lazily added when the context is firt accessed.
-                            context = new LoggerContext(true);
-                        }
-                        javaAwtAccess.put(ecx, LoggerContext.class, context);
+                        // Create a new LoggerContext for the applet.
+                        // The new logger context has its requiresDefaultLoggers
+                        // flag set to true - so that these loggers will be
+                        // lazily added when the context is firt accessed.
+                        context = new LoggerContext(true);
+                        contextsMap.put(ecx, context);
                     }
                 }
             }
         }
+        // for standalone app, return userContext
         return context != null ? context : userContext;
     }
 

src/share/classes/sun/awt/AppContext.java

@@ -838,21 +838,59 @@ public final class AppContext {
             public boolean isMainAppContext() {
                 return (numAppContexts.get() == 1 && mainAppContext != null);
             }
-            public Object getContext() {
-                return getAppContext();
-            }
-            public Object getExecutionContext() {
-                return getExecutionAppContext();
-            }
-            public Object get(Object context, Object key) {
-                return ((AppContext)context).get(key);
-            }
-            public void put(Object context, Object key, Object value) {
-                ((AppContext)context).put(key, value);
-            }
-            public void remove(Object context, Object key) {
-                ((AppContext)context).remove(key);
+
+            /**
+             * Returns the AppContext used for applet logging isolation, or null if
+             * the default global context can be used.
+             * If there's no applet, or if the caller is a stand alone application,
+             * or running in the main app context, returns null.
+             * Otherwise, returns the AppContext of the calling applet.
+             * @return null if the global default context can be used,
+             *         an AppContext otherwise.
+             **/
+            public Object getAppletContext() {
+                // There's no AppContext: return null.
+                // No need to call getAppContext() if numAppContext == 0:
+                // it means that no AppContext has been created yet, and
+                // we don't want to trigger the creation of a main app
+                // context since we don't need it.
+                if (numAppContexts.get() == 0) return null;
+
+                // Get the context from the security manager
+                AppContext ecx = getExecutionAppContext();
+
+                // Not sure we really need to re-check numAppContexts here.
+                // If all applets have gone away then we could have a
+                // numAppContexts coming back to 0. So we recheck
+                // it here because we don't want to trigger the
+                // creation of a main AppContext in that case.
+                // This is probably not 100% MT-safe but should reduce
+                // the window of opportunity in which that issue could
+                // happen.
+                if (numAppContexts.get() > 0) {
+                   // Defaults to thread group caching.
+                   // This is probably not required as we only really need
+                   // isolation in a deployed applet environment, in which
+                   // case ecx will not be null when we reach here
+                   // However it helps emulate the deployed environment,
+                   // in tests for instance.
+                   ecx = ecx != null ? ecx : getAppContext();
+                }
+
+                // getAppletContext() may be called when initializing the main
+                // app context - in which case mainAppContext will still be
+                // null. To work around this issue we simply use
+                // AppContext.threadGroup.getParent() == null instead, since
+                // mainAppContext is the only AppContext which should have
+                // the root TG as its thread group.
+                // See: JDK-8023258
+                final boolean isMainAppContext = ecx == null
+                    || mainAppContext == ecx
+                    || mainAppContext == null && ecx.threadGroup.getParent() == null;
+
+                return isMainAppContext ? null : ecx;
             }
+
         });
     }
 }

src/share/classes/sun/misc/JavaAWTAccess.java

@@ -26,14 +26,16 @@
 package sun.misc;
 
 public interface JavaAWTAccess {
-    public Object getContext();
-    public Object getExecutionContext();
 
-    public Object get(Object context, Object key);
-    public void put(Object context, Object key, Object value);
-    public void remove(Object context, Object key);
+    // Returns the AppContext used for applet logging isolation, or null if
+    // no isolation is required.
+    // If there's no applet, or if the caller is a stand alone application,
+    // or running in the main app context, returns null.
+    // Otherwise, returns the AppContext of the calling applet.
+    public Object getAppletContext();
 
-    // convenience methods whose context is the object returned by getContext()
+    // convenience methods to cache objects in the current thread group's
+    // AppContext
     public Object get(Object key);
     public void put(Object key, Object value);
     public void remove(Object key);

src/share/classes/sun/misc/SharedSecrets.java

@@ -170,7 +170,7 @@ public class SharedSecrets {
     public static JavaAWTAccess getJavaAWTAccess() {
         // this may return null in which case calling code needs to
         // provision for.
-        if (javaAWTAccess == null || javaAWTAccess.getContext() == null) {
+        if (javaAWTAccess == null) {
             return null;
         }
         return javaAWTAccess;

test/java/util/logging/TestAppletLoggerContext.java

@@ -110,28 +110,19 @@ public class TestAppletLoggerContext {
             }
 
             TestExc exc;
-            TestExc global = new TestExc();
 
             @Override
-            public Object getContext() { return active ? global : null; }
+            public Object getAppletContext() { return active ? exc : null; }
             @Override
-            public Object getExecutionContext() { return active ? exc : null; }
+            public Object get(Object o) { return exc.get(o); }
             @Override
-            public Object get(Object o, Object o1) { return TestExc.exc(o).get(o1); }
+            public void put(Object o, Object o1) { exc.put(o, o1); }
             @Override
-            public void put(Object o, Object o1, Object o2) { TestExc.exc(o).put(o1, o2); }
-            @Override
-            public void remove(Object o, Object o1) { TestExc.exc(o).remove(o1); }
-            @Override
-            public Object get(Object o) { return global.get(o); }
-            @Override
-            public void put(Object o, Object o1) { global.put(o, o1); }
-            @Override
-            public void remove(Object o) { global.remove(o); }
+            public void remove(Object o) { exc.remove(o); }
             @Override
             public boolean isDisposed() { return false; }
             @Override
-            public boolean isMainAppContext() { return exc == null; }
+            public boolean isMainAppContext() { return !active || exc == null; }
         }
 
         final static JavaAWTAccessStub javaAwtAccess = new JavaAWTAccessStub();

test/java/util/logging/TestLoggingWithMainAppContext.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.util.logging.Logger;
+import javax.imageio.ImageIO;
+
+/**
+ * @test
+ * @bug 8019853 8023258
+ * @summary Test that the default user context is used when in the main
+ *          application context. This test must not be run in same VM or agent
+ *          VM mode: it would not test the intended behavior.
+ * @run main/othervm TestLoggingWithMainAppContext
+ */
+public class TestLoggingWithMainAppContext {
+
+    public static void main(String[] args) throws IOException {
+        System.out.println("Creating loggers.");
+
+        // These loggers will be created in the default user context.
+        final Logger foo1 = Logger.getLogger( "foo" );
+        final Logger bar1 = Logger.getLogger( "foo.bar" );
+        if (bar1.getParent() != foo1) {
+            throw new RuntimeException("Parent logger of bar1 "+bar1+" is not "+foo1);
+        }
+        System.out.println("bar1.getParent() is the same as foo1");
+
+        // Set a security manager
+        System.setSecurityManager(new SecurityManager());
+        System.out.println("Now running with security manager");
+
+        // Triggers the creation of the main AppContext
+        ByteArrayInputStream is = new ByteArrayInputStream(new byte[] { 0, 1 });
+        ImageIO.read(is); // triggers calls to system loggers & creation of main AppContext
+
+        // verify that we're still using the default user context
+        final Logger bar2 = Logger.getLogger( "foo.bar" );
+        if (bar1 != bar2) {
+            throw new RuntimeException("bar2 "+bar2+" is not the same as bar1 "+bar1);
+        }
+        System.out.println("bar2 is the same as bar1");
+        if (bar2.getParent() != foo1) {
+            throw new RuntimeException("Parent logger of bar2 "+bar2+" is not foo1 "+foo1);
+        }
+        System.out.println("bar2.getParent() is the same as foo1");
+        final Logger foo2 = Logger.getLogger("foo");
+        if (foo1 != foo2) {
+            throw new RuntimeException("foo2 "+foo2+" is not the same as foo1 "+foo1);
+        }
+        System.out.println("foo2 is the same as foo1");
+
+        System.out.println("Test passed.");
+    }
+}