Correctly close streams in exception flowsrefs/changes/86/6586/1

This is a fix of several findbugs findings of streams potentially not
getting closed correctly in case of exceptions.

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

15 files changed, 130 insertions, 80 deletions

diff --git a/hudson-core/src/main/java/hudson/FilePath.java b/hudson-core/src/main/java/hudson/FilePath.java
index 67c1d1a..98bd97d 100644
--- a/hudson-core/src/main/java/hudson/FilePath.java
+++ b/hudson-core/src/main/java/hudson/FilePath.java
@@ -950,10 +950,13 @@ public final class FilePath implements Serializable {
                     } catch (IOException e) {
                         throw new IOException2("Failed to create a temporary directory in "+dir,e);
                     }
-
-                    Writer w = new FileWriter(f);
-                    w.write(contents);
-                    w.close();
+                    Writer w = null;
+                    try {
+                        w = new FileWriter(f);
+                        w.write(contents);
+                    } finally {
+                        IOUtils.closeQuietly(w);
+                    }
 
                     return f.getAbsolutePath();
                 }
diff --git a/hudson-core/src/main/java/hudson/Util.java b/hudson-core/src/main/java/hudson/Util.java
index 3182f9e..4c7831e 100644
--- a/hudson-core/src/main/java/hudson/Util.java
+++ b/hudson-core/src/main/java/hudson/Util.java
@@ -189,15 +189,18 @@ public class Util {
             return "";
         }
 
+        BufferedReader r = null;
         StringBuilder str = new StringBuilder((int) logfile.length());
-
-        BufferedReader r = new BufferedReader(new InputStreamReader(new FileInputStream(logfile), charset));
-        char[] buf = new char[1024];
-        int len;
-        while ((len = r.read(buf, 0, buf.length)) > 0) {
-            str.append(buf, 0, len);
+        try {            
+            r = new BufferedReader(new InputStreamReader(new FileInputStream(logfile), charset));
+            char[] buf = new char[1024];
+            int len;
+            while ((len = r.read(buf, 0, buf.length)) > 0) {
+                str.append(buf, 0, len);
+            }
+        } finally {
+            IOUtils.closeQuietly(r);
         }
-        r.close();
 
         return str.toString();
     }
diff --git a/hudson-core/src/main/java/hudson/console/AnnotatedLargeText.java b/hudson-core/src/main/java/hudson/console/AnnotatedLargeText.java
index cea577c..f891bd4 100644
--- a/hudson-core/src/main/java/hudson/console/AnnotatedLargeText.java
+++ b/hudson-core/src/main/java/hudson/console/AnnotatedLargeText.java
@@ -19,6 +19,7 @@ package hudson.console;
 import hudson.model.Hudson;
 import hudson.remoting.ObjectInputStreamEx;
 import hudson.util.IOException2;
+import hudson.util.IOUtils;
 import hudson.util.Secret;
 import hudson.util.TimeUnit2;
 import org.apache.commons.io.output.ByteArrayOutputStream;
@@ -146,19 +147,21 @@ public class AnnotatedLargeText<T> extends LargeText {
                 w, createAnnotator(Stapler.getCurrentRequest()), context, charset);
         long r = super.writeLogTo(start,caw);
 
+        ObjectOutputStream oos = null;
         try {
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             Cipher sym = Secret.getCipher("AES");
             sym.init(Cipher.ENCRYPT_MODE, Hudson.getInstance().getSecretKeyAsAES128());
-            ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
+            oos = new ObjectOutputStream(new GZIPOutputStream(new CipherOutputStream(baos,sym)));
             oos.writeLong(System.currentTimeMillis()); // send timestamp to prevent a replay attack
-            oos.writeObject(caw.getConsoleAnnotator());
-            oos.close();
+            oos.writeObject(caw.getConsoleAnnotator());            
             StaplerResponse rsp = Stapler.getCurrentResponse();
             if (rsp!=null)
                 rsp.setHeader("X-ConsoleAnnotator", new String(Base64.encodeBase64(baos.toByteArray())));
         } catch (GeneralSecurityException e) {
             throw new IOException2(e);
+        } finally {
+            IOUtils.closeQuietly(oos);
         }
         return r;
     }
diff --git a/hudson-core/src/main/java/hudson/console/ConsoleNote.java b/hudson-core/src/main/java/hudson/console/ConsoleNote.java
index 855f6a9..ca027be 100644
--- a/hudson-core/src/main/java/hudson/console/ConsoleNote.java
+++ b/hudson-core/src/main/java/hudson/console/ConsoleNote.java
@@ -164,18 +164,26 @@ public abstract class ConsoleNote<T> implements Serializable, Describable<Consol
 
     private ByteArrayOutputStream encodeToBytes() throws IOException {
         ByteArrayOutputStream buf = new ByteArrayOutputStream();
-        ObjectOutputStream oos = new ObjectOutputStream(new GZIPOutputStream(buf));
-        oos.writeObject(this);
-        oos.close();
+        ObjectOutputStream oos = null;
+        try {
+            oos = new ObjectOutputStream(new GZIPOutputStream(buf));
+            oos.writeObject(this);
+        } finally {
+            IOUtils.closeQuietly(oos);
+        }
 
         ByteArrayOutputStream buf2 = new ByteArrayOutputStream();
 
-        DataOutputStream dos = new DataOutputStream(new Base64OutputStream(buf2,true,-1,null));
-        buf2.write(PREAMBLE);
-        dos.writeInt(buf.size());
-        buf.writeTo(dos);
-        dos.close();
-        buf2.write(POSTAMBLE);
+        DataOutputStream dos = null;
+        try {
+            dos = new DataOutputStream(new Base64OutputStream(buf2,true,-1,null));
+            buf2.write(PREAMBLE);
+            dos.writeInt(buf.size());
+            buf.writeTo(dos);        
+            buf2.write(POSTAMBLE);
+        } finally {
+            IOUtils.closeQuietly(dos);
+        }
         return buf2;
     }
 
diff --git a/hudson-core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java b/hudson-core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java
index abc6d53..1dad61f 100644
--- a/hudson-core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java
+++ b/hudson-core/src/main/java/hudson/lifecycle/WindowsServiceLifecycle.java
@@ -20,6 +20,7 @@ import hudson.FilePath;
 import hudson.Launcher.LocalLauncher;
 import hudson.Util;
 import hudson.model.Hudson;
+import hudson.util.IOUtils;
 import hudson.util.StreamTaskListener;
 import org.eclipse.hudson.jna.NativeAccessException;
 import org.eclipse.hudson.jna.NativeUtils;
@@ -98,9 +99,13 @@ public class WindowsServiceLifecycle extends Lifecycle {
         File rootDir = Hudson.getInstance().getRootDir();
         File copyFiles = new File(rootDir,"hudson.copies");
 
-        FileWriter w = new FileWriter(copyFiles, true);
-        w.write(by.getAbsolutePath()+'>'+getHudsonWar().getAbsolutePath()+'\n');
-        w.close();
+        FileWriter w = null;
+        try {
+            w = new FileWriter(copyFiles, true);
+            w.write(by.getAbsolutePath()+'>'+getHudsonWar().getAbsolutePath()+'\n');
+        } finally {
+            IOUtils.closeQuietly(w);
+        }
     }
 
     @Override
diff --git a/hudson-core/src/main/java/hudson/lifecycle/WindowsSlaveInstaller.java b/hudson-core/src/main/java/hudson/lifecycle/WindowsSlaveInstaller.java
index 98d6e9f..b843b31 100644
--- a/hudson-core/src/main/java/hudson/lifecycle/WindowsSlaveInstaller.java
+++ b/hudson-core/src/main/java/hudson/lifecycle/WindowsSlaveInstaller.java
@@ -112,9 +112,14 @@ public class WindowsSlaveInstaller implements Callable<Void,RuntimeException>, A
             Logger.getLogger(WindowsSlaveInstaller.class.getName()).log(Level.SEVERE, null, ex);
             return -1;
         } finally {
-            FileInputStream fin = new FileInputStream(new File(pwd, "redirect.log"));
-            IOUtils.copy(fin, out.getLogger());
-            fin.close();
+            FileInputStream fin = null;
+            try {
+                fin = new FileInputStream(new File(pwd, "redirect.log"));
+                IOUtils.copy(fin, out.getLogger());
+            } finally {
+                IOUtils.closeQuietly(fin);
+            }
+            
         }
     }
 
diff --git a/hudson-core/src/main/java/hudson/model/Queue.java b/hudson-core/src/main/java/hudson/model/Queue.java
index f3312bd..31a0197 100644
--- a/hudson-core/src/main/java/hudson/model/Queue.java
+++ b/hudson-core/src/main/java/hudson/model/Queue.java
@@ -28,15 +28,12 @@ import static hudson.util.Iterators.reverse;
 
 import hudson.cli.declarative.CLIMethod;
 import hudson.cli.declarative.CLIResolver;
