RFR: JDK-8046565: Platform Logger API and Service

Mandy Chung mandy.chung at oracle.com
Tue Oct 20 07:10:38 UTC 2015


> On Oct 19, 2015, at 11:18 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/8046565/proto.02/webrev/index.html

Looks good in general.  Some minor comments:

Good to see limited privileges being used that limits the permissions when invoking logger finder which is good.

LoggerFinderLoader.java

 110                     (PrivilegedAction<ServiceLoader<DefaultLoggerFinder>>)

- can this use diamond operation (PrivilegedAction<>)?

System.java
1547         return LazyLoggers.getLogger(Objects.requireNonNull(name), caller);

Objects.requireNonNull(name) - already called.   It can just do getLogger(name, caller).

RuntimePermission.java
360  *   to JDK classes.</td>

s/JDK/system/

LazyLoggers.java

sun/management/ManagementFactoryHelper.java

 172     public interface LoggingMXBean
 173         extends PlatformLoggingMXBean, java.util.logging.LoggingMXBean {
 174     }

LazyLoggers.java
  68             this(Objects.requireNonNull(loggerSupplier),
  69                  (Void)null);

 130             this(Objects.requireNonNull(name), Objects.requireNonNull(factories),
 131                     Objects.requireNonNull(caller), null);

I would expect Objects.requireNonNull on the constructor initializes all the fields.  But the above lines checks non-null before calling this(….).  Does the other constructor allow null parameters?  Why not moving Objects.requireNonNull to the other constructor?

 365            new BiFunction<String, Class<?>, Logger>() {
- it can use diamond operator

BootstrapLogger.DetectBackend looks like have some duplication as LoggerFinderLoader.
- does it make sense to merge them?

 923         boolean cleared;
- should that be volatile field?

AbstractLoggerWrapper.java

 212             if (sourceClass==null && sourceMethod==null) {
 234             if (sourceClass==null && sourceMethod==null) {

Missing space around “==“ and also in a few other lines.

java.util.logging.LogManager.demandLoggerFor
- I think this method is intended for the LoggerFinder provider to use.  Perhaps we should add a note in the javadoc to say that.

SimpleConsoleLogger.skipLoggingFrame and another place is finding the caller information.  JEP 259 may be useful for stack walking with filtering.

I skimmed through the tests and nothing pops up.  There are multiple copies of AccessSystemLogger that can be shared as I comment last time.

Mandy




More information about the core-libs-dev mailing list