Fix NPE on Exception and failure to close streamrefs/changes/61/6661/1

- The stream (configFile) opened in loadInstances was never closed.
- The reader in load could cause NPE when closing in finally
- Number of whitespace and other cleanup

Change-Id: Ib43f9d8a3d3ba316f89b17dc0c5fa1d310d82730
Signed-off-by: Henrik Lynggaard Hansen <henrik@hlyh.dk>

1 files changed, 59 insertions, 45 deletions

diff --git a/hudson-core/src/main/java/hudson/util/Service.java b/hudson-core/src/main/java/hudson/util/Service.java
index 75b1f0d..c8fc745 100644
--- a/hudson-core/src/main/java/hudson/util/Service.java
+++ b/hudson-core/src/main/java/hudson/util/Service.java
@@ -1,19 +1,20 @@
-/*******************************************************************************
+/**
+ * *****************************************************************************
  *
  * Copyright (c) 2004-2009 Oracle Corporation.
  *
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
+ * All rights reserved. This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License v1.0 which
+ * accompanies this distribution, and is available at
  * http://www.eclipse.org/legal/epl-v10.html
  *
- * Contributors: 
-*
-*    Kohsuke Kawaguchi
- *     
+ * Contributors:
+ * 
+* Kohsuke Kawaguchi
  *
- *******************************************************************************/ 
-
+ *
+ ******************************************************************************
+ */
 package hudson.util;
 
 import java.io.BufferedReader;
@@ -34,75 +35,88 @@ import static java.util.logging.Level.WARNING;
  * @author Kohsuke Kawaguchi
  */
 public class Service {
+
+    private static final Logger LOGGER = Logger.getLogger(Service.class.getName());
+
     /**
      * Poorman's clone of JDK6 ServiceLoader.
      */
     public static <T> List<T> loadInstances(ClassLoader classLoader, Class<T> type) throws IOException {
         List<T> result = new ArrayList<T>();
 
-        final Enumeration<URL> e = classLoader.getResources("META-INF/services/"+type.getName());
-        while (e.hasMoreElements()) {
-            URL url = e.nextElement();
-            BufferedReader configFile = new BufferedReader(new InputStreamReader(url.openStream(),"UTF-8"));
-            String line;
-            while ((line = configFile.readLine()) != null) {
-                line = line.trim();
-                if (line.startsWith("#") || line.length()==0)   continue;
-
-                try {
-                    Class<?> t = classLoader.loadClass(line);
-                    if (!type.isAssignableFrom(t))   continue;      // invalid type
+        final Enumeration<URL> e = classLoader.getResources("META-INF/services/" + type.getName());
+        BufferedReader configFile = null;
+        try {            
+            while (e.hasMoreElements()) {
+                URL url = e.nextElement();
+                configFile = new BufferedReader(new InputStreamReader(url.openStream(), "UTF-8"));
+                String line;
+                while ((line = configFile.readLine()) != null) {
+                    line = line.trim();
+                    if (line.startsWith("#") || line.length() == 0) {
+                        continue;
+                    }
 
-                    result.add(type.cast(t.newInstance()));
-                } catch (ClassNotFoundException x) {
-                    LOGGER.log(WARNING,"Failed to load "+line,x);
-                } catch (InstantiationException x) {
-                    LOGGER.log(WARNING,"Failed to load "+line,x);
-                } catch (IllegalAccessException x) {
-                    LOGGER.log(WARNING,"Failed to load "+line,x);
+                    try {
+                        Class<?> t = classLoader.loadClass(line);
+                        if (!type.isAssignableFrom(t)) {
+                            // invalid type
+                            continue;
+                        }
+                        result.add(type.cast(t.newInstance()));
+                    } catch (ClassNotFoundException x) {
+                        LOGGER.log(WARNING, "Failed to load " + line, x);
+                    } catch (InstantiationException x) {
+                        LOGGER.log(WARNING, "Failed to load " + line, x);
+                    } catch (IllegalAccessException x) {
+                        LOGGER.log(WARNING, "Failed to load " + line, x);
+                    }
                 }
             }
+        } finally {
+            IOUtils.closeQuietly(configFile);
         }
 
         return result;
     }
 
     /**
-     * Look up <tt>META-INF/service/<i>SPICLASSNAME</i></tt> from the classloader
-     * and all the discovered classes into the given collection.
+     * Look up <tt>META-INF/service/<i>SPICLASSNAME</i></tt> from the
+     * classloader and all the discovered classes into the given collection.
      */
     public static <T> void load(Class<T> spi, ClassLoader cl, Collection<Class<? extends T>> result) {
         try {
             Enumeration<URL> e = cl.getResources("META-INF/services/" + spi.getName());
-            while(e.hasMoreElements()) {
+            while (e.hasMoreElements()) {
                 BufferedReader r = null;
                 URL url = e.nextElement();
                 try {
-                    r = new BufferedReader(new InputStreamReader(url.openStream(),"UTF-8"));
+                    r = new BufferedReader(new InputStreamReader(url.openStream(), "UTF-8"));
                     String line;
-                    while((line=r.readLine())!=null) {
-                        if(line.startsWith("#"))
-                            continue;   // comment line
+                    while ((line = r.readLine()) != null) {
+                        // comment line
+                        if (line.startsWith("#")) {
+                            continue;
+                        }
                         line = line.trim();
-                        if(line.length()==0)
-                            continue;   // empty line. ignore.
-
+                        if (line.length() == 0) {
+                            // empty line. ignore.
+                            continue;
+                        }
                         try {
                             result.add(cl.loadClass(line).asSubclass(spi));
                         } catch (ClassNotFoundException x) {
-                            LOGGER.log(Level.WARNING, "Failed to load "+line, x);
+                            LOGGER.log(Level.WARNING, "Failed to load " + line, x);
                         }
                     }
                 } catch (IOException x) {
-                    LOGGER.log(Level.WARNING, "Failed to load "+url, x);
+                    LOGGER.log(Level.WARNING, "Failed to load " + url, x);
                 } finally {
-                    r.close();
+                    IOUtils.closeQuietly(r);
                 }
             }
         } catch (IOException x) {
-            LOGGER.log(Level.WARNING, "Failed to look up service providers for "+spi, x);
+            LOGGER.log(Level.WARNING, "Failed to look up service providers for " + spi, x);
         }
     }
-
-    private static final Logger LOGGER = Logger.getLogger(Service.class.getName());
 }