8314263: Signed jars triggering Logger finder recursion and StackOverflowError

8315696: SignedLoggerFinderTest.java test failed
8316087: Test SignedLoggerFinderTest.java is still failing

Reviewed-by: lucy
Backport-of: 7daae1fb4267f92b38f0152611d69b7b89691087

Author: Andrew Lu

src/java.base/share/classes/java/lang/System.java

@@ -60,6 +60,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
+import jdk.internal.logger.LoggerFinderLoader.TemporaryLoggerFinder;
 import jdk.internal.util.StaticProperty;
 import jdk.internal.module.ModuleBootstrap;
 import jdk.internal.module.ServicesCatalog;
@@ -1619,13 +1620,16 @@ static LoggerFinder accessProvider() {
             // We do not need to synchronize: LoggerFinderLoader will
             // always return the same instance, so if we don't have it,
             // just fetch it again.
-            if (service == null) {
+            LoggerFinder finder = service;
+            if (finder == null) {
                 PrivilegedAction<LoggerFinder> pa =
                         () -> LoggerFinderLoader.getLoggerFinder();
-                service = AccessController.doPrivileged(pa, null,
+                finder = AccessController.doPrivileged(pa, null,
                         LOGGERFINDER_PERMISSION);
+                if (finder instanceof TemporaryLoggerFinder) return finder;
+                service = finder;
             }
-            return service;
+            return finder;
         }
 
     }

src/java.base/share/classes/jdk/internal/logger/BootstrapLogger.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, 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
@@ -38,7 +38,6 @@
 import java.util.function.Supplier;
 import java.lang.System.LoggerFinder;
 import java.lang.System.Logger;
-import java.lang.System.Logger.Level;
 import java.lang.ref.WeakReference;
 import java.util.Objects;
 import java.util.concurrent.ExecutionException;
@@ -228,9 +227,19 @@ static void flush() {
 
     // The accessor in which this logger is temporarily set.
     final LazyLoggerAccessor holder;
+    // tests whether the logger is invoked by the loading thread before
+    // the LoggerFinder is loaded; can be null;
+    final BooleanSupplier isLoadingThread;
+
+    // returns true if the logger is invoked by the loading thread before the
+    // LoggerFinder service is loaded
+    boolean isLoadingThread() {
+        return isLoadingThread != null && isLoadingThread.getAsBoolean();
+    }
 
-    BootstrapLogger(LazyLoggerAccessor holder) {
+    BootstrapLogger(LazyLoggerAccessor holder, BooleanSupplier isLoadingThread) {
         this.holder = holder;
+        this.isLoadingThread = isLoadingThread;
     }
 
     // Temporary data object storing log events
@@ -499,14 +508,15 @@ static LogEvent valueOf(BootstrapLogger bootstrap, PlatformLogger.Level level,
         static void log(LogEvent log, PlatformLogger.Bridge logger) {
             final SecurityManager sm = System.getSecurityManager();
             if (sm == null || log.acc == null) {
-                log.log(logger);
+                BootstrapExecutors.submit(() -> log.log(logger));
             } else {
                 // not sure we can actually use lambda here. We may need to create
                 // an anonymous class. Although if we reach here, then it means
                 // the VM is booted.
-                AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
-                    log.log(logger); return null;
-                }, log.acc);
+                BootstrapExecutors.submit(() ->
+                    AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+                        log.log(logger); return null;
+                }, log.acc));
             }
         }
 
