Reporting false positives when detecting leaks of SQLiteDatabase cursors

[andrew659]

Dear Infer developers,

I am trying Infer on some Android projects. I found that Infer will report false positives when detecting the leaks of Android sqlite database cursors. See the code below. The version of Infer I am using now is v0.9.4-8d48c10.

import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;

import java.io.File;
import java.io.FileInputStream;

public class HelloWorld {

    public void foo() {
        Cursor c = null;
        try {
            SQLiteDatabase db = SQLiteDatabase.openDatabase("myDb", null, SQLiteDatabase.CREATE_IF_NECESSARY);
            c = db.rawQuery("SELECT * FROM student where id='1'", null);
        } finally{
            if(c != null && !c.isClosed()){
                c.close();
            }
        }

    }

    public void bar() throws Exception {
        FileInputStream fis = null;
        try {
            fis = new FileInputStream(new File("myfile.txt"));
        } finally {
            if(fis != null) {
                fis.close();
            }
        }
    }
}

In the foo() method, the cursor c is closed in the finally block, but Infer still reports the leak of c. Interestingly, for the input stream used in bar(), which is also closed in the finally block, Infer does not report any leak. I don't understand why there is such difference? The two resources, i.e., cursor and input stream, are used and closed in a similar way. Is it a bug of Infer? Thanks.

[jeremydubreil]

@andrew659: what happens if you remove !c.isClosed()?

  if (c != null) {
    c.close();
  }

[andrew659]

@jeremydubreil

Interesting, the warning is gone after removing the check !c.isClosed(). Why Infer reports a warning if the check is there?

Thank you!

[andrew659]

@jeremydubreil

I also find that if I replace c.close() with a method call closeCursor(c) (see code below), Infer will report there is a leak. Is Infer's analysis inter-procedural? Thanks!

public void foo() {
    Cursor c = null;
    try {
        SQLiteDatabase db = SQLiteDatabase.openDatabase("myDb", null, SQLiteDatabase.CREATE_IF_NECESSARY);
        c = db.rawQuery("SELECT * FROM student where id='1'", null);
    } finally{
        if(c != null){
            closeCursor(c);
        }
    }
}

public void closeCursor(Cursor c) {
    c.close();
}

[jeremydubreil]

The issue is that Infer does not know about isClosed(). Therefore, the analysis will assume that this method can either return false or true, considering the case where c != null and isClosed() = false and not closing the cursor in this case.

In order to see how to fix the issue. What happens if the cursor is already closed and we call close on it?

[andrew659]

@jeremydubreil

Thanks for the explanation.

I tried in an emulator. Nothing will happen if we close the same cursor multiple times. The app won't crash. Maybe the cursor resource is not reference counted.

[andrew659]

@jeremydubreil

Any idea why Infer will not report the leak warning for the code snippet 1, but would report the leak warning for the code snippet 2? The only difference is that c.close() is wrapped in the method call closeCursor(). Is it because when analyzing the foo() method in the code snippet 2, Infer would not further analyze the internals of closeCursor()?

//code snippet 1
public void foo() {
    Cursor c = null;
    try {
        SQLiteDatabase db = SQLiteDatabase.openDatabase("myDb", null, SQLiteDatabase.CREATE_IF_NECESSARY);
        c = db.rawQuery("SELECT * FROM student where id='1'", null);
    } finally{
        if(c != null){
           c.close();
        }
    }
}

//code snippet 2
public void foo() {
    Cursor c = null;
    try {
        SQLiteDatabase db = SQLiteDatabase.openDatabase("myDb", null, SQLiteDatabase.CREATE_IF_NECESSARY);
        c = db.rawQuery("SELECT * FROM student where id='1'", null);
    } finally{
        if(c != null){
            closeCursor(c);
        }
    }
}

public void closeCursor(Cursor c) {
    c.close();
}

[jvillard]

Is it because when analyzing the foo() method in the code snippet 2, Infer would not further analyze the internals of closeCursor()?

Infer is inter-procedural and should be able to handle that too so I'm not sure what happens here. @jeremydubreil any idea?

[SolomonSun2010]

1758 related