From e59795ad1f3120c1dc3a742d5a2a722fe7827b96 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 15 Aug 2024 08:44:05 -0400 Subject: [PATCH] Use a supplier instead of testing twice is internal logging is enabled - Only check once if logging is enabled in other call sites since logDiagnostic(String) already checks - Less string building - Use longer lines - Whitespace - Remove unused private constants --- .../apache/commons/logging/LogFactory.java | 298 +++++++----------- 1 file changed, 115 insertions(+), 183 deletions(-) diff --git a/src/main/java/org/apache/commons/logging/LogFactory.java b/src/main/java/org/apache/commons/logging/LogFactory.java index 74e0661..5b09c22 100644 --- a/src/main/java/org/apache/commons/logging/LogFactory.java +++ b/src/main/java/org/apache/commons/logging/LogFactory.java @@ -34,6 +34,7 @@ import java.util.Objects; import java.util.Properties; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; +import java.util.function.Supplier; /** * Factory for creating {@link Log} instances, with discovery and @@ -94,11 +95,10 @@ public abstract class LogFactory { public static final String FACTORY_PROPERTY = "org.apache.commons.logging.LogFactory"; private static final String FACTORY_LOG4J_API = "org.apache.commons.logging.impl.Log4jApiLogFactory"; - private static final String LOG4J_API_LOGGER = "org.apache.logging.log4j.Logger"; + private static final String LOG4J_TO_SLF4J_BRIDGE = "org.apache.logging.slf4j.SLF4JProvider"; private static final String FACTORY_SLF4J = "org.apache.commons.logging.impl.Slf4jLogFactory"; - private static final String SLF4J_API_LOGGER = "org.slf4j.Logger"; /** * The fully qualified class name of the fallback {@code LogFactory} @@ -115,8 +115,7 @@ public abstract class LogFactory { * JDK 1.3+ * 'Service Provider' specification. */ - protected static final String SERVICE_ID = - "META-INF/services/org.apache.commons.logging.LogFactory"; + protected static final String SERVICE_ID = "META-INF/services/org.apache.commons.logging.LogFactory"; /** * The name ({@code org.apache.commons.logging.diagnostics.dest}) @@ -133,8 +132,7 @@ public abstract class LogFactory { * Diagnostic logging should be used only to debug problematic * configurations and should not be set in normal production use. */ - public static final String DIAGNOSTICS_DEST_PROPERTY = - "org.apache.commons.logging.diagnostics.dest"; + public static final String DIAGNOSTICS_DEST_PROPERTY = "org.apache.commons.logging.diagnostics.dest"; /** * When null (the usual case), no diagnostic output will be @@ -148,7 +146,7 @@ public abstract class LogFactory { * logDiagnostic method, so that users can clearly see which * LogFactory class is generating the output. */ - private static final String diagnosticPrefix; + private static final String DIAGNOSTICS_PREFIX; /** * Setting this system property @@ -189,12 +187,10 @@ public abstract class LogFactory { * the need to release them (on 1.3+ JVMs only, of course ;). *

*/ - public static final String HASHTABLE_IMPLEMENTATION_PROPERTY = - "org.apache.commons.logging.LogFactory.HashtableImpl"; + public static final String HASHTABLE_IMPLEMENTATION_PROPERTY = "org.apache.commons.logging.LogFactory.HashtableImpl"; /** Name used to load the weak hash table implementation by names. */ - private static final String WEAK_HASHTABLE_CLASSNAME = - "org.apache.commons.logging.impl.WeakHashtable"; + private static final String WEAK_HASHTABLE_CLASSNAME = "org.apache.commons.logging.impl.WeakHashtable"; /** * A reference to the class loader that loaded this class. This is the @@ -251,21 +247,15 @@ public abstract class LogFactory { // output diagnostics from this class are static. String classLoaderName; try { - if (thisClassLoader == null) { - classLoaderName = "BOOTLOADER"; - } else { - classLoaderName = objectId(thisClassLoader); - } + classLoaderName = thisClassLoader != null ? objectId(thisClassLoader) : "BOOTLOADER"; } catch (final SecurityException e) { classLoaderName = "UNKNOWN"; } - diagnosticPrefix = "[LogFactory from " + classLoaderName + "] "; + DIAGNOSTICS_PREFIX = "[LogFactory from " + classLoaderName + "] "; DIAGNOSTICS_STREAM = initDiagnostics(); logClassLoaderEnvironment(LogFactory.class); factories = createFactoryStore(); - if (isDiagnosticsEnabled()) { - logDiagnostic("BOOTSTRAP COMPLETED"); - } + logDiagnostic("BOOTSTRAP COMPLETED"); } /** @@ -280,7 +270,6 @@ public abstract class LogFactory { private static void cacheFactory(final ClassLoader classLoader, final LogFactory factory) { // Ideally we would assert(factory != null) here. However reporting // errors from within a logging implementation is a little tricky! - if (factory != null) { if (classLoader == null) { nullClassLoaderFactory = factory; @@ -359,41 +348,30 @@ public abstract class LogFactory { // tries again with thisClassLoaderRef, because we've just tried // loading with that loader (not the TCCL). Just throw an // appropriate exception here. - final boolean implementsLogFactory = implementsLogFactory(logFactoryClass); - // // Construct a good message: users may not actual expect that a custom implementation // has been specified. Several well known containers use this mechanism to adapt JCL // to their native logging system. // final StringBuilder msg = new StringBuilder(); - msg.append("The application has specified that a custom LogFactory implementation "); - msg.append("should be used but Class '"); + msg.append("The application has specified that a custom LogFactory implementation should be used but Class '"); msg.append(factoryClassName); msg.append("' cannot be converted to '"); msg.append(LogFactory.class.getName()); msg.append("'. "); if (implementsLogFactory) { - msg.append("The conflict is caused by the presence of multiple LogFactory classes "); - msg.append("in incompatible class loaders. "); - msg.append("Background can be found in https://commons.apache.org/logging/tech.html. "); - msg.append("If you have not explicitly specified a custom LogFactory then it is likely "); - msg.append("that the container has set one without your knowledge. "); - msg.append("In this case, consider using the commons-logging-adapters.jar file or "); - msg.append("specifying the standard LogFactory from the command line. "); + msg.append("The conflict is caused by the presence of multiple LogFactory classes in incompatible class loaders. Background can"); + msg.append(" be found in https://commons.apache.org/logging/tech.html. If you have not explicitly specified a custom LogFactory"); + msg.append(" then it is likely that the container has set one without your knowledge. In this case, consider using the "); + msg.append("commons-logging-adapters.jar file or specifying the standard LogFactory from the command line. "); } else { msg.append("Please check the custom implementation. "); } msg.append("Help can be found at https://commons.apache.org/logging/troubleshooting.html."); - - if (isDiagnosticsEnabled()) { - logDiagnostic(msg.toString()); - } - + logDiagnostic(msg.toString()); throw new ClassCastException(msg.toString()); } - // Ignore exception, continue. Presumably the class loader was the // TCCL; the code below will try to load the class via thisClassLoaderRef. // This will handle the case where the original calling class is in @@ -433,8 +411,7 @@ public abstract class LogFactory { logDiagnostic("Unable to create LogFactory instance."); } if (logFactoryClass != null && !LogFactory.class.isAssignableFrom(logFactoryClass)) { - return new LogConfigurationException("The chosen LogFactory implementation does not extend LogFactory." + " Please check your configuration.", - e); + return new LogConfigurationException("The chosen LogFactory implementation does not extend LogFactory. Please check your configuration.", e); } return new LogConfigurationException(e); } @@ -466,7 +443,6 @@ public abstract class LogFactory { // weak hash table implementation if it is available. storeImplementationClass = null; } - if (storeImplementationClass == null) { storeImplementationClass = WEAK_HASHTABLE_CLASSNAME; } @@ -476,7 +452,6 @@ public abstract class LogFactory { result = implementationClass.getConstructor().newInstance(); } catch (final Throwable t) { handleThrowable(t); // may re-throw t - // ignore if (!WEAK_HASHTABLE_CLASSNAME.equals(storeImplementationClass)) { // if the user's trying to set up a custom implementation, give a clue @@ -598,9 +573,7 @@ public abstract class LogFactory { try { return clazz.getClassLoader(); } catch (final SecurityException ex) { - if (isDiagnosticsEnabled()) { - logDiagnostic("Unable to get class loader for class '" + clazz + "' due to security restrictions - " + ex.getMessage()); - } + logDiagnostic(() -> "Unable to get class loader for class '" + clazz + "' due to security restrictions - " + ex.getMessage()); throw ex; } } @@ -633,14 +606,11 @@ public abstract class LogFactory { URL propsUrl = null; try { final Enumeration urls = getResources(classLoader, fileName); - if (urls == null) { return null; } - while (urls.hasMoreElements()) { final URL url = urls.nextElement(); - final Properties newProps = getProperties(url); if (newProps != null) { if (props == null) { @@ -651,7 +621,6 @@ public abstract class LogFactory { if (priorityStr != null) { priority = Double.parseDouble(priorityStr); } - if (isDiagnosticsEnabled()) { logDiagnostic("[LOOKUP] Properties file found at '" + url + "'" + " with priority " + priority); } @@ -661,13 +630,11 @@ public abstract class LogFactory { if (newPriorityStr != null) { newPriority = Double.parseDouble(newPriorityStr); } - if (newPriority > priority) { if (isDiagnosticsEnabled()) { logDiagnostic("[LOOKUP] Properties file at '" + url + "'" + " with priority " + newPriority + " overrides file at '" + propsUrl + "'" + " with priority " + priority); } - propsUrl = url; props = newProps; priority = newPriority; @@ -680,11 +647,8 @@ public abstract class LogFactory { } } } catch (final SecurityException e) { - if (isDiagnosticsEnabled()) { - logDiagnostic("SecurityException thrown while trying to find/read config files."); - } + logDiagnostic("SecurityException thrown while trying to find/read config files."); } - if (isDiagnosticsEnabled()) { if (props == null) { logDiagnostic("[LOOKUP] No properties file of name '" + fileName + "' found."); @@ -692,7 +656,6 @@ public abstract class LogFactory { logDiagnostic("[LOOKUP] Properties file of name '" + fileName + "' found at '" + propsUrl + '"'); } } - return props; } @@ -766,7 +729,7 @@ public abstract class LogFactory { // This is an odd enough situation to report about. This // output will be a nuisance on JDK1.1, as the system // class loader is null in that environment. - if (contextClassLoader == null && isDiagnosticsEnabled()) { + if (contextClassLoader == null) { logDiagnostic("Context class loader is null."); } @@ -805,17 +768,16 @@ public abstract class LogFactory { // If TCCL is still enabled at this point, we check if it resolves this class if (useTccl) { try { - if (!LogFactory.class.equals( - Class.forName(LogFactory.class.getName(), false, contextClassLoader))) { - logDiagnostic("The class " + LogFactory.class.getName() + " loaded by the context class loader " + objectId(contextClassLoader) + if (!LogFactory.class.equals(Class.forName(LogFactory.class.getName(), false, contextClassLoader))) { + logDiagnostic(() -> "The class " + LogFactory.class.getName() + " loaded by the context class loader " + objectId(contextClassLoader) + " and this class differ. Disabling the usage of the context class loader." + "Background can be found in https://commons.apache.org/logging/tech.html. "); logHierarchy("[BAD CL TREE] ", contextClassLoader); useTccl = false; } } catch (final ClassNotFoundException ignored) { - logDiagnostic("The class " + LogFactory.class.getName() + " is not present in the the context class loader " + objectId(contextClassLoader) - + ". Disabling the usage of the context class loader." + logDiagnostic(() -> "The class " + LogFactory.class.getName() + " is not present in the the context class loader " + + objectId(contextClassLoader) + ". Disabling the usage of the context class loader." + "Background can be found in https://commons.apache.org/logging/tech.html. "); logHierarchy("[BAD CL TREE] ", contextClassLoader); useTccl = false; @@ -825,28 +787,21 @@ public abstract class LogFactory { // Determine which concrete LogFactory subclass to use. // First, try a global system property - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] Looking for system property [" + FACTORY_PROPERTY + - "] to define the LogFactory subclass to use..."); - } + logDiagnostic(() -> "[LOOKUP] Looking for system property [" + FACTORY_PROPERTY + + "] to define the LogFactory subclass to use..."); try { final String factoryClass = getSystemProperty(FACTORY_PROPERTY, null); if (factoryClass != null) { - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] Creating an instance of LogFactory class '" + factoryClass + - "' as specified by system property " + FACTORY_PROPERTY); - } + logDiagnostic(() -> "[LOOKUP] Creating an instance of LogFactory class '" + factoryClass + + "' as specified by system property " + FACTORY_PROPERTY); factory = newFactory(factoryClass, baseClassLoader, contextClassLoader); - } else if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] No system property [" + FACTORY_PROPERTY + "] defined."); + } else { + logDiagnostic(() -> "[LOOKUP] No system property [" + FACTORY_PROPERTY + "] defined."); } } catch (final SecurityException e) { - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] A security exception occurred while trying to create an" + - " instance of the custom factory class" + ": [" + trim(e.getMessage()) + - "]. Trying alternative implementations..."); - } + logDiagnostic(() -> "[LOOKUP] A security exception occurred while trying to create an instance of the custom factory class" + ": [" + + trim(e.getMessage()) + "]. Trying alternative implementations..."); // ignore } catch (final RuntimeException e) { // This is not consistent with the behavior when a bad LogFactory class is @@ -854,25 +809,18 @@ public abstract class LogFactory { // // One possible exception that can occur here is a ClassCastException when // the specified class wasn't castable to this LogFactory type. - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] An exception occurred while trying to create an" + - " instance of the custom factory class" + ": [" + - trim(e.getMessage()) + - "] as specified by a system property."); - } + logDiagnostic(() -> "[LOOKUP] An exception occurred while trying to create an instance of the custom factory class: [" + trim(e.getMessage()) + + "] as specified by a system property."); throw e; } - + // // Second, try to find a service by using the JDK 1.3 class // discovery mechanism, which involves putting a file with the name // of an interface class in the META-INF/services directory, where the // contents of the file is a single line specifying a concrete class // that implements the desired interface. - if (factory == null) { - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] Using ServiceLoader to define the LogFactory subclass to use..."); - } + logDiagnostic("[LOOKUP] Using ServiceLoader to define the LogFactory subclass to use..."); try { final ServiceLoader serviceLoader = ServiceLoader.load(LogFactory.class, baseClassLoader); final Iterator iterator = serviceLoader.iterator(); @@ -884,55 +832,40 @@ public abstract class LogFactory { factory = iterator.next(); } } catch (final ServiceConfigurationError | LinkageError ex) { - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] An exception occurred while trying to find an" + - " instance of LogFactory" + - ": [" + trim(ex.getMessage()) + - "]. Trying alternative implementations..."); - } + logDiagnostic(() -> "[LOOKUP] An exception occurred while trying to find an instance of LogFactory: [" + trim(ex.getMessage()) + + "]. Trying alternative implementations..."); } } } catch (final Exception ex) { // note: if the specified LogFactory class wasn't compatible with LogFactory // for some reason, a ClassCastException will be caught here, and attempts will // continue to find a compatible class. - if (isDiagnosticsEnabled()) { - logDiagnostic( - "[LOOKUP] A security exception occurred while trying to create an" + - " instance of the custom factory class" + - ": [" + trim(ex.getMessage()) + - "]. Trying alternative implementations..."); - } + logDiagnostic(() -> "[LOOKUP] A security exception occurred while trying to create an instance of the custom factory class: [" + + trim(ex.getMessage()) + "]. Trying alternative implementations..."); // ignore } } - + // // Third try looking into the properties file read earlier (if found) - if (factory == null) { if (props != null) { - if (isDiagnosticsEnabled()) { - logDiagnostic( - "[LOOKUP] Looking in properties file for entry with key '" + FACTORY_PROPERTY + - "' to define the LogFactory subclass to use..."); - } + logDiagnostic(() -> + "[LOOKUP] Looking in properties file for entry with key '" + FACTORY_PROPERTY + + "' to define the LogFactory subclass to use..."); final String factoryClass = props.getProperty(FACTORY_PROPERTY); if (factoryClass != null) { - if (isDiagnosticsEnabled()) { - logDiagnostic( - "[LOOKUP] Properties file specifies LogFactory subclass '" + factoryClass + "'"); - } + logDiagnostic(() -> + "[LOOKUP] Properties file specifies LogFactory subclass '" + factoryClass + "'"); factory = newFactory(factoryClass, baseClassLoader, contextClassLoader); - // TODO: think about whether we need to handle exceptions from newFactory - } else if (isDiagnosticsEnabled()) { + } else { logDiagnostic("[LOOKUP] Properties file has no entry specifying LogFactory subclass."); } - } else if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] No properties file available to determine" + " LogFactory subclass from.."); + } else { + logDiagnostic("[LOOKUP] No properties file available to determine LogFactory subclass from.."); } } - + // // Fourth, try one of the three provided factories first from the specified classloader // and then from the current one. if (factory == null) { @@ -946,13 +879,9 @@ public abstract class LogFactory { logDiagnostic("Created object " + objectId(factory) + " to manage class loader " + objectId(contextClassLoader)); } } else { - if (isDiagnosticsEnabled()) { - logDiagnostic( - "[LOOKUP] Loading the default LogFactory implementation '" + FACTORY_DEFAULT + - "' via the same class loader that loaded this LogFactory" + - " class (ie not looking in the context class loader)."); - } - + logDiagnostic(() -> + "[LOOKUP] Loading the default LogFactory implementation '" + FACTORY_DEFAULT + + "' via the same class loader that loaded this LogFactory class (ie not looking in the context class loader)."); // Note: unlike the above code which can try to load custom LogFactory // implementations via the TCCL, we don't try to load the default LogFactory // implementation via the context class loader because: @@ -964,13 +893,11 @@ public abstract class LogFactory { // necessarily a good idea anyway. factory = newFactory(FACTORY_DEFAULT, thisClassLoaderRef.get(), contextClassLoader); } - if (factory != null) { /** * Always cache using context class loader. */ cacheFactory(contextClassLoader, factory); - if (props != null) { final Enumeration names = props.propertyNames(); while (names.hasMoreElements()) { @@ -980,7 +907,6 @@ public abstract class LogFactory { } } } - return factory; } @@ -1031,14 +957,10 @@ public abstract class LogFactory { return props; } } catch (final IOException e) { - if (isDiagnosticsEnabled()) { - logDiagnostic("Unable to close stream for URL " + url); - } + logDiagnostic(() -> "Unable to close stream for URL " + url); } } catch (final IOException e) { - if (isDiagnosticsEnabled()) { - logDiagnostic("Unable to read URL " + url); - } + logDiagnostic(() -> "Unable to read URL " + url); } return null; @@ -1068,9 +990,7 @@ public abstract class LogFactory { } return ClassLoader.getSystemResources(name); } catch (final IOException e) { - if (isDiagnosticsEnabled()) { - logDiagnostic("Exception while trying to find configuration file " + name + ":" + e.getMessage()); - } + logDiagnostic(() -> "Exception while trying to find configuration file " + name + ":" + e.getMessage()); return null; } catch (final NoSuchMethodError e) { // we must be running on a 1.1 JVM which doesn't support @@ -1144,9 +1064,9 @@ public abstract class LogFactory { implementsLogFactory = factoryFromCustomLoader.isAssignableFrom(logFactoryClass); final String logFactoryClassName = logFactoryClass.getName(); if (implementsLogFactory) { - logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClassName + " implements LogFactory but was loaded by an incompatible class loader."); + logDiagnostic(() -> "[CUSTOM LOG FACTORY] " + logFactoryClassName + " implements LogFactory but was loaded by an incompatible class loader."); } else { - logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClassName + " does not implement LogFactory."); + logDiagnostic(() -> "[CUSTOM LOG FACTORY] " + logFactoryClassName + " does not implement LogFactory."); } } } catch (final SecurityException e) { @@ -1155,8 +1075,9 @@ public abstract class LogFactory { // This will make it very hard to diagnose issues with JCL. // Consider running less securely whilst debugging this issue. // - logDiagnostic("[CUSTOM LOG FACTORY] SecurityException caught trying to determine whether " - + "the compatibility was caused by a class loader conflict: " + e.getMessage()); + logDiagnostic( + () -> "[CUSTOM LOG FACTORY] SecurityException caught trying to determine whether the compatibility was caused by a class loader conflict: " + + e.getMessage()); } catch (final LinkageError e) { // // This should be an unusual circumstance. @@ -1164,8 +1085,9 @@ public abstract class LogFactory { // Another possibility may be an exception thrown by an initializer. // Time for a clean rebuild? // - logDiagnostic("[CUSTOM LOG FACTORY] LinkageError caught trying to determine whether " - + "the compatibility was caused by a class loader conflict: " + e.getMessage()); + logDiagnostic( + () -> "[CUSTOM LOG FACTORY] LinkageError caught trying to determine whether the compatibility was caused by a class loader conflict: " + + e.getMessage()); } catch (final ClassNotFoundException e) { // // LogFactory cannot be loaded by the class loader which loaded the custom factory implementation. @@ -1174,7 +1096,7 @@ public abstract class LogFactory { // Running with diagnostics on should give information about the class loaders used // to load the custom factory. // - logDiagnostic("[CUSTOM LOG FACTORY] LogFactory class cannot be loaded by the class loader which loaded " + logDiagnostic(() -> "[CUSTOM LOG FACTORY] LogFactory class cannot be loaded by the class loader which loaded " + "the custom LogFactory implementation. Is the custom factory in the right class loader?"); } } @@ -1217,12 +1139,12 @@ public abstract class LogFactory { } private static boolean isClassAvailable(final String className, final ClassLoader classLoader) { - logDiagnostic("Checking if class '" + className + "' is available in class loader " + objectId(classLoader)); + logDiagnostic(() -> "Checking if class '" + className + "' is available in class loader " + objectId(classLoader)); try { Class.forName(className, true, classLoader); return true; } catch (final ClassNotFoundException | LinkageError e) { - logDiagnostic("Failed to load class '" + className + "' from class loader " + objectId(classLoader) + ": " + e.getMessage()); + logDiagnostic(() -> "Failed to load class '" + className + "' from class loader " + objectId(classLoader) + ": " + e.getMessage()); } return false; } @@ -1264,7 +1186,6 @@ public abstract class LogFactory { if (!isDiagnosticsEnabled()) { return; } - try { // Deliberately use System.getProperty here instead of getSystemProperty; if // the overall security policy for the calling application forbids access to @@ -1274,10 +1195,8 @@ public abstract class LogFactory { } catch (final SecurityException ex) { logDiagnostic("[ENV] Security setting prevent interrogation of system classpaths."); } - final String className = clazz.getName(); ClassLoader classLoader; - try { classLoader = getClassLoader(clazz); } catch (final SecurityException ex) { @@ -1285,7 +1204,6 @@ public abstract class LogFactory { logDiagnostic("[ENV] Security forbids determining the class loader for " + className); return; } - logDiagnostic("[ENV] Class " + className + " was loaded via class loader " + objectId(classLoader)); logHierarchy("[ENV] Ancestry of class loader which loaded " + className + " is ", classLoader); } @@ -1313,12 +1231,43 @@ public abstract class LogFactory { */ private static void logDiagnostic(final String msg) { if (DIAGNOSTICS_STREAM != null) { - DIAGNOSTICS_STREAM.print(diagnosticPrefix); - DIAGNOSTICS_STREAM.println(msg); - DIAGNOSTICS_STREAM.flush(); + logDiagnosticDirect(msg); } } + /** + * Writes the specified message to the internal logging destination. + *

+ * Note that this method is private; concrete subclasses of this class + * should not call it because the diagnosticPrefix string this + * method puts in front of all its messages is LogFactory@...., + * while subclasses should put SomeSubClass@... + *

+ *

+ * Subclasses should instead compute their own prefix, then call + * logRawDiagnostic. Note that calling isDiagnosticsEnabled is + * fine for subclasses. + *

+ *

+ * Note that it is safe to call this method before initDiagnostics + * is called; any output will just be ignored (as isDiagnosticsEnabled + * will return false). + *

+ * + * @param msg is the diagnostic message to be output. + */ + private static void logDiagnostic(final Supplier msg) { + if (DIAGNOSTICS_STREAM != null) { + logDiagnosticDirect(msg.get()); + } + } + + private static void logDiagnosticDirect(final String msg) { + DIAGNOSTICS_STREAM.print(DIAGNOSTICS_PREFIX); + DIAGNOSTICS_STREAM.println(msg); + DIAGNOSTICS_STREAM.flush(); + } + /** * Logs diagnostic messages about the given class loader * and it's hierarchy. The prefix is prepended to the message @@ -1332,10 +1281,8 @@ public abstract class LogFactory { } ClassLoader systemClassLoader; if (classLoader != null) { - final String classLoaderString = classLoader.toString(); - logDiagnostic(prefix + objectId(classLoader) + " == '" + classLoaderString + "'"); + logDiagnostic(prefix + objectId(classLoader) + " == '" + classLoader.toString() + "'"); } - try { systemClassLoader = ClassLoader.getSystemClassLoader(); } catch (final SecurityException ex) { @@ -1349,14 +1296,12 @@ public abstract class LogFactory { if (classLoader == systemClassLoader) { buf.append(" (SYSTEM) "); } - try { classLoader = classLoader.getParent(); } catch (final SecurityException ex) { buf.append(" --> SECRET"); break; } - buf.append(" --> "); if (classLoader == null) { buf.append("BOOT"); @@ -1447,17 +1392,12 @@ public abstract class LogFactory { // method will propagate out of this method; in particular a // ClassCastException can be thrown. final Object result = AccessController.doPrivileged((PrivilegedAction) () -> createFactory(factoryClass, classLoader)); - if (result instanceof LogConfigurationException) { final LogConfigurationException ex = (LogConfigurationException) result; - if (isDiagnosticsEnabled()) { - logDiagnostic("An error occurred while loading the factory class:" + ex.getMessage()); - } + logDiagnostic(() -> "An error occurred while loading the factory class:" + ex.getMessage()); throw ex; } - if (isDiagnosticsEnabled()) { - logDiagnostic("Created object " + objectId(result) + " to manage class loader " + objectId(contextClassLoader)); - } + logDiagnostic(() -> "Created object " + objectId(result) + " to manage class loader " + objectId(contextClassLoader)); return (LogFactory) result; } @@ -1475,7 +1415,7 @@ public abstract class LogFactory { return (LogFactory) Class.forName(FACTORY_SLF4J, true, classLoader).getConstructor().newInstance(); } catch (final LinkageError | ReflectiveOperationException ignored) { } finally { - logDiagnostic( + logDiagnostic(() -> "[LOOKUP] Log4j API to SLF4J redirection detected. Loading the SLF4J LogFactory implementation '" + FACTORY_SLF4J + "'."); } } @@ -1483,19 +1423,19 @@ public abstract class LogFactory { return (LogFactory) Class.forName(FACTORY_LOG4J_API, true, classLoader).getConstructor().newInstance(); } catch (final LinkageError | ReflectiveOperationException ignored) { } finally { - logDiagnostic("[LOOKUP] Loading the Log4j API LogFactory implementation '" + FACTORY_LOG4J_API + "'."); + logDiagnostic(() -> "[LOOKUP] Loading the Log4j API LogFactory implementation '" + FACTORY_LOG4J_API + "'."); } try { return (LogFactory) Class.forName(FACTORY_SLF4J, true, classLoader).getConstructor().newInstance(); } catch (final LinkageError | ReflectiveOperationException ignored) { } finally { - logDiagnostic("[LOOKUP] Loading the SLF4J LogFactory implementation '" + FACTORY_SLF4J + "'."); + logDiagnostic(() -> "[LOOKUP] Loading the SLF4J LogFactory implementation '" + FACTORY_SLF4J + "'."); } try { return (LogFactory) Class.forName(FACTORY_DEFAULT, true, classLoader).getConstructor().newInstance(); } catch (final LinkageError | ReflectiveOperationException ignored) { } finally { - logDiagnostic("[LOOKUP] Loading the legacy LogFactory implementation '" + FACTORY_DEFAULT + "'."); + logDiagnostic(() -> "[LOOKUP] Loading the legacy LogFactory implementation '" + FACTORY_DEFAULT + "'."); } return null; } @@ -1504,13 +1444,13 @@ public abstract class LogFactory { * Returns a string that uniquely identifies the specified object, including * its class. *

- * The returned string is of form "className@hashCode", that is, is the same as - * the return value of the Object.toString() method, but works even when + * The returned string is of form {@code "className@hashCode"}, that is, is the same as + * the return value of the {@link Object#toString()} method, but works even when * the specified object's class has overridden the toString method. *

* * @param obj may be null. - * @return a string of form className@hashCode, or "null" if param o is null. + * @return a string of form {@code className@hashCode}, or "null" if obj is null. * @since 1.1 */ public static String objectId(final Object obj) { @@ -1529,9 +1469,7 @@ public abstract class LogFactory { * @param classLoader ClassLoader for which to release the LogFactory */ public static void release(final ClassLoader classLoader) { - if (isDiagnosticsEnabled()) { - logDiagnostic("Releasing factory for class loader " + objectId(classLoader)); - } + logDiagnostic(() -> "Releasing factory for class loader " + objectId(classLoader)); // factories is not final and could be replaced in this block. final Hashtable factories = LogFactory.factories; synchronized (factories) { @@ -1559,15 +1497,12 @@ public abstract class LogFactory { * garbage collection. */ public static void releaseAll() { - if (isDiagnosticsEnabled()) { - logDiagnostic("Releasing factory for all class loaders."); - } + logDiagnostic("Releasing factory for all class loaders."); // factories is not final and could be replaced in this block. final Hashtable factories = LogFactory.factories; synchronized (factories) { factories.values().forEach(LogFactory::release); factories.clear(); - if (nullClassLoaderFactory != null) { nullClassLoaderFactory.release(); nullClassLoaderFactory = null; @@ -1575,12 +1510,9 @@ public abstract class LogFactory { } } - /** Utility method to safely trim a string. */ + /** Trims the given string in a null-safe manner. */ private static String trim(final String src) { - if (src == null) { - return null; - } - return src.trim(); + return src != null ? src.trim() : null; } /**