From 944b51634a0510c8d392752e6604451f997bd4f0 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 26 Dec 2016 09:06:23 +0100 Subject: [PATCH] JarScanner: Fix resource leak flagged by infer This fixes resource leak with JarFile instance acquired by call to ctor of JarScanner. Bug: Issue 5198 Change-Id: I80feabcf9e0bfd683d1d8c47f3fd6c9be3ff65a6 --- .../google/gerrit/pgm/SwitchSecureStore.java | 27 ++++++++++--------- .../com/google/gerrit/pgm/init/BaseInit.java | 13 +++++---- .../gerrit/server/plugins/JarScanner.java | 7 ++++- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/SwitchSecureStore.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/SwitchSecureStore.java index ac84e827fe..280795a2ba 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/SwitchSecureStore.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/SwitchSecureStore.java @@ -163,20 +163,21 @@ public class SwitchSecureStore extends SiteProgram { private String getNewSecureStoreClassName(Path secureStore) throws IOException { - JarScanner scanner = new JarScanner(secureStore); - List newSecureStores = - scanner.findSubClassesOf(SecureStore.class); - if (newSecureStores.isEmpty()) { - throw new RuntimeException(String.format( - "Cannot find implementation of SecureStore interface in %s", - secureStore.toAbsolutePath())); + try (JarScanner scanner = new JarScanner(secureStore)) { + List newSecureStores = + scanner.findSubClassesOf(SecureStore.class); + if (newSecureStores.isEmpty()) { + throw new RuntimeException(String.format( + "Cannot find implementation of SecureStore interface in %s", + secureStore.toAbsolutePath())); + } + if (newSecureStores.size() > 1) { + throw new RuntimeException(String.format( + "Found too many implementations of SecureStore:\n%s\nin %s", Joiner + .on("\n").join(newSecureStores), secureStore.toAbsolutePath())); + } + return Iterables.getOnlyElement(newSecureStores); } - if (newSecureStores.size() > 1) { - throw new RuntimeException(String.format( - "Found too many implementations of SecureStore:\n%s\nin %s", Joiner - .on("\n").join(newSecureStores), secureStore.toAbsolutePath())); - } - return Iterables.getOnlyElement(newSecureStores); } private String getCurrentSecureStoreClassName(SitePaths sitePaths) { diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java index 8ccdebce4a..16683874bd 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/BaseInit.java @@ -310,13 +310,12 @@ public class BaseInit extends SiteProgram { return null; } - try { - Path secureStoreLib = Paths.get(secureStore); - if (!Files.exists(secureStoreLib)) { - throw new InvalidSecureStoreException(String.format( - "File %s doesn't exist", secureStore)); - } - JarScanner scanner = new JarScanner(secureStoreLib); + Path secureStoreLib = Paths.get(secureStore); + if (!Files.exists(secureStoreLib)) { + throw new InvalidSecureStoreException(String.format( + "File %s doesn't exist", secureStore)); + } + try (JarScanner scanner = new JarScanner(secureStoreLib)) { List secureStores = scanner.findSubClassesOf(SecureStore.class); if (secureStores.isEmpty()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java index 3dc4c0be57..db564cc36c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java @@ -53,7 +53,7 @@ import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.Manifest; -public class JarScanner implements PluginContentScanner { +public class JarScanner implements PluginContentScanner, AutoCloseable { private static final int SKIP_ALL = ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES; private final JarFile jarFile; @@ -131,6 +131,11 @@ public class JarScanner implements PluginContentScanner { return findSubClassesOf(superClass.getName()); } + @Override + public void close() throws IOException { + jarFile.close(); + } + private List findSubClassesOf(String superClass) throws IOException { String name = superClass.replace('.', '/');