BridgeMethodResolver resolveAll does not close InputStream #115

yogregg · 2017-12-04T17:42:20Z

I uncovered a problem in BridgeMethodResolver resolveAll: it opens an InputStream that is never closed.

The problem is in cglib/cglib/src/main/java/net/sf/cglib/proxy/BridgeMethodResolver.java, in line 63 of resolveAll, which has this code:

        try {
            new ClassReader(classLoader.getResourceAsStream(owner.getName().replace('.', '/') + ".class"))
              .accept(new BridgedFinder(bridges, resolved),
                      ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG);
        } catch(IOException ignored) {}

The stream returned by classsLoader.getResourceAsStream is never closed. The ClassReader instance that it is passed to doesn't close it, and nothing else holds a reference to it that could be used to close it.

The fix is simple, and I've attached a patch file for it. The heart of it is this change to the above code block:

        InputStream classStream = null;
        try {
            classStream = classLoader.getResourceAsStream(owner.getName().replace('.', '/') + ".class");
            new ClassReader(classStream)
              .accept(new BridgedFinder(bridges, resolved),
                      ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG);
        } catch(IOException ignored) {}
        finally {
          try {
            if (classStream != null) {
              classStream.close();
            }
          }
          catch (IOException ignoreMe) {}
        }

Here is the patch file:
BridgeMethodResolver.zip

I noticed this when using cglib indirectly via the Spring framework. When my web application is deployed to Tomcat 8.5 on Windows, Tomcat refuses to undeploy the web application -- I have to shut down the entire Tomcat server. This happens because Tomcat's classloader keeps a Windows file lock on jar files when someone has an open input stream on one of the classes in the jar file (when opened by classLoader.getResourceAsStream). This is a substantial problem for users of my web application, since they cannot upgrade it without shutting down Tomcat, which often means that user get kicked out of all other web applications that Tomcat is hosting.

The change above resolved the problem I was having in Spring with Tomcat 8.5.

The text was updated successfully, but these errors were encountered:

raphw · 2017-12-05T19:54:41Z

You are very much right. I just applied your patch with a few modifications.

This is serious enough to fix despite of the dormant state.

@sameb could you trigger a new release?

yogregg · 2017-12-05T20:46:09Z

Thanks very much @raphw for the very prompt attention.

sameb · 2017-12-05T20:50:21Z

lets see if i can remember how to do this...

sameb · 2017-12-05T20:54:35Z

looks like travis is failing for JDK6 for a weird reason:

$ mvn test -B -V
The JAVA_HOME environment variable is not defined correctly
This environment variable is needed to run this program
NB: JAVA_HOME should point to a JDK not a JRE

any idea what that's about?

sameb · 2017-12-05T20:59:24Z

(also did you want to update the ASM version before i do a new release?)

yogregg · 2017-12-05T21:05:22Z

From the build log:

$ jdk_switcher use openjdk6
Switching to OpenJDK6 (java-1.6.0-openjdk-amd64), JAVA_HOME will be set to /usr/lib/jvm/java-6-openjdk-amd64
update-java-alternatives: directory does not exist: /usr/lib/jvm/java-1.6.0-openjdk-amd64

My guess is that because it can't find /usr/lib/jvm/java-1.6.0-openjdk-amd64, the jdk_switcher doesn't actually set JAVA_HOME, and it ends up using whatever is the default Java which appears to be a Java 8 JRE.

yogregg · 2017-12-05T21:51:15Z

I found some more info on the travis openjdk6 failure. It looks like travis no longer supports openjdk6 out of the box due to a switch in the Ubuntu images it uses. See travis-ci/travis-ci#8199 It can still be made to work, though, as shown for example here: scala/scala-parser-combinators@daf9fe2

It looks like the essential peice is to add this to your project's .travis.yml file:

addons:
  apt:
    packages:
      - openjdk-6-jdk

sameb · 2017-12-05T21:58:44Z

nice! added in baeb5e3, though now it fails with:

Exception in thread "main" java.lang.UnsupportedClassVersionError: org/apache/maven/cli/MavenCli : Unsupported major.minor version 51.0
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClass(ClassLoader.java:648)
at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
at java.net.URLClassLoader.defineClass(URLClassLoader.java:272)
at java.net.URLClassLoader.access$000(URLClassLoader.java:68)
at java.net.URLClassLoader$1.run(URLClassLoader.java:207)
at java.net.URLClassLoader$1.run(URLClassLoader.java:201)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:200)
at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClassFromSelf(ClassRealm.java:401)
at org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy.loadClass(SelfFirstStrategy.java:42)
at org.codehaus.plexus.classworlds.realm.ClassRealm.unsynchronizedLoadClass(ClassRealm.java:271)
at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:254)
at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:239)
at org.codehaus.plexus.classworlds.launcher.Launcher.getMainClass(Launcher.java:144)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:266)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)

... looks like maven doesn't like java6. :-/

raphw · 2017-12-05T22:25:00Z

I updated ASM, good catch!

yogregg · 2017-12-05T22:28:29Z

Hmm, travis has also updated the maven version it uses to 3.3+, which no longer support Java 6. Maven 3.2.5 was the last one to support Java 6. You could try another change to .travis.yml, this time to add these four lines to the beginning of the before_script:

  - wget https://archive.apache.org/dist/maven/maven-3/3.2.5/binaries/apache-maven-3.2.5-bin.zip
  - unzip -qq apache-maven-3.2.5-bin.zip
  - export M2_HOME="$PWD/apache-maven-3.2.5"
  - export PATH="$M2_HOME/bin:$PATH"

So the complete before_script section would look like this:

