From f4769c7ad702fa529a790106ea5c318a7af5c7b7 Mon Sep 17 00:00:00 2001
From: Christian Melchior <christian@ilios.dk>
Date: Sun, 12 Nov 2017 15:17:19 +0100
Subject: [PATCH] Remove close method on ClassPath. Add unit tests for new
 behaviour.

---
 Readme.html                                |   8 +++
 pom.xml                                    |  10 ++-
 src/main/javassist/ByteArrayClassPath.java |   5 --
 src/main/javassist/ClassClassPath.java     |   6 --
 src/main/javassist/ClassPath.java          |   7 --
 src/main/javassist/ClassPoolTail.java      |  71 ++++++++++-----------
 src/main/javassist/LoaderClassPath.java    |   7 --
 src/main/javassist/URLClassPath.java       |   5 --
 src/test/javassist/JvstTest.java           |  22 +++++++
 src/test/resources/Readme.txt              |   7 ++
 src/test/resources/empty.jar               | Bin 0 -> 3360 bytes
 11 files changed, 79 insertions(+), 69 deletions(-)
 create mode 100644 src/test/resources/Readme.txt
 create mode 100755 src/test/resources/empty.jar

diff --git a/Readme.html b/Readme.html
index 563ce36f..4685062a 100644
--- a/Readme.html
+++ b/Readme.html
@@ -281,6 +281,14 @@ see javassist.Dump.
 
 <h2>Changes</h2>
 
+<p>-version 3.23 on MMM DD, YYYY
+
+<ul>
+    <li>Fix leaking file handlers in ClassPool and removed ClassPath.close(). Github issue #165.
+</ul>
+</p>
+
+
 <p>-version 3.22 on October 10, 2017
 
 <ul>
diff --git a/pom.xml b/pom.xml
index 06d1f6da..6259547a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -137,6 +137,11 @@
   <build>
     <sourceDirectory>src/main/</sourceDirectory>
     <testSourceDirectory>src/test/</testSourceDirectory>
+    <testResources>
+      <testResource>
+        <directory>src/test/resources</directory>
+      </testResource>
+    </testResources>
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
@@ -158,7 +163,10 @@
           <includes>
             <include>javassist/JvstTest.java</include>
           </includes>
-	  <forkMode>once</forkMode>
+          <additionalClasspathElements>
+            <additionalClasspathElement>resources</additionalClasspathElement>
+          </additionalClasspathElements>
+	      <forkMode>once</forkMode>
           <workingDirectory>runtest</workingDirectory>
         </configuration>
       </plugin>
diff --git a/src/main/javassist/ByteArrayClassPath.java b/src/main/javassist/ByteArrayClassPath.java
index d385eddc..00397470 100644
--- a/src/main/javassist/ByteArrayClassPath.java
+++ b/src/main/javassist/ByteArrayClassPath.java
@@ -62,11 +62,6 @@ public class ByteArrayClassPath implements ClassPath {
         this.classfile = classfile;
     }
 
-    /**
-     * Closes this class path.
-     */
-    public void close() {}
-
     public String toString() {
         return "byte[]:" + classname;
     }
diff --git a/src/main/javassist/ClassClassPath.java b/src/main/javassist/ClassClassPath.java
index 3befbf4c..e1c44f08 100644
--- a/src/main/javassist/ClassClassPath.java
+++ b/src/main/javassist/ClassClassPath.java
@@ -91,12 +91,6 @@ public class ClassClassPath implements ClassPath {
         return thisClass.getResource(filename);
     }
 
-    /**
-     * Does nothing.
-     */
-    public void close() {
-    }
-
     public String toString() {
         return thisClass.getName() + ".class";
     }
diff --git a/src/main/javassist/ClassPath.java b/src/main/javassist/ClassPath.java
index 0c605de2..60fe80e9 100644
--- a/src/main/javassist/ClassPath.java
+++ b/src/main/javassist/ClassPath.java
@@ -58,11 +58,4 @@ public interface ClassPath {
      * @return null if the specified class file could not be found.
      */
     URL find(String classname);
-
-    /**
-     * This method is invoked when the <code>ClassPath</code> object is
-     * detached from the search path.  It will be an empty method in most of
-     * classes.
-     */
-    void close();
 }
diff --git a/src/main/javassist/ClassPoolTail.java b/src/main/javassist/ClassPoolTail.java
index 1b580855..8e03873c 100644
--- a/src/main/javassist/ClassPoolTail.java
+++ b/src/main/javassist/ClassPoolTail.java
@@ -25,6 +25,9 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -72,8 +75,6 @@ final class DirClassPath implements ClassPath {
         return null;
     }
 
-    public void close() {}
-
     public String toString() {
         return directory;
     }
@@ -118,67 +119,63 @@ final class JarDirClassPath implements ClassPath {
 
         return null;    // not found
     }
-
-    public void close() {
-        if (jars != null)
-            for (int i = 0; i < jars.length; i++)
-                jars[i].close();
-    }
 }
 
 final class JarClassPath implements ClassPath {
-    JarFile jarfile;
+    List<String> jarfileEntries;
     String jarfileURL;
 
     JarClassPath(String pathname) throws NotFoundException {
+        JarFile jarfile = null;
         try {
             jarfile = new JarFile(pathname);
+            jarfileEntries = new ArrayList<String>();
+            for (JarEntry je:Collections.list(jarfile.entries()))
+                if (je.getName().endsWith(".class"))
+                    jarfileEntries.add(je.getName());
             jarfileURL = new File(pathname).getCanonicalFile()
-                                           .toURI().toURL().toString();
+                    .toURI().toURL().toString();
             return;
+        } catch (IOException e) {}
+        finally {
+            if (null != jarfile)
+                try {
+                    jarfile.close();
+                } catch (IOException e) {}
         }
-        catch (IOException e) {}
         throw new NotFoundException(pathname);
     }
 
+    @Override
     public InputStream openClassfile(String classname)
-        throws NotFoundException
+            throws NotFoundException
     {
-        try {
-            String jarname = classname.replace('.', '/') + ".class";
-            JarEntry je = jarfile.getJarEntry(jarname);
-            if (je != null)
-                return jarfile.getInputStream(je);
-            else
-                return null;    // not found
-        }
-        catch (IOException e) {}
-        throw new NotFoundException("broken jar file?: "
-                                    + jarfile.getName());
+        URL jarURL = find(classname);
+        if (null != jarURL)
+            try {
+                return jarURL.openConnection().getInputStream();
+            }
+            catch (IOException e) {
+                throw new NotFoundException("broken jar file?: "
+                        + classname);
+            }
+        return null;
     }
 
+    @Override
     public URL find(String classname) {
         String jarname = classname.replace('.', '/') + ".class";
-        JarEntry je = jarfile.getJarEntry(jarname);
-        if (je != null)
+        if (jarfileEntries.contains(jarname))
             try {
-                return new URL("jar:" + jarfileURL + "!/" + jarname);
+                return new URL(String.format("jar:%s!/%s", jarfileURL, jarname));
             }
             catch (MalformedURLException e) {}
-
         return null;            // not found
     }
 
-    public void close() {
-        try {
-            jarfile.close();
-            jarfile = null;
-        }
-        catch (IOException e) {}
-    }
-
+    @Override
     public String toString() {
-        return jarfile == null ? "<null>" : jarfile.toString();
+        return jarfileURL == null ? "<null>" : jarfileURL.toString();
     }
 }
 
@@ -235,8 +232,6 @@ final class ClassPoolTail {
                     else
                         list = list.next;
             }
-
-        cp.close();
     }
 
     public ClassPath appendSystemPath() {
diff --git a/src/main/javassist/LoaderClassPath.java b/src/main/javassist/LoaderClassPath.java
index 6921ca6e..299fdb85 100644
--- a/src/main/javassist/LoaderClassPath.java
+++ b/src/main/javassist/LoaderClassPath.java
@@ -94,11 +94,4 @@ public class LoaderClassPath implements ClassPath {
             return url;
         }
     }
-
-    /**
-     * Closes this class path.
-     */
-    public void close() {
-        clref = null;
-    }
 }
diff --git a/src/main/javassist/URLClassPath.java b/src/main/javassist/URLClassPath.java
index bea0b3c2..5eeef3a9 100644
--- a/src/main/javassist/URLClassPath.java
+++ b/src/main/javassist/URLClassPath.java
@@ -112,11 +112,6 @@ public class URLClassPath implements ClassPath {
         return null; 
     }
 
