CODEWITHSUNDEEP
javaexceptioninputstreamsonarqube
user471011
user471011

Reputation: 7384

Sonar violation: "Method may fail to close stream on exception"

I have this method:

 private void unZipElementsTo(String inputZipFileName, String destPath) throws FileNotFoundException, IOException {

        OutputStream out = null;
        InputStream in = null;
        ZipFile zf = null;

        try {
            zf = new ZipFile(inputZipFileName);

            for (Enumeration<? extends ZipEntry> em = zf.entries(); em.hasMoreElements();) {
                ZipEntry entry = em.nextElement();
                String targetFile = destPath + FILE_SEPARATOR + entry.toString().replace("/", FILE_SEPARATOR);
                File temp = new File(targetFile);

                if (!temp.getParentFile().exists()) {
                    temp.getParentFile().mkdirs();
                }

                in = zf.getInputStream(entry);

                out = new FileOutputStream(targetFile);
                byte[] buf = new byte[4096];
                int len;
                while ((len = in.read(buf)) > 0) {
                    out.write(buf, 0, len);
                }
                out.flush();
                out.close();
                in.close();
            }
        }
        finally
        {
            if (out!=null) out.close();
            if (zf!=null) zf.close();
            if (in!=null) in.close();
        }
    }

For this method Sonar give me this Violation:

Bad practice - Method may fail to close stream on exception unZipElementsTo(String, String) may fail to close stream on exception

But, I don't see any violations there. Maybe, it is just a False-positive ?

Upvotes: 0

Views: 9161

Answers (4)

Adam Parkin
Adam Parkin

Reputation: 18720

Alternatively if you're using Java 7 or better, you can use the new try-with-resources mechanism, which handles the close for you. See: http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html for details on this new mechanism.

Note that try-with-resources also works with multiple objects that are opened and closed and still preserves the guarantee that objects will be closed in the reverse order of their construction. Quoth that same page:

Note that the close methods of resources are called in the opposite order of their creation.

Upvotes: 2

mestachs
mestachs

Reputation: 1889

To avoid masking an exception with an exception during the close of the streams It's often recommended to "hide" any io exception in the finally.

To fix use the org.apache.commons.io.IOUtils.closeQuietly(...) or guava Closeables.html#closeQuietly(java.io.Closeable) in the finally close

more on exception handling issues :
http://mestachs.wordpress.com/2012/10/10/through-the-eyes-of-sonar-exception-handling/

Upvotes: 0

Joachim Sauer
Joachim Sauer

Reputation: 308239

If out.close() or zf.close() in the finally block throw an exception then the other closes won't be executed.

Upvotes: 2

Alex Vayda
Alex Vayda

Reputation: 6504

That's right. The OutputStream.close() method can itself throw an exception. If this happens, e.g. on the 1st line of your finally{} block, then the other streams will be left open.

Upvotes: 8

Related Questions