Issue #4340 - Continuing after ServiceLoader ServiceConfigurationError

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-alpn/jetty-alpn-client/src/main/java/org/eclipse/jetty/alpn/client/ALPNClientConnectionFactory.java

@@ -19,10 +19,8 @@
 package org.eclipse.jetty.alpn.client;
 
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.concurrent.Executor;
 import javax.net.ssl.SSLEngine;
 
@@ -32,6 +30,7 @@ import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.NegotiatingClientConnectionFactory;
 import org.eclipse.jetty.io.ssl.ALPNProcessor.Client;
 import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
@@ -54,21 +53,8 @@ public class ALPNClientConnectionFactory extends NegotiatingClientConnectionFact
         IllegalStateException failure = new IllegalStateException("No Client ALPNProcessors!");
 
         // Use a for loop on iterator so load exceptions can be caught and ignored
-        for (Iterator<Client> i = ServiceLoader.load(Client.class).iterator(); i.hasNext(); )
+        for (Client processor : ServiceLoaderUtil.load(Client.class))
         {
-            Client processor;
-            try
-            {
-                processor = i.next();
-            }
-            catch (Throwable x)
-            {
-                if (LOG.isDebugEnabled())
-                    LOG.debug(x);
-                failure.addSuppressed(x);
-                continue;
-            }
-
             try
             {
                 processor.init();

jetty-alpn/jetty-alpn-server/src/main/java/org/eclipse/jetty/alpn/server/ALPNServerConnectionFactory.java

@@ -20,9 +20,7 @@ package org.eclipse.jetty.alpn.server;
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Iterator;
 import java.util.List;
-import java.util.ServiceLoader;
 import javax.net.ssl.SSLEngine;
 
 import org.eclipse.jetty.io.AbstractConnection;
@@ -30,6 +28,7 @@ import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.ssl.ALPNProcessor.Server;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.NegotiatingServerConnectionFactory;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.annotation.Name;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
@@ -51,22 +50,8 @@ public class ALPNServerConnectionFactory extends NegotiatingServerConnectionFact
 
         IllegalStateException failure = new IllegalStateException("No Server ALPNProcessors!");
         // Use a for loop on iterator so load exceptions can be caught and ignored
-        for (Iterator<Server> i = ServiceLoader.load(Server.class).iterator(); i.hasNext(); )
+        for (Server processor : ServiceLoaderUtil.load(Server.class))
         {
-            Server processor;
-            try
-            {
-                processor = i.next();
-            }
-            catch (Throwable x)
-            {
-                if (LOG.isDebugEnabled())
-                    LOG.debug(x);
-                if (x != failure)
-                    failure.addSuppressed(x);
-                continue;
-            }
-
             try
             {
                 processor.init();

jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java

@@ -31,7 +31,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
@@ -48,6 +47,7 @@ import org.eclipse.jetty.plus.webapp.PlusConfiguration;
 import org.eclipse.jetty.util.JavaVersion;
 import org.eclipse.jetty.util.MultiException;
 import org.eclipse.jetty.util.ProcessorUtils;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.log.Log;
@@ -811,7 +811,7 @@ public class AnnotationConfiguration extends AbstractConfiguration
         long start = 0;
         if (LOG.isDebugEnabled())
             start = System.nanoTime();
-        ServiceLoader<ServletContainerInitializer> loader = ServiceLoader.load(ServletContainerInitializer.class);
+        List<ServletContainerInitializer> scis = ServiceLoaderUtil.load(ServletContainerInitializer.class);
         if (LOG.isDebugEnabled())
             LOG.debug("Service loaders found in {}ms", (TimeUnit.MILLISECONDS.convert((System.nanoTime() - start), TimeUnit.NANOSECONDS)));
 
@@ -820,22 +820,8 @@ public class AnnotationConfiguration extends AbstractConfiguration
 
         //Get initial set of SCIs that aren't from excluded jars or excluded by the containerExclusionPattern, or excluded
         //because containerInitializerOrdering omits it
-        Iterator<ServletContainerInitializer> iter = loader.iterator();
+        for (ServletContainerInitializer sci : scis)
-        while (iter.hasNext())
         {
-            ServletContainerInitializer sci;
-            try
-            {
-                sci = iter.next();
-            }
-            catch (Error e)
-            {
-                // Probably a SCI discovered on the system classpath that is hidden by the context classloader
-                LOG.info("Error: " + e.getMessage() + " for " + context);
-                LOG.debug(e);
-                continue;
-            }
-
             if (matchesExclusionPattern(sci))
             {
                 if (LOG.isDebugEnabled())

jetty-http/src/main/java/org/eclipse/jetty/http/PreEncodedHttpField.java

@@ -20,10 +20,10 @@ package org.eclipse.jetty.http;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.ServiceLoader;
 
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
@@ -44,20 +44,13 @@ public class PreEncodedHttpField extends HttpField
     static
     {
         List<HttpFieldPreEncoder> encoders = new ArrayList<>();
-        Iterator<HttpFieldPreEncoder> iter = ServiceLoader.load(HttpFieldPreEncoder.class).iterator();
+        List<HttpFieldPreEncoder> discoveredEncoders = ServiceLoaderUtil.load(HttpFieldPreEncoder.class);
-        while (iter.hasNext())
+        for (HttpFieldPreEncoder encoder : discoveredEncoders)
         {
-            try
+            if (index(encoder.getHttpVersion()) >= 0)
-            {
+                encoders.add(encoder);
-                HttpFieldPreEncoder encoder = iter.next();
-                if (index(encoder.getHttpVersion()) >= 0)
-                    encoders.add(encoder);
-            }
-            catch (Error | RuntimeException e)
-            {
-                LOG.debug(e);
-            }
         }
+
         LOG.debug("HttpField encoders loaded: {}", encoders);
         int size = encoders.size();
 

jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java

@@ -26,7 +26,6 @@ import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.Set;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -41,6 +40,7 @@ import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.server.handler.ContextHandler.Context;
 import org.eclipse.jetty.server.handler.HandlerWrapper;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.component.DumpableCollection;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
@@ -76,11 +76,7 @@ public abstract class SecurityHandler extends HandlerWrapper implements Authenti
 
     static
     {
-        for (Authenticator.Factory factory : ServiceLoader.load(Authenticator.Factory.class))
+        __knownAuthenticatorFactories.addAll(ServiceLoaderUtil.load(Authenticator.Factory.class));
-        {
-            __knownAuthenticatorFactories.add(factory);
-        }
-
         __knownAuthenticatorFactories.add(new DefaultAuthenticatorFactory());
     }
 

jetty-util/src/main/java/org/eclipse/jetty/util/ServiceLoaderUtil.java

@@ -0,0 +1,68 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.util;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
+
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+
+public class ServiceLoaderUtil
+{
+    private static final Logger LOG = Log.getLogger(ServiceLoaderUtil.class);
+    private static final int MAX_ERRORS = 100;
+
+    /**
+     * Uses the {@link ServiceLoader} to assemble the service providers into a list.
+     * If loading a service type throws {@link ServiceConfigurationError},
+     * it warns and continues iterating through the service loader.
+     * @param service The interface or abstract class representing the service.
+     * @param <T> The class of the service type.
+     * @return a list of the loaded service providers.
+     * @throws ServiceConfigurationError If the number of errors exceeds {@link #MAX_ERRORS}
+     */
+    public static <T> List<T> load(Class<T> service)
+    {
+        List<T> list = new ArrayList<>();
+        Iterator<T> iterator = ServiceLoader.load(service).iterator();
+
+        int errors = 0;
+        while (true)
+        {
+            try
+            {
+                if (!iterator.hasNext())
+                    break;
+                list.add(iterator.next());
+            }
+            catch (ServiceConfigurationError e)
+            {
+                LOG.warn(e);
+                if (++errors >= MAX_ERRORS)
+                    throw e;
+            }
+        }
+
+        return list;
+    }
+}

jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java

@@ -21,8 +21,9 @@ package org.eclipse.jetty.util.security;
 import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
-import java.util.ServiceLoader;
+import java.util.List;
 
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
@@ -42,7 +43,7 @@ public abstract class Credential implements Serializable
 {
     private static final long serialVersionUID = -7760551052768181572L;
     private static final Logger LOG = Log.getLogger(Credential.class);
-    private static final ServiceLoader<CredentialProvider> CREDENTIAL_PROVIDER_LOADER = ServiceLoader.load(CredentialProvider.class);
+    private static final List<CredentialProvider> CREDENTIAL_PROVIDERS = ServiceLoaderUtil.load(CredentialProvider.class);
 
     /**
      * Check a credential
@@ -68,7 +69,7 @@ public abstract class Credential implements Serializable
         if (credential.startsWith(MD5.__TYPE))
             return new MD5(credential);
 
-        for (CredentialProvider cp : CREDENTIAL_PROVIDER_LOADER)
+        for (CredentialProvider cp : CREDENTIAL_PROVIDERS)
         {
             if (credential.startsWith(cp.getPrefix()))
             {

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java

@@ -30,12 +30,12 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.Loader;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.TopologicalSort;
 import org.eclipse.jetty.util.annotation.Name;
 import org.eclipse.jetty.util.component.Dumpable;
@@ -74,26 +74,18 @@ public class Configurations extends AbstractList<Configuration> implements Dumpa
     {
         if (__known.isEmpty())
         {
-            ServiceLoader<Configuration> configs = ServiceLoader.load(Configuration.class);
+            List<Configuration> configs = ServiceLoaderUtil.load(Configuration.class);
-            for (Iterator<Configuration> i = configs.iterator(); i.hasNext(); )
+            for (Configuration configuration : configs)
             {
-                try
+                if (!configuration.isAvailable())
                 {
-                    Configuration configuration = i.next();
+                    if (LOG.isDebugEnabled())
-                    if (!configuration.isAvailable())
+                        LOG.debug("Configuration unavailable: " + configuration);
-                    {
+                    __unavailable.add(configuration);
-                        if (LOG.isDebugEnabled())
+                    continue;
-                            LOG.debug("Configuration unavailable: " + configuration);
-                        __unavailable.add(configuration);
-                        continue;
-                    }
-                    __known.add(configuration);
-                    __knownByClassName.add(configuration.getClass().getName());
-                }
-                catch (Throwable e)
-                {
-                    LOG.warn(e);
                 }
+                __known.add(configuration);
+                __knownByClassName.add(configuration.getClass().getName());
             }
 
             sort(__known);

jetty-websocket/websocket-jetty-api/src/main/java/module-info.java

@@ -22,4 +22,6 @@ module org.eclipse.jetty.websocket.jetty.api
     exports org.eclipse.jetty.websocket.api.annotations;
     exports org.eclipse.jetty.websocket.api.extensions;
     exports org.eclipse.jetty.websocket.api.util;
+
+    uses org.eclipse.jetty.websocket.api.extensions.ExtensionConfig.Parser;
 }

jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionConfig.java

@@ -34,7 +34,7 @@ public interface ExtensionConfig
 
     private static ExtensionConfig.Parser getParser()
     {
-        return ServiceLoader.load(ExtensionConfig.Parser.class).findFirst().get();
+        return ServiceLoader.load(ExtensionConfig.Parser.class).iterator().next();
     }
 
     static ExtensionConfig parse(String parameterizedName)

jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java

@@ -55,6 +55,7 @@ import java.util.Set;
 import org.eclipse.jetty.util.LazyList;
 import org.eclipse.jetty.util.Loader;
 import org.eclipse.jetty.util.MultiException;
+import org.eclipse.jetty.util.ServiceLoaderUtil;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.annotation.Name;
@@ -95,7 +96,7 @@ public class XmlConfiguration
         {
             ArrayList.class, HashSet.class, Queue.class, List.class, Set.class, Collection.class
         };
-    private static final Iterable<ConfigurationProcessorFactory> PROCESSOR_FACTORIES = ServiceLoader.load(ConfigurationProcessorFactory.class);
+    private static final List<ConfigurationProcessorFactory> PROCESSOR_FACTORIES = ServiceLoaderUtil.load(ConfigurationProcessorFactory.class);
     private static final XmlParser PARSER = initParser();
     private static final Comparator<Executable> EXECUTABLE_COMPARATOR = (o1, o2) ->
     {