@@ -553,8 +563,9 @@ public String getName() {
      * @return true if the VM is still bootstrapping.
      */
     boolean checkBootstrapping() {
-        if (isBooted()) {
+        if (isBooted() && !isLoadingThread()) {
             BootstrapExecutors.flush();
+            holder.getConcreteLogger(this);
             return false;
         }
         return true;
@@ -928,20 +939,26 @@ private static  boolean useSurrogateLoggers() {
     //    - the logging backend is a custom backend
     //    - the logging backend is JUL, there is no custom config,
     //      and the LogManager has not been initialized yet.
-    public static synchronized boolean useLazyLoggers() {
-        return !BootstrapLogger.isBooted()
-                || DetectBackend.detectedBackend == LoggingBackend.CUSTOM
-                || useSurrogateLoggers();
+    public static boolean useLazyLoggers() {
+        // Note: avoid triggering the initialization of the DetectBackend class
+        // while holding the BootstrapLogger class monitor
+        if (!BootstrapLogger.isBooted() ||
+                DetectBackend.detectedBackend == LoggingBackend.CUSTOM) {
+            return true;
+        }
+        synchronized (BootstrapLogger.class) {
+            return useSurrogateLoggers();
+        }
     }
 
     // Called by LazyLoggerAccessor. This method will determine whether
     // to create a BootstrapLogger (if the VM is not yet booted),
     // a SurrogateLogger (if JUL is the default backend and there
     // is no custom JUL configuration and LogManager is not yet initialized),
     // or a logger returned by the loaded LoggerFinder (all other cases).
-    static Logger getLogger(LazyLoggerAccessor accessor) {
-        if (!BootstrapLogger.isBooted()) {
-            return new BootstrapLogger(accessor);
+    static Logger getLogger(LazyLoggerAccessor accessor, BooleanSupplier isLoading) {
+        if (!BootstrapLogger.isBooted() || isLoading != null && isLoading.getAsBoolean()) {
+            return new BootstrapLogger(accessor, isLoading);
         } else {
             if (useSurrogateLoggers()) {
                 // JUL is the default backend, there is no custom configuration,
@@ -957,6 +974,12 @@ static Logger getLogger(LazyLoggerAccessor accessor) {
         }
     }
 
+    // trigger class initialization outside of holding lock
+    static void ensureBackendDetected() {
+        assert VM.isBooted() : "VM is not booted";
+        // triggers detection of the backend
+        var backend = DetectBackend.detectedBackend;
+    }
 
     // If the backend is JUL, and there is no custom configuration, and
     // nobody has attempted to call LogManager.getLogManager() yet, then

src/java.base/share/classes/jdk/internal/logger/LazyLoggers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, 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
@@ -32,6 +32,9 @@
 import java.lang.System.Logger;
 import java.lang.ref.WeakReference;
 import java.util.Objects;
+import java.util.function.BooleanSupplier;
+
+import jdk.internal.logger.LoggerFinderLoader.TemporaryLoggerFinder;
 import jdk.internal.misc.VM;
 import sun.util.logging.PlatformLogger;
 
@@ -110,6 +113,9 @@ static final class LazyLoggerAccessor implements LoggerAccessor {
         // We need to pass the actual caller module when creating the logger.
         private final WeakReference<Module> moduleRef;
 
+        // whether this is the loading thread, can be null
+        private final BooleanSupplier isLoadingThread;
+
         // The name of the logger that will be created lazyly
         final String name;
         // The plain logger SPI object - null until it is accessed for the
@@ -122,16 +128,24 @@ static final class LazyLoggerAccessor implements LoggerAccessor {
         private LazyLoggerAccessor(String name,
                                    LazyLoggerFactories<? extends Logger> factories,
                                    Module module) {
+            this(name, factories, module, null);
+        }
+
+        private LazyLoggerAccessor(String name,
+                                   LazyLoggerFactories<? extends Logger> factories,
+                                   Module module, BooleanSupplier isLoading) {
+
             this(Objects.requireNonNull(name), Objects.requireNonNull(factories),
-                    Objects.requireNonNull(module), null);
+                    Objects.requireNonNull(module), isLoading, null);
         }
 
         private LazyLoggerAccessor(String name,
                                    LazyLoggerFactories<? extends Logger> factories,
-                                   Module module, Void unused) {
+                                   Module module, BooleanSupplier isLoading, Void unused) {
             this.name = name;
             this.factories = factories;
             this.moduleRef = new WeakReference<>(module);
+            this.isLoadingThread = isLoading;
         }
 
         /**
@@ -162,7 +176,7 @@ public Logger wrapped() {
             // BootstrapLogger has the logic to decide whether to invoke the
             // SPI or use a temporary (BootstrapLogger or SimpleConsoleLogger)
             // logger.
-            wrapped = BootstrapLogger.getLogger(this);
+            wrapped = BootstrapLogger.getLogger(this, isLoadingThread);
             synchronized(this) {
                 // if w has already been in between, simply drop 'wrapped'.
                 setWrappedIfNotSet(wrapped);
@@ -194,7 +208,7 @@ public PlatformLogger.Bridge platform() {
             // BootstrapLogger has the logic to decide whether to invoke the
             // SPI or use a temporary (BootstrapLogger or SimpleConsoleLogger)
             // logger.
-            final Logger wrapped = BootstrapLogger.getLogger(this);
+            final Logger wrapped = BootstrapLogger.getLogger(this, isLoadingThread);
             synchronized(this) {
                 // if w has already been set, simply drop 'wrapped'.
                 setWrappedIfNotSet(wrapped);
@@ -282,10 +296,10 @@ Logger createLogger() {
          * Creates a new lazy logger accessor for the named logger. The given
          * factories will be use when it becomes necessary to actually create
          * the logger.
-         * @param <T> An interface that extends {@link Logger}.
          * @param name The logger name.
          * @param factories The factories that should be used to create the
          *                  wrapped logger.
+         * @param module The module for which the logger is being created
          * @return A new LazyLoggerAccessor.
          */
         public static LazyLoggerAccessor makeAccessor(String name,
@@ -339,6 +353,7 @@ private static LoggerFinder accessLoggerFinder() {
             prov = sm == null ? LoggerFinder.getLoggerFinder() :
                 AccessController.doPrivileged(
                         (PrivilegedAction<LoggerFinder>)LoggerFinder::getLoggerFinder);
+            if (prov instanceof TemporaryLoggerFinder) return prov;
             provider = prov;
         }
         return prov;
@@ -358,7 +373,6 @@ public Logger apply(String name, Module module) {
            new LazyLoggerFactories<>(loggerSupplier);
 
 
-
     // A concrete implementation of Logger that delegates to a  System.Logger,
     // but only creates the System.Logger instance lazily when it's used for
     // the first time.
@@ -376,6 +390,11 @@ private JdkLazyLogger(LazyLoggerAccessor holder, Void unused) {
         }
     }
 
+    static Logger makeLazyLogger(String name, Module module, BooleanSupplier isLoading) {
+        final LazyLoggerAccessor holder = new LazyLoggerAccessor(name, factories, module, isLoading);
+        return new JdkLazyLogger(holder, null);
+    }
+
     /**
      * Gets a logger from the LoggerFinder. Creates the actual concrete
      * logger.

src/java.base/share/classes/jdk/internal/logger/LoggerFinderLoader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, 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
@@ -25,13 +25,18 @@
 package jdk.internal.logger;
 
 import java.io.FilePermission;
+import java.lang.System.Logger;
+import java.lang.System.LoggerFinder;
 import java.security.AccessController;
 import java.security.Permission;
 import java.security.PrivilegedAction;
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
+import java.util.function.BooleanSupplier;
+
+import jdk.internal.vm.annotation.Stable;
 import sun.security.util.SecurityConstants;
 import sun.security.action.GetPropertyAction;
 
@@ -64,20 +69,41 @@ private LoggerFinderLoader() {
         throw new InternalError("LoggerFinderLoader cannot be instantiated");
     }
 
-
+    // record the loadingThread while loading the backend
+    static volatile Thread loadingThread;
     // Return the loaded LoggerFinder, or load it if not already loaded.
     private static System.LoggerFinder service() {
         if (service != null) return service;
+        // ensure backend is detected before attempting to load the finder
+        BootstrapLogger.ensureBackendDetected();
         synchronized(lock) {
             if (service != null) return service;
-            service = loadLoggerFinder();
+            Thread currentThread = Thread.currentThread();
+            if (loadingThread == currentThread) {
+                // recursive attempt to load the backend while loading the backend
+                // use a temporary logger finder that returns special BootstrapLogger
+                // which will wait until loading is finished
+                return TemporaryLoggerFinder.INSTANCE;
+            }
+            loadingThread = currentThread;
+            try {
+                service = loadLoggerFinder();
+            } finally {
+                loadingThread = null;
+            }
         }
         // Since the LoggerFinder is already loaded - we can stop using
         // temporary loggers.
         BootstrapLogger.redirectTemporaryLoggers();
         return service;
     }
 
+    // returns true if called by the thread that loads the LoggerFinder, while
+    // loading the LoggerFinder.
+    static boolean isLoadingThread() {
+        return loadingThread != null && loadingThread == Thread.currentThread();
+    }
+
     // Get configuration error policy
     private static ErrorPolicy configurationErrorPolicy() {
         String errorPolicy =
@@ -115,6 +141,34 @@ private static Iterator<System.LoggerFinder> findLoggerFinderProviders() {
         return iterator;
     }
 
+    public static final class TemporaryLoggerFinder extends LoggerFinder {
+        private TemporaryLoggerFinder() {}
+        @Stable
+        private LoggerFinder loadedService;
+
+        private static final BooleanSupplier isLoadingThread = new BooleanSupplier() {
+            @Override
+            public boolean getAsBoolean() {
+                return LoggerFinderLoader.isLoadingThread();
+            }
+        };
+        private static final TemporaryLoggerFinder INSTANCE = new TemporaryLoggerFinder();
+
+        @Override
+        public Logger getLogger(String name, Module module) {
+            if (loadedService == null) {
+                loadedService = service;
+                if (loadedService == null) {
+                    return LazyLoggers.makeLazyLogger(name, module, isLoadingThread);
+                }
+            }
+            assert loadedService != null;
+            assert !LoggerFinderLoader.isLoadingThread();
+            assert loadedService != this;
+            return LazyLoggers.getLogger(name, module);
+        }
+    }
+
     // Loads the LoggerFinder using ServiceLoader. If no LoggerFinder
     // is found returns the default (possibly JUL based) implementation
     private static System.LoggerFinder loadLoggerFinder() {

test/jdk/java/lang/System/LoggerFinder/RecursiveLoading/META-INF/services/java.lang.System$LoggerFinder

@@ -0,0 +1 @@
+loggerfinder.SimpleLoggerFinder