-    /**
-     * Closes this class path.
-     */
-    public void close() {}
-
     /**
      * Reads a class file on an http server.
      *
diff --git a/src/test/javassist/JvstTest.java b/src/test/javassist/JvstTest.java
index d660a335..11cdbe96 100644
--- a/src/test/javassist/JvstTest.java
+++ b/src/test/javassist/JvstTest.java
@@ -1,6 +1,7 @@
 package javassist;
 
 import junit.framework.*;
+import java.io.File;
 import java.io.FileInputStream;
 import java.lang.reflect.Method;
 import javassist.bytecode.*;
@@ -63,6 +64,27 @@ public class JvstTest extends JvstTestRoot {
         assertTrue("[class path: ]".equals(pool.toString()));
     }
 
+    public void testReleaseJarClassPathFileHandle() throws Exception {
+        String jarFileName = "./empty.jar";
+        ClassLoader classLoader = getClass().getClassLoader();
+        File jarFile = new File(classLoader.getResource(jarFileName).getFile());
+        assertTrue(jarFile.exists());
+
+        // Prepare class pool and force it to open the Jar file
+        ClassPool pool = ClassPool.getDefault();
+        ClassPath cp = pool.appendClassPath(jarFile.getAbsolutePath());
+        assertNull(cp.openClassfile("nothere.Dummy"));
+
+        // Assert that it is possible to delete the jar file.
+        // On Windows deleting an open file will fail, while on on Mac/Linux this is always possible.
+        // This check will thus only fail on Windos if the file is still open.
+        assertTrue(jarFile.delete());
+    }
+
+    public void testJarClassPath() throws Exception {
+        // TODO: Verify that classes can be loaded from a JarClassPath
+    }
+
     public void testSubtype() throws Exception {
         CtClass cc = sloader.get("test1.Subtype");
         assertTrue(cc.subtypeOf(cc));
diff --git a/src/test/resources/Readme.txt b/src/test/resources/Readme.txt
new file mode 100644
index 00000000..93de248e
--- /dev/null
+++ b/src/test/resources/Readme.txt
@@ -0,0 +1,7 @@
+This directory contains files used by the unit tests.
+
+empty.jar:
+An empty, but valid, jar file.
+
+
+
diff --git a/src/test/resources/empty.jar b/src/test/resources/empty.jar
new file mode 100755
index 0000000000000000000000000000000000000000..3a12dbae0407a4d0a5c543f5424112cdd0f09afb
GIT binary patch
literal 3360
zcmb7Gc{r4N8y?HpPB99f?Z}es6Gr5tFvdh9dkPIhMwS@Hme3d38C@Apk#QpA5M@bZ
zUyfzSp={YDTj`XNopa{pWQM++-0$`N@m}wB-@oN~p8I)>Pz;P*AQs94mbs|{q5&6(
z5p+h!OiNDB0D1I`mVq8p$J7jZ2DwfKfe6Me26gg~@>67uMHXF;q<cJZRyN`0tUCK|
zis4DFE)j<cNMWld*bzaZ?1H|&4!#b`62b3ldPi7`Y)#T7YsH4|x(M@-G2K1{1)&N=
z&s16#n9SGpqNssh#Zg?ca<0Ncj?5MuV+>(3@hURUu<cdae=>p}70B|NnJd^|z#OyB
zF`%gYQw8@qlm3yEb``ukBD}sJayEyn9y3BNW<TtB_BP~lO_QZ5Gb|}AS)TK4B{rGK
zn2Aq8`L=jJOp^4cWNG5zS$jc3T<^j!AO0%oQsb+2Z!0^#Jpc1(Y{^iBQ_u@8?BxfM
z4wz?le#IENFz3vf$+(p3-ER=>$RJJA3~8jE3KBeZa0-{1ezSi@w$32KbVZvanPJc|
zp^EgBiYffv`DD!J^^6;lS9rPQhV^`xTAF@kRoAu1$U?nTZR~J;?H_FpX`A|R3B8o^
zArNGQVrFrWP|7u<1A#Q@L6q<BMTV6UrLD*?Y!HFCQ8*i*5}!+L59tBQ|1Bcc)!oYv
zYUgI3Yg*&hrwOU}%x!p@T@MUZcDi4i!5&)ANf$y8y=pb1Fw{MhHh8S_f5Z7irR*eB
z#v<mG8Von=5AzHgS~)?24IDM&<?uKf`&!hmr2e%g+vunf+3fgl+;)nm6q8lE!~2@h
zqwWfoi1rUQW0}&Zdh4m*t?%AT)*Xas6dfp@)San%YdYdo_%7K)T`FBi>M8OL|7xOu
z=y_5ZQC<n2E0YedF*=e9!&|8adqJdEvIOY;NzLVMvC;{*f@D}?TZiD2@BeroK;W}z
z$+t4GM?_+uV8t(}6Q!h35T5XfR-SlUtEb*C!*o~m8BL#m%$?`d#g~`fm3vGO2%4-K
zn2+fVRc_5kO5JHir7dK>K4;69?rh8!h~W-(C*%?BHJ*l$O{^2p$dL=`$mT&il5jHa
zBX6GhsQRmz<2~F-_r2034wp`5D#%C6!%LI=6szm(k5!IEeLRw+)0`=yMu=NF9IJqS
z2(jbKi*qbb36+uU$r%?-xiXUZGWtHj7w5O!*pV!<kVI%{Vzua#m=^dm6B~>Tg9}EM
zCB>;Lo(PI(p02VqcYfmX1>c;LD%K=;x(sT7{FHTk4F}^Je?medq`y2{oCCAwTZCYx
zOx!RRj(xeIRTVAZiY6{W=+~+j)>1U4Y+$YbWnWsWEb|q@YRexy=Wmrz!@+dpolpMN
zYt;*iL8Dz5-hlNsSZa}%YTIyP-J2dQfgY=<xd*+R9F1)*%Ef5RBW(9{Ku$t$i&EcT
z3CpRU1%RY|rVlGprzBq#<8DdIL7RK7wmw+5qr0W<PB;4bTRIMZ35xSoggWEgY(4iR
zCiZV)8?~_~NrfMibjP_u-SIegEZ)lr>&e|(!$vOx0kxOeC_G-!5@+R^H{&F|V|MWF
z-e>CZq_65x8^@6QM12Ck+8vdqE-~%`--ghOi<rp4@DNW!XaR3t=SUMzJe?D=LLecy
zZkoxGTyez%t!+TmiZoeF@-Ho!wwZX^aLS;FEn803zMo^!qVGs05VI3RFQ^Y?%hxII
z_x(cp#}9SI^)kU^vnfo<qzzpRH-X;DuMc9#IxBM)K9KENa2PtPU3@pbsxgwah$Vgb
z4Ji@*$HUf`E@s)lt8uybg~A4ZQzPkAZJlb(SHc~|nRmZLL3Is2S(x!iNK|$O4indX
zI-h@D4Xe!irN3QL<RID4NIVQG!KM<7LI?lH5<Po<d12`hj<LWkzr|NowbZ$RsDx#b
zmDaeRZEO{&WXR2dPg_w{bp*$Du*Ql$wl3G2W|046Av4eOu=<|*SXznpJ2?L}6<!A~
zX)&k#{T6Bi*Kx;1Ivw?D*Lf?1nQqTD9w9!foga5;7j3Jkj?0Nlp44<&ZNx3;2O-wW
z%{lsuBwvij<0XYI)@4Ol$quLTWKXQz&$blYXQ+E1YLT!g0o;515lcp4ioqN0YVedo
z0#UMiPYKBK4~J3_d*ZRv@b+FwT;H(wUOI=qqoK3Wpe|mv_4M?P&3CKSgdi@}^hS<>
z(xS7b;8Dr<Qb8T}(hK0XBGzY;M`Xve8yLq6O?1*m7#6c4Dt?kHdoeZP<lz0Fs*bZ7
z^d90p!0gE6$hjH`^*Scrd1&RQU=`S}QSDLQsY5>R5DAAAn-jh4h~xkfl<OM!YS(ET
z>tJs@%64A+RwCRj)DoACo3-=SXBmE;r{9%ROla?z)W6ouBE%mOo`Z(>PavSt8FwTS
zPs_|1jScsUSf8wZTX?XIf$+elokPQ!<$NIXsS;blIzx%h)LUODH;E2N2k?-PV<+YK
zyg*8z=;*mX)LD#n764|k?bvGLJ0`Q8Dgf*2U;xYwR3uT>+)|;X0aF95rvp<16)$ON
zTQdYL|EqO>T^4YDDhw$n*;;n959~$;e4mQgZ^_%cz;052>r>JHZ&I6yyi<_YI04V6
z0t)`-VB7b$DY(4@(8>e%PU{bV`%=Nc^uzKLFQ(Pq8ISF$<j=h6HM=nYZ=vGRKMcTW
zc4Gm~Ld7HvYtv_T;{v`yg*ascfFA-YZgT&BaA?a3a1|<?zN4T+^%x@*m~zD+(0<Bu
Loq;mWY?RC2bS)wY

literal 0
HcmV?d00001

-- 
2.39.5