JRuby (please use github issues at http://bugs.jruby.org)
  1. JRuby (please use github issues at http://bugs.jruby.org)
  2. JRUBY-6611

LoadServiceResource Opens But Never Closes InputStreams When Constructed With a URL

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JRuby 1.7.0.pre1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      The line in question - https://github.com/jruby/jruby/blob/master/src/org/jruby/runtime/load/LoadServiceResource.java#L80

      The getInputStream method of LoadServiceResource, only when constructed with a URL, leaks via a call to resource.openStream().

      Because LoadServiceInputStream completely buffers the contents of the InputStream passed to it, you can close the InputStream created from resource.openStream() after constructing the new LoadServiceInputStream. There may be a better solution, but we've been using this workaround in a custom NonLeakingLoadServiceResource in TorqueBox for several months and it seems to work fine.

        Activity

        Hide
        Ben Browning added a comment -

        See TorqueBox's fixed version here - https://github.com/torquebox/torquebox/blob/3b772e24514203786d0b4a0113a4cb239b19b1f1/modules/core/src/main/java/org/torquebox/core/runtime/NonLeakingLoadServiceResource.java

        Not that our fix of closing the InputStream in LoadServiceResource only works because we know the implementation details of LoadServiceResourceInputStream and motivated by our need to find an easy way to hook into JRuby's LoadService implementation without too much effort. The correct long-term fix probably shouldn't rely on knowing the internals of LoadServiceResourceInputStream.

        Maybe a better solution is to change LoadServiceResourceInputStream to only have the byte array constructor and push the buffering of the InputStream to a byte array into LoadServiceResource, which can then safely close the stream after all the contents are buffered?

        Show
        Ben Browning added a comment - See TorqueBox's fixed version here - https://github.com/torquebox/torquebox/blob/3b772e24514203786d0b4a0113a4cb239b19b1f1/modules/core/src/main/java/org/torquebox/core/runtime/NonLeakingLoadServiceResource.java Not that our fix of closing the InputStream in LoadServiceResource only works because we know the implementation details of LoadServiceResourceInputStream and motivated by our need to find an easy way to hook into JRuby's LoadService implementation without too much effort. The correct long-term fix probably shouldn't rely on knowing the internals of LoadServiceResourceInputStream. Maybe a better solution is to change LoadServiceResourceInputStream to only have the byte array constructor and push the buffering of the InputStream to a byte array into LoadServiceResource, which can then safely close the stream after all the contents are buffered?
        Hide
        Charles Oliver Nutter added a comment -

        I went with James Abley's pull request fix, which wraps construction of LoadServiceResourceInputStream with exception-handling that closes the provided InputStream.

        commit 18f1726597d333f0084133679c88e70e8eda1d1c
        Merge: 428429e 54ef294
        Author: Charles Oliver Nutter <headius@headius.com>
        Date:   Sun Apr 29 23:05:31 2012 -0700
        
            Merge pull request #139 from jabley/master
            
            JRUBY-6611: Candidate fix
        
        commit 54ef294b2eaddcf6e883f946bdaae34104197046
        Author: James Abley <james.abley@gmail.com>
        Date:   Wed Apr 18 22:46:37 2012 +0100
        
            JRUBY-6611: Fix resource leak in LoadServiceResource
        
        
        Show
        Charles Oliver Nutter added a comment - I went with James Abley's pull request fix, which wraps construction of LoadServiceResourceInputStream with exception-handling that closes the provided InputStream. commit 18f1726597d333f0084133679c88e70e8eda1d1c Merge: 428429e 54ef294 Author: Charles Oliver Nutter <headius@headius.com> Date: Sun Apr 29 23:05:31 2012 -0700 Merge pull request #139 from jabley/master JRUBY-6611: Candidate fix commit 54ef294b2eaddcf6e883f946bdaae34104197046 Author: James Abley <james.abley@gmail.com> Date: Wed Apr 18 22:46:37 2012 +0100 JRUBY-6611: Fix resource leak in LoadServiceResource

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Ben Browning
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: