From 1fa262cd49d2d4436c8278f13f70bdf6d6d11254 Mon Sep 17 00:00:00 2001 From: Stephen Neal Date: Tue, 23 Jun 2015 13:47:27 +0930 Subject: [PATCH] Clean up; close streams, readers (potential memory leaks) --- .../com/threerings/getdown/data/Digest.java | 44 +-- .../threerings/getdown/tools/Digester.java | 47 ++-- .../getdown/tools/JarDiffPatcher.java | 250 ++++++++++-------- .../com/threerings/getdown/tools/Patcher.java | 5 +- .../threerings/getdown/util/VersionUtil.java | 13 +- 5 files changed, 201 insertions(+), 158 deletions(-) diff --git a/src/main/java/com/threerings/getdown/data/Digest.java b/src/main/java/com/threerings/getdown/data/Digest.java index 451ed7d..1ed9cad 100644 --- a/src/main/java/com/threerings/getdown/data/Digest.java +++ b/src/main/java/com/threerings/getdown/data/Digest.java @@ -102,28 +102,34 @@ public class Digest { MessageDigest md = getMessageDigest(); StringBuilder data = new StringBuilder(); - PrintWriter pout = new PrintWriter( - new OutputStreamWriter(new FileOutputStream(output), "UTF-8")); + PrintWriter pout = null; + try { + pout = new PrintWriter( + new OutputStreamWriter(new FileOutputStream(output), "UTF-8")); - // compute and append the MD5 digest of each resource in the list - for (Resource rsrc : resources) { - String path = rsrc.getPath(); - try { - String digest = rsrc.computeDigest(md, null); - note(data, path, digest); - pout.println(path + " = " + digest); - } catch (Throwable t) { - throw (IOException) new IOException( - "Error computing digest for: " + rsrc).initCause(t); + // compute and append the MD5 digest of each resource in the list + for (Resource rsrc : resources) { + String path = rsrc.getPath(); + try { + String digest = rsrc.computeDigest(md, null); + note(data, path, digest); + pout.println(path + " = " + digest); + } catch (Throwable t) { + throw (IOException) new IOException( + "Error computing digest for: " + rsrc).initCause(t); + } + } + + // finally compute and append the digest for the file contents + md.reset(); + byte[] contents = data.toString().getBytes("UTF-8"); + pout.println(DIGEST_FILE + " = " + StringUtil.hexlate(md.digest(contents))); + + } finally { + if (pout != null) { + pout.close(); } } - - // finally compute and append the digest for the file contents - md.reset(); - byte[] contents = data.toString().getBytes("UTF-8"); - pout.println(DIGEST_FILE + " = " + StringUtil.hexlate(md.digest(contents))); - - pout.close(); } /** diff --git a/src/main/java/com/threerings/getdown/tools/Digester.java b/src/main/java/com/threerings/getdown/tools/Digester.java index cd730d3..b932de1 100644 --- a/src/main/java/com/threerings/getdown/tools/Digester.java +++ b/src/main/java/com/threerings/getdown/tools/Digester.java @@ -20,6 +20,7 @@ import java.util.List; import org.apache.commons.codec.binary.Base64; +import com.samskivert.io.StreamUtil; import com.threerings.getdown.data.Application; import com.threerings.getdown.data.Digest; import com.threerings.getdown.data.Resource; @@ -81,26 +82,36 @@ public class Digester File inputFile = new File(appdir, Digest.DIGEST_FILE); File signatureFile = new File(appdir, Digest.DIGEST_FILE + Application.SIGNATURE_SUFFIX); - // initialize the keystore - KeyStore store = KeyStore.getInstance("JKS"); - FileInputStream storeInput = new FileInputStream(storePath); - store.load(storeInput, storePass.toCharArray()); - PrivateKey key = (PrivateKey)store.getKey(storeAlias, storePass.toCharArray()); + FileInputStream storeInput = null; + FileInputStream dataInput = null; + FileOutputStream signatureOutput = null; + try { + // initialize the keystore + KeyStore store = KeyStore.getInstance("JKS"); + storeInput = new FileInputStream(storePath); + store.load(storeInput, storePass.toCharArray()); + PrivateKey key = (PrivateKey)store.getKey(storeAlias, storePass.toCharArray()); - // sign the digest file - Signature sig = Signature.getInstance("SHA1withRSA"); - FileInputStream dataInput = new FileInputStream(inputFile); - byte[] buffer = new byte[8192]; - int length; + // sign the digest file + Signature sig = Signature.getInstance("SHA1withRSA"); + dataInput = new FileInputStream(inputFile); + byte[] buffer = new byte[8192]; + int length; + + sig.initSign(key); + while ((length = dataInput.read(buffer)) != -1) { + sig.update(buffer, 0, length); + } - sig.initSign(key); - while ((length = dataInput.read(buffer)) != -1) { - sig.update(buffer, 0, length); + // Write out the signature + signatureOutput = new FileOutputStream(signatureFile); + String signed = new String(Base64.encodeBase64(sig.sign())); + signatureOutput.write(signed.getBytes("utf8")); + + } finally { + StreamUtil.close(storeInput); + StreamUtil.close(dataInput); + StreamUtil.close(signatureOutput); } - - // Write out the signature - FileOutputStream signatureOutput = new FileOutputStream(signatureFile); - String signed = new String(Base64.encodeBase64(sig.sign())); - signatureOutput.write(signed.getBytes("utf8")); } } diff --git a/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java b/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java index e8aa2f2..3806c9e 100644 --- a/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java +++ b/src/main/java/com/threerings/getdown/tools/JarDiffPatcher.java @@ -5,25 +5,25 @@ package com.threerings.getdown.tools; -import java.io.IOException; -import java.io.OutputStream; import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; import java.io.InputStream; -import java.io.LineNumberReader; import java.io.InputStreamReader; +import java.io.LineNumberReader; -import java.util.Set; -import java.util.HashSet; -import java.util.Map; -import java.util.HashMap; +import java.util.ArrayList; import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.ArrayList; +import java.util.Map; +import java.util.Set; -import java.util.jar.JarOutputStream; -import java.util.jar.JarFile; import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.JarOutputStream; import com.threerings.getdown.util.ProgressObserver; @@ -43,122 +43,130 @@ public class JarDiffPatcher implements JarDiffCodes * * @throws IOException if any problem occurs during patching. */ - public void patchJar (String jarPath, String diffPath, OutputStream target, + public void patchJar (String jarPath, String diffPath, File target, ProgressObserver observer) throws IOException { File oldFile = new File(jarPath); File diffFile = new File(diffPath); - JarOutputStream jos = new JarOutputStream(target); - JarFile oldJar = new JarFile(oldFile); - JarFile jarDiff = new JarFile(diffFile); - Set ignoreSet = new HashSet(); + JarOutputStream jos = null; + JarFile oldJar = null; + JarFile jarDiff = null; + try { + jos = new JarOutputStream(new FileOutputStream(target)); + oldJar = new JarFile(oldFile); + jarDiff = new JarFile(diffFile); + Set ignoreSet = new HashSet(); - Map renameMap = new HashMap(); - determineNameMapping(jarDiff, ignoreSet, renameMap); - - // get all keys in renameMap - String[] keys = renameMap.keySet().toArray(new String[renameMap.size()]); - - // Files to implicit move - Set oldjarNames = new HashSet(); - Enumeration oldEntries = oldJar.entries(); - if (oldEntries != null) { - while (oldEntries.hasMoreElements()) { - oldjarNames.add(oldEntries.nextElement().getName()); + Map renameMap = new HashMap(); + determineNameMapping(jarDiff, ignoreSet, renameMap); + + // get all keys in renameMap + String[] keys = renameMap.keySet().toArray(new String[renameMap.size()]); + + // Files to implicit move + Set oldjarNames = new HashSet(); + Enumeration oldEntries = oldJar.entries(); + if (oldEntries != null) { + while (oldEntries.hasMoreElements()) { + oldjarNames.add(oldEntries.nextElement().getName()); + } } - } - - // size depends on the three parameters below, which is basically the - // counter for each loop that do the actual writes to the output file - // since oldjarNames.size() changes in the first two loop below, we - // need to adjust the size accordingly also when oldjarNames.size() - // changes - double size = oldjarNames.size() + keys.length + jarDiff.size(); - double currentEntry = 0; - - // Handle all remove commands - oldjarNames.removeAll(ignoreSet); - size -= ignoreSet.size(); - - // Add content from JARDiff - Enumeration entries = jarDiff.entries(); - if (entries != null) { - while (entries.hasMoreElements()) { - JarEntry entry = entries.nextElement(); - if (!INDEX_NAME.equals(entry.getName())) { - updateObserver(observer, currentEntry, size); - currentEntry++; - writeEntry(jos, entry, jarDiff); - - // Remove entry from oldjarNames since no implicit move is - // needed - boolean wasInOld = oldjarNames.remove(entry.getName()); - - // Update progress counters. If it was in old, we do not - // need an implicit move, so adjust total size. - if (wasInOld) { + + // size depends on the three parameters below, which is basically the + // counter for each loop that do the actual writes to the output file + // since oldjarNames.size() changes in the first two loop below, we + // need to adjust the size accordingly also when oldjarNames.size() + // changes + double size = oldjarNames.size() + keys.length + jarDiff.size(); + double currentEntry = 0; + + // Handle all remove commands + oldjarNames.removeAll(ignoreSet); + size -= ignoreSet.size(); + + // Add content from JARDiff + Enumeration entries = jarDiff.entries(); + if (entries != null) { + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + if (!INDEX_NAME.equals(entry.getName())) { + updateObserver(observer, currentEntry, size); + currentEntry++; + writeEntry(jos, entry, jarDiff); + + // Remove entry from oldjarNames since no implicit move is + // needed + boolean wasInOld = oldjarNames.remove(entry.getName()); + + // Update progress counters. If it was in old, we do not + // need an implicit move, so adjust total size. + if (wasInOld) { + size--; + } + + } else { + // no write is done, decrement size size--; } - - } else { - // no write is done, decrement size + } + } + + // go through the renameMap and apply move for each entry + for (String newName : keys) { + // Apply move command + String oldName = renameMap.get(newName); + + // Get source JarEntry + JarEntry oldEntry = oldJar.getJarEntry(oldName); + if (oldEntry == null) { + String moveCmd = MOVE_COMMAND + oldName + " " + newName; + throw new IOException("error.badmove: " + moveCmd); + } + + // Create dest JarEntry + JarEntry newEntry = new JarEntry(newName); + newEntry.setTime(oldEntry.getTime()); + newEntry.setSize(oldEntry.getSize()); + newEntry.setCompressedSize(oldEntry.getCompressedSize()); + newEntry.setCrc(oldEntry.getCrc()); + newEntry.setMethod(oldEntry.getMethod()); + newEntry.setExtra(oldEntry.getExtra()); + newEntry.setComment(oldEntry.getComment()); + + updateObserver(observer, currentEntry, size); + currentEntry++; + + writeEntry(jos, newEntry, oldJar.getInputStream(oldEntry)); + + // Remove entry from oldjarNames since no implicit move is needed + boolean wasInOld = oldjarNames.remove(oldName); + + // Update progress counters. If it was in old, we do not need an + // implicit move, so adjust total size. + if (wasInOld) { size--; } } - } - - // go through the renameMap and apply move for each entry - for (String newName : keys) { - // Apply move command - String oldName = renameMap.get(newName); - - // Get source JarEntry - JarEntry oldEntry = oldJar.getJarEntry(oldName); - if (oldEntry == null) { - String moveCmd = MOVE_COMMAND + oldName + " " + newName; - throw new IOException("error.badmove: " + moveCmd); + + // implicit move + Iterator iEntries = oldjarNames.iterator(); + if (iEntries != null) { + while (iEntries.hasNext()) { + String name = iEntries.next(); + JarEntry entry = oldJar.getJarEntry(name); + updateObserver(observer, currentEntry, size); + currentEntry++; + writeEntry(jos, entry, oldJar); + } } - - // Create dest JarEntry - JarEntry newEntry = new JarEntry(newName); - newEntry.setTime(oldEntry.getTime()); - newEntry.setSize(oldEntry.getSize()); - newEntry.setCompressedSize(oldEntry.getCompressedSize()); - newEntry.setCrc(oldEntry.getCrc()); - newEntry.setMethod(oldEntry.getMethod()); - newEntry.setExtra(oldEntry.getExtra()); - newEntry.setComment(oldEntry.getComment()); - updateObserver(observer, currentEntry, size); - currentEntry++; - writeEntry(jos, newEntry, oldJar.getInputStream(oldEntry)); - - // Remove entry from oldjarNames since no implicit move is needed - boolean wasInOld = oldjarNames.remove(oldName); - - // Update progress counters. If it was in old, we do not need an - // implicit move, so adjust total size. - if (wasInOld) { - size--; - } + } finally { + jos = closeStream(jos); + oldJar = closeFile(oldJar); + jarDiff = closeFile(jarDiff); } - - // implicit move - Iterator iEntries = oldjarNames.iterator(); - if (iEntries != null) { - while (iEntries.hasNext()) { - String name = iEntries.next(); - JarEntry entry = oldJar.getJarEntry(name); - updateObserver(observer, currentEntry, size); - currentEntry++; - writeEntry(jos, entry, oldJar); - } - } - updateObserver(observer, currentEntry, size); - - jos.finish(); } protected void updateObserver (ProgressObserver observer, @@ -281,7 +289,27 @@ public class JarDiffPatcher implements JarDiffCodes data.close(); } - protected static final int DEFAULT_READ_SIZE = 2048; + private static JarFile closeFile(JarFile jar) { + if (jar != null) { + try { + jar.close(); + } catch (IOException e) { + } + } + return null; + } + + private static JarOutputStream closeStream(JarOutputStream jar) { + if (jar != null) { + try { + jar.close(); + } catch (IOException e) { + } + } + return null; + } + + protected static final int DEFAULT_READ_SIZE = 2048; protected static byte[] newBytes = new byte[DEFAULT_READ_SIZE]; protected static byte[] oldBytes = new byte[DEFAULT_READ_SIZE]; diff --git a/src/main/java/com/threerings/getdown/tools/Patcher.java b/src/main/java/com/threerings/getdown/tools/Patcher.java index 6e8f0bc..777a73c 100644 --- a/src/main/java/com/threerings/getdown/tools/Patcher.java +++ b/src/main/java/com/threerings/getdown/tools/Patcher.java @@ -149,8 +149,6 @@ public class Patcher FileOutputStream fout = null; try { StreamUtil.copy(in = file.getInputStream(entry), fout = new FileOutputStream(patch)); - StreamUtil.close(fout); - fout = null; // move the current version of the jar to .old if (!FileUtil.renameTo(target, otarget)) { @@ -168,8 +166,7 @@ public class Patcher // now apply the patch to create the new target file patcher = new JarDiffPatcher(); - fout = new FileOutputStream(target); - patcher.patchJar(otarget.getPath(), patch.getPath(), fout, obs); + patcher.patchJar(otarget.getPath(), patch.getPath(), target, obs); } catch (IOException ioe) { if (patcher == null) { diff --git a/src/main/java/com/threerings/getdown/util/VersionUtil.java b/src/main/java/com/threerings/getdown/util/VersionUtil.java index 38527e5..2afd160 100644 --- a/src/main/java/com/threerings/getdown/util/VersionUtil.java +++ b/src/main/java/com/threerings/getdown/util/VersionUtil.java @@ -32,11 +32,10 @@ public class VersionUtil */ public static long readVersion (File vfile) { - FileInputStream fin = null; long fileVersion = -1; + BufferedReader bin = null; try { - fin = new FileInputStream(vfile); - BufferedReader bin = new BufferedReader(new InputStreamReader(fin)); + bin = new BufferedReader(new InputStreamReader(new FileInputStream(vfile))); String vstr = bin.readLine(); if (!StringUtil.isBlank(vstr)) { fileVersion = Long.parseLong(vstr); @@ -44,7 +43,7 @@ public class VersionUtil } catch (Exception e) { log.info("Unable to read version file: " + e.getMessage()); } finally { - StreamUtil.close(fin); + StreamUtil.close(bin); } return fileVersion; @@ -89,15 +88,15 @@ public class VersionUtil */ public static long readReleaseVersion (File relfile, String versRegex) { + BufferedReader in = null; try { - BufferedReader in = new BufferedReader(new FileReader(relfile)); + in = new BufferedReader(new FileReader(relfile)); String line = null, relvers = null; while ((line = in.readLine()) != null) { if (line.startsWith("JAVA_VERSION=")) { relvers = line.substring("JAVA_VERSION=".length()).replace('"', ' ').trim(); } } - in.close(); if (relvers == null) { log.warning("No JAVA_VERSION line in 'release' file", "file", relfile); @@ -108,6 +107,8 @@ public class VersionUtil } catch (Exception e) { log.warning("Failed to read version from 'release' file", "file", relfile, e); return 0L; + } finally { + StreamUtil.close(in); } }