before_script:
  - wget https://archive.apache.org/dist/maven/maven-3/3.2.5/binaries/apache-maven-3.2.5-bin.zip
  - unzip -qq apache-maven-3.2.5-bin.zip
  - export M2_HOME="$PWD/apache-maven-3.2.5"
  - export PATH="$M2_HOME/bin:$PATH"
  - echo "MAVEN_OPTS='-Xmx512m -Dgpg.skip=true'" > ~/.mavenrc
  - if [[ "x$JDK" == *'x9'* ]]; then remove_dir_from_path $JAVA_HOME/bin; export JAVA_HOME=/usr/lib/jvm/java-9-oracle; export PATH=$JAVA_HOME/bin:$PATH; java -Xmx32m -version; fi

yogregg · 2017-12-11T18:18:32Z

I'm glad to see you got past the build / test problems. By the way, I've filed a Jira issue for Spring about this too (https://jira.spring.io/browse/SPR-16267) and Juergen Hoeller responded that they'll pick up cglib 3.2.6 once it is released. Do you know when that might be? I'm hoping that the timing will allow this to make it into the next Spring 4.3.x release.

raphw · 2017-12-11T22:15:39Z

I cannot release cglib but maybe @sameb can do it now that the build is ok again.

sameb · 2017-12-11T22:28:54Z

i'll take a look tomorrow evening

quaff · 2018-01-08T01:48:21Z

Please release 3.2.6.

jhoeller · 2018-01-10T12:18:26Z

From a Spring perspective, we need CGLIB 3.2.6 by January 22nd for pick-up in the Spring Framework 5.0.3 and 4.3.14 releases. @sameb, your timely efforts would be much appreciated!

yogregg · 2018-01-11T12:27:06Z

Yes @sameb please please release this in time for the Spring release. My customers are really frustrated by their inability to undeploy/redeploy my web applications in Tomcat 8.5, and to fix that I need not just the new cglib but the a new Spring that incorporates it.

rtenhove · 2018-01-11T17:29:16Z

I am running into this exact problem, through Spring 4.3.X. I'd love to see this fixed ASAP, so Spring can pick this up in their next releases. @sameb please cut a new release incorporating @yogregg 's fixes. If I can do anything to help, please let me know.

yogregg · 2018-01-18T15:14:13Z

@sameb are you reading? A response with your plans, please? The Spring release deadline is now just a few days away on this coming Monday Jan 22.

sameb · 2018-01-18T15:16:04Z

Yes reading. Life is getting in the way. Will do this when I have some free time. Hopefully before the 22nd.

…

On Thu, Jan 18, 2018, 10:14 AM yogregg ***@***.***> wrote: @sameb <https://github.com/sameb> are you reading? A response with your plans, please? The Spring release deadline is now just a few days away on this coming Monday Jan 22. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#115 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACC7Es-d2nQgwR9apT60-Uc2rt0TX53Zks5tL1_GgaJpZM4Q1CcP> .

sameb · 2018-01-19T21:09:58Z

just released 3.2.6. should be a couple hours before it's available on maven.

yogregg · 2018-01-19T22:49:18Z

Thank you very much @sameb

jhoeller · 2018-01-20T13:30:56Z

Picked up for the upcoming Spring Framework releases now. Thanks a lot, @sameb!

raphw closed this as completed in 3719976 Dec 5, 2017

spring-projects-issues mentioned this issue Jan 11, 2019

Embedded cglib 3.2.5 not closing input streams that read class files [SPR-16267] spring-projects/spring-framework#20814

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BridgeMethodResolver resolveAll does not close InputStream #115

BridgeMethodResolver resolveAll does not close InputStream #115

yogregg commented Dec 4, 2017

raphw commented Dec 5, 2017

yogregg commented Dec 5, 2017

sameb commented Dec 5, 2017

sameb commented Dec 5, 2017 •

edited

sameb commented Dec 5, 2017

yogregg commented Dec 5, 2017

yogregg commented Dec 5, 2017

sameb commented Dec 5, 2017

raphw commented Dec 5, 2017

yogregg commented Dec 5, 2017

yogregg commented Dec 11, 2017

raphw commented Dec 11, 2017

sameb commented Dec 11, 2017

quaff commented Jan 8, 2018

jhoeller commented Jan 10, 2018

yogregg commented Jan 11, 2018

rtenhove commented Jan 11, 2018

yogregg commented Jan 18, 2018

sameb commented Jan 18, 2018 via email

sameb commented Jan 19, 2018

yogregg commented Jan 19, 2018

jhoeller commented Jan 20, 2018

BridgeMethodResolver resolveAll does not close InputStream #115

BridgeMethodResolver resolveAll does not close InputStream #115

Comments

yogregg commented Dec 4, 2017

raphw commented Dec 5, 2017

yogregg commented Dec 5, 2017

sameb commented Dec 5, 2017

sameb commented Dec 5, 2017 • edited

sameb commented Dec 5, 2017

yogregg commented Dec 5, 2017

yogregg commented Dec 5, 2017

sameb commented Dec 5, 2017

raphw commented Dec 5, 2017

yogregg commented Dec 5, 2017

yogregg commented Dec 11, 2017

raphw commented Dec 11, 2017

sameb commented Dec 11, 2017

quaff commented Jan 8, 2018

jhoeller commented Jan 10, 2018

yogregg commented Jan 11, 2018

rtenhove commented Jan 11, 2018

yogregg commented Jan 18, 2018

sameb commented Jan 18, 2018 via email

sameb commented Jan 19, 2018

yogregg commented Jan 19, 2018

jhoeller commented Jan 20, 2018

sameb commented Dec 5, 2017 •

edited