Skip to content

Commit 0e02204

Browse files
GoeLinGoeLin
committed
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
1 parent 069743d commit 0e02204

File tree

17 files changed

+1010
-40
lines changed

17 files changed

+1010
-40
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import java.util.function.Supplier;
6767
import java.util.concurrent.ConcurrentHashMap;
6868
import java.util.stream.Stream;
69+
import jdk.internal.logger.LoggerFinderLoader.TemporaryLoggerFinder;
6970
import jdk.internal.misc.Unsafe;
7071
import jdk.internal.util.StaticProperty;
7172
import jdk.internal.module.ModuleBootstrap;
@@ -1713,13 +1714,16 @@ static LoggerFinder accessProvider() {
17131714
// We do not need to synchronize: LoggerFinderLoader will
17141715
// always return the same instance, so if we don't have it,
17151716
// just fetch it again.
1716-
if (service == null) {
1717+
LoggerFinder finder = service;
1718+
if (finder == null) {
17171719
PrivilegedAction<LoggerFinder> pa =
17181720
() -> LoggerFinderLoader.getLoggerFinder();
1719-
service = AccessController.doPrivileged(pa, null,
1721+
finder = AccessController.doPrivileged(pa, null,
17201722
LOGGERFINDER_PERMISSION);
1723+
if (finder instanceof TemporaryLoggerFinder) return finder;
1724+
service = finder;
17211725
}
1722-
return service;
1726+
return finder;
17231727
}
17241728

17251729
}

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

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -38,7 +38,6 @@
3838
import java.util.function.Supplier;
3939
import java.lang.System.LoggerFinder;
4040
import java.lang.System.Logger;
41-
import java.lang.System.Logger.Level;
4241
import java.lang.ref.WeakReference;
4342
import java.util.Objects;
4443
import java.util.concurrent.ExecutionException;
@@ -227,9 +226,19 @@ static void flush() {
227226

228227
// The accessor in which this logger is temporarily set.
229228
final LazyLoggerAccessor holder;
229+
// tests whether the logger is invoked by the loading thread before
230+
// the LoggerFinder is loaded; can be null;
231+
final BooleanSupplier isLoadingThread;
232+
233+
// returns true if the logger is invoked by the loading thread before the
234+
// LoggerFinder service is loaded
235+
boolean isLoadingThread() {
236+
return isLoadingThread != null && isLoadingThread.getAsBoolean();
237+
}
230238

231-
BootstrapLogger(LazyLoggerAccessor holder) {
239+
BootstrapLogger(LazyLoggerAccessor holder, BooleanSupplier isLoadingThread) {
232240
this.holder = holder;
241+
this.isLoadingThread = isLoadingThread;
233242
}
234243

235244
// Temporary data object storing log events
@@ -505,14 +514,15 @@ static LogEvent valueOf(BootstrapLogger bootstrap, PlatformLogger.Level level,
505514
static void log(LogEvent log, PlatformLogger.Bridge logger) {
506515
final SecurityManager sm = System.getSecurityManager();
507516
if (sm == null || log.acc == null) {
508-
log.log(logger);
517+
BootstrapExecutors.submit(() -> log.log(logger));
509518
} else {
510519
// not sure we can actually use lambda here. We may need to create
511520
// an anonymous class. Although if we reach here, then it means
512521
// the VM is booted.
513-
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
514-
log.log(logger); return null;
515-
}, log.acc);
522+
BootstrapExecutors.submit(() ->
523+
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
524+
log.log(logger); return null;
525+
}, log.acc));
516526
}
517527
}
518528

@@ -559,8 +569,9 @@ public String getName() {
559569
* @return true if the VM is still bootstrapping.
560570
*/
561571
boolean checkBootstrapping() {
562-
if (isBooted()) {
572+
if (isBooted() && !isLoadingThread()) {
563573
BootstrapExecutors.flush();
574+
holder.getConcreteLogger(this);
564575
return false;
565576
}
566577
return true;
@@ -935,20 +946,26 @@ private static boolean useSurrogateLoggers() {
935946
// - the logging backend is a custom backend
936947
// - the logging backend is JUL, there is no custom config,
937948
// and the LogManager has not been initialized yet.
938-
public static synchronized boolean useLazyLoggers() {
939-
return !BootstrapLogger.isBooted()
940-
|| DetectBackend.detectedBackend == LoggingBackend.CUSTOM
941-
|| useSurrogateLoggers();
949+
public static boolean useLazyLoggers() {
950+
// Note: avoid triggering the initialization of the DetectBackend class
951+
// while holding the BootstrapLogger class monitor
952+
if (!BootstrapLogger.isBooted() ||
953+
DetectBackend.detectedBackend == LoggingBackend.CUSTOM) {
954+
return true;
955+
}
956+
synchronized (BootstrapLogger.class) {
957+
return useSurrogateLoggers();
958+
}
942959
}
943960

944961
// Called by LazyLoggerAccessor. This method will determine whether
945962
// to create a BootstrapLogger (if the VM is not yet booted),
946963
// a SurrogateLogger (if JUL is the default backend and there
947964
// is no custom JUL configuration and LogManager is not yet initialized),
948965
// or a logger returned by the loaded LoggerFinder (all other cases).
949-
static Logger getLogger(LazyLoggerAccessor accessor) {
950-
if (!BootstrapLogger.isBooted()) {
951-
return new BootstrapLogger(accessor);
966+
static Logger getLogger(LazyLoggerAccessor accessor, BooleanSupplier isLoading) {
967+
if (!BootstrapLogger.isBooted() || isLoading != null && isLoading.getAsBoolean()) {
968+
return new BootstrapLogger(accessor, isLoading);
952969
} else {
953970
if (useSurrogateLoggers()) {
954971
// JUL is the default backend, there is no custom configuration,
@@ -964,6 +981,12 @@ static Logger getLogger(LazyLoggerAccessor accessor) {
964981
}
965982
}
966983

984+
// trigger class initialization outside of holding lock
985+
static void ensureBackendDetected() {
986+
assert VM.isBooted() : "VM is not booted";
987+
// triggers detection of the backend
988+
var backend = DetectBackend.detectedBackend;
989+
}
967990

968991
// If the backend is JUL, and there is no custom configuration, and
969992
// nobody has attempted to call LogManager.getLogManager() yet, then

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,9 @@
3232
import java.lang.System.Logger;
3333
import java.lang.ref.WeakReference;
3434
import java.util.Objects;
35+
import java.util.function.BooleanSupplier;
36+
37+
import jdk.internal.logger.LoggerFinderLoader.TemporaryLoggerFinder;
3538
import jdk.internal.misc.VM;
3639
import sun.util.logging.PlatformLogger;
3740

@@ -110,6 +113,9 @@ static final class LazyLoggerAccessor implements LoggerAccessor {
110113
// We need to pass the actual caller module when creating the logger.
111114
private final WeakReference<Module> moduleRef;
112115

116+
// whether this is the loading thread, can be null
117+
private final BooleanSupplier isLoadingThread;
118+
113119
// The name of the logger that will be created lazyly
114120
final String name;
115121
// The plain logger SPI object - null until it is accessed for the
@@ -122,16 +128,24 @@ static final class LazyLoggerAccessor implements LoggerAccessor {
122128
private LazyLoggerAccessor(String name,
123129
LazyLoggerFactories<? extends Logger> factories,
124130
Module module) {
131+
this(name, factories, module, null);
132+
}
133+
134+
private LazyLoggerAccessor(String name,
135+
LazyLoggerFactories<? extends Logger> factories,
136+
Module module, BooleanSupplier isLoading) {
137+
125138
this(Objects.requireNonNull(name), Objects.requireNonNull(factories),
126-
Objects.requireNonNull(module), null);
139+
Objects.requireNonNull(module), isLoading, null);
127140
}
128141

129142
private LazyLoggerAccessor(String name,
130143
LazyLoggerFactories<? extends Logger> factories,
131-
Module module, Void unused) {
144+
Module module, BooleanSupplier isLoading, Void unused) {
132145
this.name = name;
133146
this.factories = factories;
134147
this.moduleRef = new WeakReference<>(module);
148+
this.isLoadingThread = isLoading;
135149
}
136150

137151
/**
@@ -162,7 +176,7 @@ public Logger wrapped() {
162176
// BootstrapLogger has the logic to decide whether to invoke the
163177
// SPI or use a temporary (BootstrapLogger or SimpleConsoleLogger)
164178
// logger.
165-
wrapped = BootstrapLogger.getLogger(this);
179+
wrapped = BootstrapLogger.getLogger(this, isLoadingThread);
166180
synchronized(this) {
167181
// if w has already been in between, simply drop 'wrapped'.
168182
setWrappedIfNotSet(wrapped);
@@ -194,7 +208,7 @@ public PlatformLogger.Bridge platform() {
194208
// BootstrapLogger has the logic to decide whether to invoke the
195209
// SPI or use a temporary (BootstrapLogger or SimpleConsoleLogger)
196210
// logger.
197-
final Logger wrapped = BootstrapLogger.getLogger(this);
211+
final Logger wrapped = BootstrapLogger.getLogger(this, isLoadingThread);
198212
synchronized(this) {
199213
// if w has already been set, simply drop 'wrapped'.
200214
setWrappedIfNotSet(wrapped);
@@ -282,10 +296,10 @@ Logger createLogger() {
282296
* Creates a new lazy logger accessor for the named logger. The given
283297
* factories will be use when it becomes necessary to actually create
284298
* the logger.
285-
* @param <T> An interface that extends {@link Logger}.
286299
* @param name The logger name.
287300
* @param factories The factories that should be used to create the
288301
* wrapped logger.
302+
* @param module The module for which the logger is being created
289303
* @return A new LazyLoggerAccessor.
290304
*/
291305
public static LazyLoggerAccessor makeAccessor(String name,
@@ -340,6 +354,7 @@ private static LoggerFinder accessLoggerFinder() {
340354
prov = sm == null ? LoggerFinder.getLoggerFinder() :
341355
AccessController.doPrivileged(
342356
(PrivilegedAction<LoggerFinder>)LoggerFinder::getLoggerFinder);
357+
if (prov instanceof TemporaryLoggerFinder) return prov;
343358
provider = prov;
344359
}
345360
return prov;
@@ -359,7 +374,6 @@ public Logger apply(String name, Module module) {
359374
new LazyLoggerFactories<>(loggerSupplier);
360375

361376

362-
363377
// A concrete implementation of Logger that delegates to a System.Logger,
364378
// but only creates the System.Logger instance lazily when it's used for
365379
// the first time.
@@ -377,6 +391,11 @@ private JdkLazyLogger(LazyLoggerAccessor holder, Void unused) {
377391
}
378392
}
379393

394+
static Logger makeLazyLogger(String name, Module module, BooleanSupplier isLoading) {
395+
final LazyLoggerAccessor holder = new LazyLoggerAccessor(name, factories, module, isLoading);
396+
return new JdkLazyLogger(holder, null);
397+
}
398+
380399
/**
381400
* Gets a logger from the LoggerFinder. Creates the actual concrete
382401
* logger.

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

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -25,13 +25,18 @@
2525
package jdk.internal.logger;
2626

2727
import java.io.FilePermission;
28+
import java.lang.System.Logger;
29+
import java.lang.System.LoggerFinder;
2830
import java.security.AccessController;
2931
import java.security.Permission;
3032
import java.security.PrivilegedAction;
3133
import java.util.Iterator;
3234
import java.util.Locale;
3335
import java.util.ServiceConfigurationError;
3436
import java.util.ServiceLoader;
37+
import java.util.function.BooleanSupplier;
38+
39+
import jdk.internal.vm.annotation.Stable;
3540
import sun.security.util.SecurityConstants;
3641
import sun.security.action.GetPropertyAction;
3742

@@ -64,20 +69,41 @@ private LoggerFinderLoader() {
6469
throw new InternalError("LoggerFinderLoader cannot be instantiated");
6570
}
6671

67-
72+
// record the loadingThread while loading the backend
73+
static volatile Thread loadingThread;
6874
// Return the loaded LoggerFinder, or load it if not already loaded.
6975
private static System.LoggerFinder service() {
7076
if (service != null) return service;
77+
// ensure backend is detected before attempting to load the finder
78+
BootstrapLogger.ensureBackendDetected();
7179
synchronized(lock) {
7280
if (service != null) return service;
73-
service = loadLoggerFinder();
81+
Thread currentThread = Thread.currentThread();
82+
if (loadingThread == currentThread) {
83+
// recursive attempt to load the backend while loading the backend
84+
// use a temporary logger finder that returns special BootstrapLogger
85+
// which will wait until loading is finished
86+
return TemporaryLoggerFinder.INSTANCE;
87+
}
88+
loadingThread = currentThread;
89+
try {
90+
service = loadLoggerFinder();
91+
} finally {
92+
loadingThread = null;
93+
}
7494
}
7595
// Since the LoggerFinder is already loaded - we can stop using
7696
// temporary loggers.
7797
BootstrapLogger.redirectTemporaryLoggers();
7898
return service;
7999
}
80100

101+
// returns true if called by the thread that loads the LoggerFinder, while
102+
// loading the LoggerFinder.
103+
static boolean isLoadingThread() {
104+
return loadingThread != null && loadingThread == Thread.currentThread();
105+
}
106+
81107
// Get configuration error policy
82108
private static ErrorPolicy configurationErrorPolicy() {
83109
String errorPolicy =
@@ -116,6 +142,34 @@ private static Iterator<System.LoggerFinder> findLoggerFinderProviders() {
116142
return iterator;
117143
}
118144

145+
public static final class TemporaryLoggerFinder extends LoggerFinder {
146+
private TemporaryLoggerFinder() {}
147+
@Stable
148+
private LoggerFinder loadedService;
149+
150+
private static final BooleanSupplier isLoadingThread = new BooleanSupplier() {
151+
@Override
152+
public boolean getAsBoolean() {
153+
return LoggerFinderLoader.isLoadingThread();
154+
}
155+
};
156+
private static final TemporaryLoggerFinder INSTANCE = new TemporaryLoggerFinder();
157+
158+
@Override
159+
public Logger getLogger(String name, Module module) {
160+
if (loadedService == null) {
161+
loadedService = service;
162+
if (loadedService == null) {
163+
return LazyLoggers.makeLazyLogger(name, module, isLoadingThread);
164+
}
165+
}
166+
assert loadedService != null;
167+
assert !LoggerFinderLoader.isLoadingThread();
168+
assert loadedService != this;
169+
return LazyLoggers.getLogger(name, module);
170+
}
171+
}
172+
119173
// Loads the LoggerFinder using ServiceLoader. If no LoggerFinder
120174
// is found returns the default (possibly JUL based) implementation
121175
private static System.LoggerFinder loadLoggerFinder() {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
loggerfinder.SimpleLoggerFinder

0 commit comments

Comments
 (0)