You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2022/10/29 19:31:21 UTC

[logging-log4j2] branch master updated (50458fdd28 -> 37941b6fff)

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


    from 50458fdd28 Fixing HtmlLayoutTest when timezone has minutes (#1127)
     new 04ca410b74 Add `ServiceLoaderUtil` from `release-2.x`
     new 37941b6fff Remove dependency on PropertiesUtil from `Strings`

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/logging/log4j/util/Strings.java     |  3 +-
 .../apache/logging/log4j/util3/PropertiesUtil.java |  3 +-
 .../logging/log4j/util3/ServiceLoaderUtil.java     | 72 ++++++++++++++++++----
 .../logging/log4j/util3/ServiceRegistry.java       |  9 ++-
 .../util3/SystemPropertiesPropertySource.java      | 12 ++++
 5 files changed, 83 insertions(+), 16 deletions(-)


[logging-log4j2] 01/02: Add `ServiceLoaderUtil` from `release-2.x`

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 04ca410b744d724857f1807432f92337f45f47d3
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Tue Apr 5 22:10:53 2022 +0200

    Add `ServiceLoaderUtil` from `release-2.x`
    
    The Java 8 version of `ServiceLoaderUtil` is much more resilient to
    errors: the spliterator used in`ServiceLoader#stream()` can fail e.g. if
    the service class loading fails, so we can not catch the error and
    ignore it.
---
 .../apache/logging/log4j/util3/PropertiesUtil.java |  3 +-
 .../logging/log4j/util3/ServiceLoaderUtil.java     | 72 ++++++++++++++++++----
 .../logging/log4j/util3/ServiceRegistry.java       |  9 ++-
 3 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util3/PropertiesUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util3/PropertiesUtil.java
index 4000416263..0eea506a84 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util3/PropertiesUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util3/PropertiesUtil.java
@@ -244,7 +244,8 @@ public class PropertiesUtil implements PropertyEnvironment {
             }
             sources.add(propertySource);
             final ServiceRegistry registry = ServiceRegistry.getInstance();
-            sources.addAll(registry.getServices(PropertySource.class, MethodHandles.lookup(), null));
+            // Does not log errors using StatusLogger, which depends on PropertiesUtil being initialized.
+            sources.addAll(registry.getServices(PropertySource.class, MethodHandles.lookup(), null, /*verbose=*/false));
             reload();
         }
 
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceLoaderUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceLoaderUtil.java
index 44542b0fd9..d655779778 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceLoaderUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceLoaderUtil.java
@@ -14,6 +14,7 @@
  * See the license for the specific language governing permissions and
  * limitations under the license.
  */
+
 package org.apache.logging.log4j.util3;
 
 import java.lang.invoke.CallSite;
@@ -23,17 +24,22 @@ import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.Collections;
 import java.util.HashSet;
-import java.util.Objects;
+import java.util.Iterator;
 import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.Set;