-import hudson.model.queue.AbstractQueueTask;
-import hudson.model.queue.Executables;
 import hudson.model.queue.SubTask;
 import hudson.model.queue.FutureImpl;
 import hudson.model.queue.MappingWorksheet;
 import hudson.model.queue.MappingWorksheet.Mapping;
 import hudson.model.queue.QueueSorter;
 import hudson.model.queue.QueueTaskDispatcher;
-import hudson.model.queue.Tasks;
 import hudson.model.queue.WorkUnit;
 import hudson.model.Node.Mode;
 import hudson.model.listeners.SaveableListener;
@@ -49,6 +46,7 @@ import hudson.model.queue.CauseOfBlockage.BecauseNodeIsBusy;
 import hudson.model.queue.WorkUnitContext;
 import hudson.triggers.SafeTimerTask;
 import hudson.triggers.Trigger;
+import hudson.util.IOUtils;
 import hudson.util.OneShotEvent;
 import hudson.util.TimeUnit2;
 import hudson.util.XStream2;
@@ -90,6 +88,7 @@ import org.kohsuke.stapler.export.ExportedBean;
 
 import com.thoughtworks.xstream.XStream;
 import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
+
 import java.util.HashSet;
 
 /**
@@ -262,14 +261,18 @@ public class Queue extends ResourceController implements Saveable {
             // first try the old format
             File queueFile = getQueueFile();
             if (queueFile.exists()) {
-                BufferedReader in = new BufferedReader(new InputStreamReader(new FileInputStream(queueFile)));
-                String line;
-                while ((line = in.readLine()) != null) {
-                    AbstractProject j = Hudson.getInstance().getItemByFullName(line, AbstractProject.class);
-                    if (j != null)
-                        j.scheduleBuild();
+                BufferedReader in = null;
+                try {
+                    in = new BufferedReader(new InputStreamReader(new FileInputStream(queueFile)));
+                    String line;
+                    while ((line = in.readLine()) != null) {
+                        AbstractProject j = Hudson.getInstance().getItemByFullName(line, AbstractProject.class);
+                        if (j != null)
+                            j.scheduleBuild();
+                    } 
+                } finally {
+                    IOUtils.closeQuietly(in);
                 }
-                in.close();
                 // discard the queue file now that we are done
                 queueFile.delete();
             } else {
diff --git a/hudson-core/src/main/java/hudson/model/Run.java b/hudson-core/src/main/java/hudson/model/Run.java
index 271d46c..cb7ceaf 100644
--- a/hudson-core/src/main/java/hudson/model/Run.java
+++ b/hudson-core/src/main/java/hudson/model/Run.java
@@ -1661,9 +1661,13 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
     public void doConsoleText(StaplerRequest req, StaplerResponse rsp) throws IOException {
         rsp.setContentType("text/plain;charset=UTF-8");
         // Prevent jelly from flushing stream so Content-Length header can be added afterwards
-        FlushProofOutputStream out = new FlushProofOutputStream(rsp.getCompressedOutputStream(req));
-        getLogText().writeLogTo(0,out);
-        out.close();
+        FlushProofOutputStream out = null;
+        try {
+            out = new FlushProofOutputStream(rsp.getCompressedOutputStream(req));
+            getLogText().writeLogTo(0,out);
+        } finally {
+            IOUtils.closeQuietly(out);
+        }
     }
 
     /**
diff --git a/hudson-core/src/main/java/hudson/model/UpdateCenter.java b/hudson-core/src/main/java/hudson/model/UpdateCenter.java
index b902b5c..99738ef 100644
--- a/hudson-core/src/main/java/hudson/model/UpdateCenter.java
+++ b/hudson-core/src/main/java/hudson/model/UpdateCenter.java
@@ -30,10 +30,7 @@ import hudson.lifecycle.Lifecycle;
 import hudson.model.UpdateSite.Data;
 import hudson.model.UpdateSite.Plugin;
 import hudson.model.listeners.SaveableListener;
-import hudson.util.DaemonThreadFactory;
-import hudson.util.IOException2;
-import hudson.util.PersistedList;
-import hudson.util.XStream2;
+import hudson.util.*;
 import org.springframework.security.Authentication;
 import org.apache.commons.io.input.CountingInputStream;
 import org.apache.commons.io.output.NullOutputStream;
@@ -531,27 +528,30 @@ public class UpdateCenter extends AbstractModelObject implements Saveable {
         public File download(DownloadJob job, URL src) throws IOException {
             URLConnection con = connect(job,src);
             int total = con.getContentLength();
-            CountingInputStream in = new CountingInputStream(con.getInputStream());
+            
             byte[] buf = new byte[8192];
             int len;
 
             File dst = job.getDestination();
             File tmp = new File(dst.getPath()+".tmp");
-            OutputStream out = new FileOutputStream(tmp);
-
+            OutputStream out = null;
+            CountingInputStream in = null;
+            
             LOGGER.info("Downloading "+job.getName());
             try {
+                in = new CountingInputStream(con.getInputStream());
+                out = new FileOutputStream(tmp);
                 while((len=in.read(buf))>=0) {
                     out.write(buf,0,len);
                     job.status = job.new Installing(total==-1 ? -1 : in.getCount()*100/total);
                 }
             } catch (IOException e) {
                 throw new IOException2("Failed to load "+src+" to "+tmp,e);
+            } finally {
+                IOUtils.closeQuietly(in);
+                IOUtils.closeQuietly(out);
             }
 
-            in.close();
-            out.close();
-
             if (total!=-1 && total!=tmp.length()) {
                 // don't know exactly how this happens, but report like
                 // http://www.ashlux.com/wordpress/2009/08/14/hudson-and-the-sonar-plugin-fail-maveninstallation-nosuchmethoderror/
diff --git a/hudson-core/src/main/java/hudson/scm/SCM.java b/hudson-core/src/main/java/hudson/scm/SCM.java
index d6c87c3..7c01001 100644
--- a/hudson-core/src/main/java/hudson/scm/SCM.java
+++ b/hudson-core/src/main/java/hudson/scm/SCM.java
@@ -37,6 +37,7 @@ import hudson.model.Descriptor;
 import hudson.model.Api;
 import hudson.model.Action;
 import hudson.model.AbstractProject.AbstractProjectDescriptor;
+import hudson.util.IOUtils;
 
 import java.io.File;
 import java.io.FileWriter;
@@ -557,14 +558,17 @@ public abstract class SCM implements Describable<SCM>, ExtensionPoint {
 //
 
     protected final boolean createEmptyChangeLog(File changelogFile, BuildListener listener, String rootTag) {
+        FileWriter w = null;
         try {
-            FileWriter w = new FileWriter(changelogFile);
+            w = new FileWriter(changelogFile);
             w.write("<"+rootTag +"/>");
             w.close();
             return true;
         } catch (IOException e) {
             e.printStackTrace(listener.error(e.getMessage()));
             return false;
+        } finally {
+            IOUtils.closeQuietly(w);
         }
     }
 
diff --git a/hudson-core/src/main/java/hudson/tools/JDKInstaller.java b/hudson-core/src/main/java/hudson/tools/JDKInstaller.java
index 0c92de0..4541031 100644
--- a/hudson-core/src/main/java/hudson/tools/JDKInstaller.java
+++ b/hudson-core/src/main/java/hudson/tools/JDKInstaller.java
@@ -338,13 +338,14 @@ public class JDKInstaller extends ToolInstaller {
         }
 
         url = new URL(form.attributeValue("action"));
+        PrintStream os = null;
         try {
             HttpURLConnection con = (HttpURLConnection) ProxyConfiguration.open(url);
             con.setRequestMethod("POST");
             con.setDoOutput(true);
             con.setRequestProperty("Cookie",cookie);
             con.setRequestProperty("Content-Type","application/x-www-form-urlencoded");
-            PrintStream os = new PrintStream(con.getOutputStream());
+            os = new PrintStream(con.getOutputStream());
 
             // select platform
             String primary=null,secondary=null;
@@ -382,10 +383,11 @@ public class JDKInstaller extends ToolInstaller {
                 else
                     os.print(URLEncoder.encode(value,"UTF-8"));
             }
-            os.close();
             return con;
         } catch (IOException e) {
             throw new IOException2("Failed to access "+url,e);
+        } finally {
+            IOUtils.closeQuietly(os);
         }
     }
 
diff --git a/hudson-core/src/main/java/hudson/triggers/SCMTrigger.java b/hudson-core/src/main/java/hudson/triggers/SCMTrigger.java
index 814cb64..9772e13 100644
--- a/hudson-core/src/main/java/hudson/triggers/SCMTrigger.java
+++ b/hudson-core/src/main/java/hudson/triggers/SCMTrigger.java
@@ -28,11 +28,7 @@ import hudson.model.Item;
 import hudson.model.Project;
 import hudson.model.SCMedItem;
 import hudson.model.AdministrativeMonitor;
-import hudson.util.FlushProofOutputStream;
-import hudson.util.FormValidation;
-import hudson.util.StreamTaskListener;
-import hudson.util.TimeUnit2;
-import hudson.util.SequentialExecutionQueue;
+import hudson.util.*;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.jelly.XMLOutput;
 import org.kohsuke.stapler.StaplerRequest;
@@ -305,9 +301,13 @@ public class SCMTrigger extends Trigger<SCMedItem> {
         public void doPollingLog(StaplerRequest req, StaplerResponse rsp) throws IOException {
             rsp.setContentType("text/plain;charset=UTF-8");
             // Prevent jelly from flushing stream so Content-Length header can be added afterwards
-            FlushProofOutputStream out = new FlushProofOutputStream(rsp.getCompressedOutputStream(req));
-            getPollingLogText().writeLogTo(0, out);
-            out.close();
+            FlushProofOutputStream out = null; 
+            try {
+                out = new FlushProofOutputStream(rsp.getCompressedOutputStream(req));
+                getPollingLogText().writeLogTo(0, out);
+            } finally {
+                IOUtils.closeQuietly(out);
+            }
         }
 
         public AnnotatedLargeText getPollingLogText() {
diff --git a/hudson-core/src/main/java/hudson/util/CompressedFile.java b/hudson-core/src/main/java/hudson/util/CompressedFile.java
index 977dabf..3a27a6d 100644
--- a/hudson-core/src/main/java/hudson/util/CompressedFile.java
+++ b/hudson-core/src/main/java/hudson/util/CompressedFile.java
@@ -107,13 +107,17 @@ public class CompressedFile {
 
         StringBuilder str = new StringBuilder((int)sizeGuess);
 
-        Reader r = new InputStreamReader(read());
-        char[] buf = new char[8192];
-        int len;
-        while((len=r.read(buf,0,buf.length))>0)
-           str.append(buf,0,len);
-        r.close();
-
+        Reader r = null;
+        try {
+            r = new InputStreamReader(read());
+            char[] buf = new char[8192];
+            int len;
+            while((len=r.read(buf,0,buf.length))>0) {
+                str.append(buf,0,len);
+            }
+        } finally {
+            r.close();
+        }
         return str.toString();
     }
 
diff --git a/hudson-core/src/main/java/hudson/util/io/ZipArchiver.java b/hudson-core/src/main/java/hudson/util/io/ZipArchiver.java
index ef1d00d..e0fc317 100644
--- a/hudson-core/src/main/java/hudson/util/io/ZipArchiver.java
+++ b/hudson-core/src/main/java/hudson/util/io/ZipArchiver.java
@@ -40,19 +40,24 @@ final class ZipArchiver extends Archiver {
     }
 
     public void visit(File f, String relativePath) throws IOException {
-        if(f.isDirectory()) {
-            ZipEntry dirZipEntry = new ZipEntry(relativePath+'/');
+        if (f.isDirectory()) {
+            ZipEntry dirZipEntry = new ZipEntry(relativePath + '/');
             // Setting this bit explicitly is needed by some unzipping applications (see HUDSON-3294).
             dirZipEntry.setExternalAttributes(BITMASK_IS_DIRECTORY);
             zip.putNextEntry(dirZipEntry);
             zip.closeEntry();
         } else {
             zip.putNextEntry(new ZipEntry(relativePath));
-            FileInputStream in = new FileInputStream(f);
-            int len;
-            while((len=in.read(buf))>0)
-                zip.write(buf,0,len);
-            in.close();
+            FileInputStream in = null;
+            try {
+                in = new FileInputStream(f);
+                int len;
+                while((len = in.read(buf)) > 0) {
+                    zip.write(buf, 0, len);
+                }
+            } finally {
+                in.close();
+            }
             zip.closeEntry();
         }
         entriesWritten++;
diff --git a/hudson-core/src/main/java/org/eclipse/hudson/init/PluginInstallationJob.java b/hudson-core/src/main/java/org/eclipse/hudson/init/PluginInstallationJob.java
index 7dfb239..7ee7091 100644
--- a/hudson-core/src/main/java/org/eclipse/hudson/init/PluginInstallationJob.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/init/PluginInstallationJob.java
@@ -18,6 +18,7 @@ package org.eclipse.hudson.init;
 
 import hudson.ProxyConfiguration;
 import hudson.Util;
+import hudson.util.IOUtils;
 import java.io.*;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -116,11 +117,11 @@ public final class PluginInstallationJob implements Runnable {
             }
         } catch (IOException e) {
             throw new IOException("Failed to load " + src + " to " + tmp, e);
+        } finally {
+           IOUtils.closeQuietly(out); 
+           IOUtils.closeQuietly(in); 
         }
 
-        in.close();
-        out.close();
-
         if (total != -1 && total != tmp.length()) {
             throw new IOException("Inconsistent file length: expected " + total + " but only got " + tmp.length());
         }