groovy
  1. groovy
  2. GROOVY-5029

XmlSlurper does not close InputStream, leaks file handles/resources

    Details

    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

        public GPathResult parse(final File file) throws IOException, SAXException {
        final InputSource input = new InputSource(new FileInputStream(file));
          
          input.setSystemId("file://" + file.getAbsolutePath());
          
          return parse(input);
          
        }
      

      I stepped through the SAX code but couldn't see the InputSource being closed so XmlSlurper is leaking file descriptors.

      I think the Reader and InputStream methods' JavaDoc should also mention that the streams aren't closed.

      GPathResult parse(final String uri) also seems to leak though that could also be a Xerces issue because I don't see it closing its input streams, either.

      Shameless plug: Use Resource.close

      I tried to get the VM to crash. After checking ulimit -a, I ran

      5000.times { new XmlSlurper().parse(new File('web.xml')) }
      

      This caused a lot of FileInputStreams to be created but I couldn't reach my limit of 1024 due to the finalizer in FileInputStream. Still, in the screenshot below you can see how the FileInputStream instances are accumulating on the heap.

      Fixing this is of course easy:

        public GPathResult parse(final File file) throws IOException, SAXException {
         FileInputStream in = null;
         try {
           in = new FileInputStream(file);
           final InputSource input = new InputSource(in);
           input.setSystemId("file://" + file.getAbsolutePath());   
           return parse(input);
         }
         finally {
           close(in); // or if (in != null) try { in.close() } catch (IOException ex) {}
         }   
        }
      

        Activity

        Hide
        Paul King added a comment -

        Fixed. FileInputStream now closed. Javadocs added. Uri version I don't think is in our control, so it was left alone.

        Show
        Paul King added a comment - Fixed. FileInputStream now closed. Javadocs added. Uri version I don't think is in our control, so it was left alone.
        Paul King made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Paul King [ paulk ]
        Fix Version/s 1.7.11 [ 17244 ]
        Fix Version/s 1.8.3 [ 17657 ]
        Fix Version/s 1.9-beta-4 [ 17656 ]
        Resolution Fixed [ 1 ]
        Hide
        Johann Burkard added a comment -

        Thanks Paul!

        Show
        Johann Burkard added a comment - Thanks Paul!
        Paul King made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Paul King
            Reporter:
            Johann Burkard
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: