CODEWITHSUNDEEP
javasonarqubesonarqube-scan
Manoj Kumar
Manoj Kumar

Reputation: 289

Resources should be closed issue is raised by sonar java even if the resource is closed

I have two resources which are properly closed in finally clause with separate try catch blocks. But an issue is raised by sonar scanner to close the second stream even if it is closed.

Below is the sample snippet. 

 protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    try {
            BufferedReader reader = new BufferedReader(new FileReader("foo.in"));
            GZIPOutputStream gzos = null;
            try
                {
                    gzos = new GZIPOutputStream(response.getOutputStream());
                    String line = null;
                    while ((line = reader.readLine()) != null) {
                        gzos.write(line.getBytes("UTF-8"));
                    }
                    gzos.flush();
                } catch (Exception exp) {

                } finally {
                    try {
                       if(reader != null) {
                        reader.close();
                       }
                    } catch (Exception exp) {}
                    try {
                       if(gzos != null) {
                        gzos.close();
                       }
                    } catch (Exception exp){}
                }
        } catch (Exception e){}
}

Is there any proper explanation for this rule?

Upvotes: 1

Views: 1859

Answers (2)

agabrys
agabrys

Reputation: 9126

I scanned your code using SonarJava 5.0.1 plugin and Sonar way profile (includes the Resources should be closed rule). The issue was not found so you probably hit a false positive which already has been fixed.

Issues found in file

Upvotes: 0

giorgiga
giorgiga

Reputation: 1778

Your resource management seems ok to me - maybe Sonarqube complains because it can't follow your logic?

Anyhow - you'll probably be better off if you use a try-with-resources and forget about the close() nonsense :)

protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    try (  BufferedReader reader = new BufferedReader(new FileReader("foo.in"));
           GZIPOutputStream gzos = new GZIPOutputStream(response.getOutputStream())  ) {
        String line; while ((line = reader.readLine()) != null) {
            gzos.write(line.getBytes("UTF-8"));
        }
        gzos.flush();
    }
}

Also, you usually don't want to flat-out ignore exceptions :) I'm talking about the two empty catch around gzos.write - the ones inside the finally are actually ok (or at least, I would have written them too)

Upvotes: 2

Related Questions