infer icon indicating copy to clipboard operation
infer copied to clipboard

Java resource leak looks like a false positive.

Open KoenDG opened this issue 5 years ago • 3 comments

Please include the following information:

  • [x] The version of infer from infer --version. 0.17.0
  • [x] Your operating system and version, for example "Debian 9", "MacOS High Sierra", whether you are using Docker, etc. Ubuntu 18.04, no docker.
  • [x] Which command you ran, for example infer -- make. infer -- ant compile. and also infer run --reactive -- ant compile

I had to infer -- ant ivy-bootstrap first to get it working though, but that's specific to the project, I think.

  • [x] The full output in a paste, for instance a gist.

https://gist.github.com/KoenDG/b1127a75c870c088fe385226528ac428

  • [x] If possible, a minimal example to reproduce your problem (for instance, some code where infer reports incorrectly, together with the way you run infer to reproduce the incorrect report).

This report is based on analysis of the solr project by the Infer tool, specifically some code here: https://github.com/apache/lucene-solr/blob/9546d8612ca050021db894fc8001134a6fdbf654/solr/core/src/java/org/apache/solr/util/FileUtils.java#L52-L54

Specifically:

 try (FileChannel in = new FileInputStream(src).getChannel();
         FileChannel out = new FileOutputStream(destination).getChannel()) {
      in.transferTo(0, in.size(), out);
    }

Now, in this original code, there is indeed a risk of resource leak.

Here's hoping I haven't misread the specification.

Java specification on try-with-resources: https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-14.20.3

The specification states that, if during the construction of a resource, an exception occurs, the previously constructed resources will be closed.

"The resource" is also defined as "the variable".

If your resource is constructed like this:

FileChannel in = new FileInputStream(src).getChannel();

That is actually creating 2 distinct closeable resources. Once in the constructor, and once in getChannel().

And the specification specifically states that the variable is considered "the resource" and that's what's going to be closed in case of an exception. Meaning that if there's another resources opened in the background, it's not certain that this is going to be closed.

Reporting the code linked above is clearly correct.

However, I changed the code to this:

    try (FileInputStream srcFIS = new FileInputStream(src);
         FileOutputStream destOFS = new FileOutputStream(destination);
         FileChannel in = srcFIS.getChannel();
         FileChannel out = destOFS.getChannel()) {
        in.transferTo(0, in.size(), out);
    }

And Infer is still saying there's a resource leak if the 2nd statement throw an exception, saying the variable created in the first line(srcFIS) may not be closed.

Which isn't what the specification says. The specification says(** symbols mine):

Resources are initialized in left-to-right order.
If a resource fails to initialize (that is, its initializer expression throws an exception),

**then all resources initialized so far by the try-with-resources statement are closed**

And it seems like Infer is saying that it isn't.

Am I missing something here?

EDIT: Fair warning, I got several instances of this, trying to run infer on this project:

While analysing function solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java:Map AutorizationEditOperation$2.edit(Map,CommandOperation) at solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java:57
Uncaught Internal Error: ("InferBase.SqliteUtils.Error(\"load bind proc name: MISUSE (bad parameter or other API misuse)\")")

Different file every time.

And then on the 4th attempt it completes without errors. The file is always different. Just mentioning it in case someone decides to do the analysis and has to do a complete compile. It's near 5.5k files.

KoenDG avatar Sep 08 '19 00:09 KoenDG

Similar issue here.

  • [x] The version of infer from infer --version. Infer version v0.17.0
  • [x] Your operating system and version, for example "Debian 9", "MacOS High Sierra", whether you are using Docker, etc. ArchLinux Docker Container on Ubuntu 16.04
  • [x] Which command you ran, for example infer -- make. infer --sqlite-lock-timeout 1280000 -j 31 -- javac Hello.java
  • [x] The full output.
Capturing in javac mode...
Found 1 source file to analyze in /home/aur/kunal/temp/try/infer-out


Analysis finished in 2.267ss

Found 1 issue

sources/Hello.java:28: error: RESOURCE_LEAK
  resource of type `java.io.LineNumberReader` acquired by call to `new()` at line 19 is not released after line 28.
  26.                   }
  27.               }
  28. >         } catch (IOException e) {
  29.               return false;
  30.           }

  • [x] If possible, a minimal example to reproduce your problem (for instance, some code where infer reports incorrectly, together with the way you run infer to reproduce the incorrect report). I'm using the following code:
import java.io.*;
import java.nio.charset.StandardCharsets;
import java.util.Random;

public class Hello {

    boolean method(String file, String text, int lineNumber) {
        int prevLine = lineNumber - 1;
        try (FileInputStream fis = new FileInputStream(file);
            InputStreamReader isr = new InputStreamReader(fis, StandardCharsets.UTF_8);
            LineNumberReader rdr = new LineNumberReader(isr)) {
            for (String line = null; (line = rdr.readLine()) != null; ) {
                if (rdr.getLineNumber() == prevLine) {
                    return line.contains(text);
                }
                if (rdr.getLineNumber() == lineNumber) {
                    return line.contains(text);
                }
            }
        } catch (IOException e) {
            return false;
        }
        return false;
    }

  public static void main(String[] args) {
    System.out.println("Hello Infer world");
  }

}

iamKunal avatar Sep 19 '19 13:09 iamKunal

Still present in 1.0:

package com.engflow.infer.test;

import java.io.FileInputStream;
import java.io.IOException;

public class Hello {
  byte[] read(String filename) throws IOException {
    byte[] data = new byte[1234];
    try (FileInputStream in = new FileInputStream(filename)) {
      in.read(data);
    }
    return data;
  }
}

It would be good to get this fixed as try-with-resources is a pretty common pattern.

ulfjack avatar Nov 02 '20 08:11 ulfjack

Duplicate of #619 ?

qrilka avatar Apr 09 '21 14:04 qrilka