Skip to content

Commit

Permalink
8314263: Signed jars triggering Logger finder recursion and StackOver…
Browse files Browse the repository at this point in the history
…flowError

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

Reviewed-by: lucy
Backport-of: 7daae1fb4267f92b38f0152611d69b7b89691087
  • Loading branch information
Andrew Lu authored and RealLucy committed Nov 9, 2023
1 parent 416c48e commit c54521b
Show file tree
Hide file tree
Showing 17 changed files with 1,074 additions and 40 deletions.
10 changes: 7 additions & 3 deletions src/java.base/share/classes/java/lang/System.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

}
Expand Down
@@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
33 changes: 26 additions & 7 deletions 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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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.
Expand Down
@@ -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
Expand All @@ -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;

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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() {
Expand Down
@@ -0,0 +1 @@
loggerfinder.SimpleLoggerFinder

1 comment on commit c54521b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.