+import java.util.Spliterator;
+import java.util.function.Consumer;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.status.StatusLogger;
 
 /**
- * Loads all valid instances of a service.
+ * This class should be considered internal.
  */
 public final class ServiceLoaderUtil {
 
@@ -88,6 +94,11 @@ public final class ServiceLoaderUtil {
 
     static <T> Stream<T> loadClassloaderServices(final Class<T> serviceType, final Lookup lookup,
             final ClassLoader classLoader, final boolean verbose) {
+        return StreamSupport.stream(new ServiceLoaderSpliterator<T>(serviceType, lookup, classLoader, verbose), false);
+    }
+
+    static <T> Iterable<T> callServiceLoader(Lookup lookup, Class<T> serviceType, ClassLoader classLoader,
+            boolean verbose) {
         try {
             // Creates a lambda in the caller's domain that calls `ServiceLoader`
             final MethodHandle loadHandle = lookup.findStatic(ServiceLoader.class, "load",
@@ -111,22 +122,57 @@ public final class ServiceLoaderUtil {
                         MethodType.methodType(Object.class, PrivilegedAction.class));
                 serviceLoader = (ServiceLoader<T>) privilegedHandle.invoke(action);
             }
-            return serviceLoader.stream().map(provider -> {
-                try {
-                    return provider.get();
-                } catch (ServiceConfigurationError e) {
-                    if (verbose) {
-                        StatusLogger.getLogger().warn("Unable to load service class for service {}", serviceType, e);
-                    }
-                }
-                return null;
-            }).filter(Objects::nonNull);
+            return serviceLoader;
         } catch (Throwable e) {
             if (verbose) {
                 StatusLogger.getLogger().error("Unable to load services for service {}", serviceType, e);
             }
         }
-        return Stream.empty();
+        return Collections.emptyList();
     }
 
+    private static class ServiceLoaderSpliterator<S> implements Spliterator<S> {
+
+        private final Iterator<S> serviceIterator;
+        private final Logger logger;
+        private final String serviceName;
+
+        public ServiceLoaderSpliterator(final Class<S> serviceType, final Lookup lookup, final ClassLoader classLoader,
+                final boolean verbose) {
+            this.serviceIterator = callServiceLoader(lookup, serviceType, classLoader, verbose).iterator();
+            this.logger = verbose ? StatusLogger.getLogger() : null;
+            this.serviceName = serviceType.toString();
+        }
+
+        @Override
+        public boolean tryAdvance(Consumer<? super S> action) {
+            while (serviceIterator.hasNext()) {
+                try {
+                    action.accept(serviceIterator.next());
+                    return true;
+                } catch (ServiceConfigurationError e) {
+                    if (logger != null) {
+                        logger.warn("Unable to load service class for service {}", serviceName, e);
+                    }
+                }
+            }
+            return false;
+        }
+
+        @Override
+        public Spliterator<S> trySplit() {
+            return null;
+        }
+
+        @Override
+        public long estimateSize() {
+            return Long.MAX_VALUE;
+        }
+
+        @Override
+        public int characteristics() {
+            return NONNULL | IMMUTABLE;
+        }
+
+    }
 }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceRegistry.java b/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceRegistry.java
index cb50982514..75e49e9280 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceRegistry.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util3/ServiceRegistry.java
@@ -70,8 +70,15 @@ public class ServiceRegistry {
      * @return loaded service instances
      */
     public <S> List<S> getServices(final Class<S> serviceType, final Lookup lookup, final Predicate<S> validator) {
+        return getServices(serviceType, lookup, validator, true);
+    }
+
+    /**
+     * Set 'verbose' to false if the `StatusLogger` is not available yet.
+     */
+    <S> List<S> getServices(final Class<S> serviceType, final Lookup lookup, final Predicate<S> validator, boolean verbose) {
         final List<S> services = cast(mainServices.computeIfAbsent(serviceType,
-                ignored -> ServiceLoaderUtil.loadServices(serviceType, lookup)
+                ignored -> ServiceLoaderUtil.loadServices(serviceType, lookup, false, verbose)
                         .filter(validator != null ? validator : unused -> true)
                         .collect(Collectors.toList())));
         return Stream.concat(services.stream(), bundleServices.values().stream().flatMap(map -> {


[logging-log4j2] 02/02: Remove dependency on PropertiesUtil from `Strings`

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 37941b6fffe55fce23fb460a13d6fd4ab0a29793
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Sat Oct 29 21:30:16 2022 +0200

    Remove dependency on PropertiesUtil from `Strings`
    
    The static initializer of `Strings` depends on `PropertiesUtil` to
    retrieve the system's line separator. This might introduce dangerous
    recursive dependencies between `Strings`, `PropertiesUtil`,
    `ServiceRegistry` and `ServiceLoaderUtil`.
---
 .../src/main/java/org/apache/logging/log4j/util/Strings.java |  3 ++-
 .../logging/log4j/util3/SystemPropertiesPropertySource.java  | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
index cc6d38835b..f3f63202e9 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
@@ -18,6 +18,7 @@ package org.apache.logging.log4j.util;
 
 import org.apache.logging.log4j.util3.Chars;
 import org.apache.logging.log4j.util3.PropertiesUtil;
+import org.apache.logging.log4j.util3.SystemPropertiesPropertySource;
 
 import java.util.Iterator;
 import java.util.Locale;
@@ -48,7 +49,7 @@ public final class Strings {
      * OS-dependent line separator, defaults to {@code "\n"} if the system property {@code ""line.separator"} cannot be
      * read.
      */
-    public static final String LINE_SEPARATOR = PropertiesUtil.getProperties().getStringProperty("line.separator",
+    public static final String LINE_SEPARATOR = SystemPropertiesPropertySource.getSystemProperty("line.separator",
             "\n");
 
     private Strings() {
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util3/SystemPropertiesPropertySource.java b/log4j-api/src/main/java/org/apache/logging/log4j/util3/SystemPropertiesPropertySource.java
index c3e6b9ee65..69de98c411 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util3/SystemPropertiesPropertySource.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util3/SystemPropertiesPropertySource.java
@@ -36,6 +36,18 @@ public class SystemPropertiesPropertySource implements PropertySource {
 	private static final int DEFAULT_PRIORITY = 0;
 	private static final String PREFIX = "log4j2.";
 
+	/**
+	 * Used by bootstrap code to get system properties without loading PropertiesUtil.
+	 */
+	public static String getSystemProperty(final String key, final String defaultValue) {
+		try {
+			return System.getProperty(key, defaultValue);
+		} catch (SecurityException e) {
+			// Silently ignore the exception
+			return defaultValue;
+		}
+	}
+
 	@Override
 	public int getPriority() {
 		return DEFAULT_PRIORITY;