False positive RESOURCE_LEAK when closing the cursor in a method that implements an interface

[markusfisch]

Closing a cursor in another method results in a false positive RESOURCE_LEAK message.

Here's a sample:

public class MainActivity extends Activity {
    @Override
    protected void onCreate(Bundle state) {
        super.onCreate(state);
        setContentView(R.layout.activity_main);

        Cursor cursor = querySomething();

        if(!closeIfEmpty(cursor)) {
            cursor.close();
        }
    }

    private Cursor querySomething() {
        return new MockCursor();
    }

    private static boolean closeIfEmpty(Cursor cursor) {
        if(cursor != null && cursor.moveToFirst()) {
            // cursor is NOT empty
            return false;
        }

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

        // cursor is empty and now closed
        return true;
    }
}

Results in:

Starting analysis (Infer version v0.6.0-ad2a906)
Computing dependencies... 100%
Analyzing 4 clusters....%
Analyzed 34 procedures in 4 files

Found 1 issue
app/src/main/java/de/markusfisch/android/inferprobeclosecursor/activity/MainActivity.java:21: error: RESOURCE_LEAK
   resource of type android.test.mock.MockCursor acquired to cursor by call to querySomething() at line 16 is not released after line 21
  19.               cursor.close();
  20.           }
  21. >     }
  22.   
  23.       private Cursor querySomething() {

[sblackshear]

Thanks for reporting this; definitely not the intended behavior for Infer.

The shallow reason that Infer gets this example wrong is that we don't have a model for MockCursor. This issue can be fixed by adding a model for MockCursor (see the SqlLiteCursor model for an example of how to do this).

The deeper reason is that Infer has problems identifying what concrete method an interface method call like Cursor.close will invoke. We are working on fixing this.

[markusfisch]

Thanks for the quick response. Good to know you're on this.

[mcomella]

Just a request to update the title to include MockCursor or the interface -> concrete method calls info – I found this because I was thinking infer did not dive into method calls but it seems this is specific interface -> concrete methods & the MockCursor.

[ozmium]

This false positive report also happens when you have something like this:

Cursor = myDatabase.rawQuery(...);
cursor.moveToFirst();
closeSilently(cursor);

A separate class that has closeSilently():

public static void closeSilently(java.io.Closeable closeable) {
    if (closeable != null) {
        try {
            // This is only allowed on API 16+ (JELLY_BEAN, Android 4.1+)
            closeable.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

However, if closeSilently() is written like this below, then the false positive report does not happen:

public static void closeSilently(android.database.Cursor closeableCursor) {
    if (closeableCursor != null) {
        closeableCursor.close();
    }
}

[jvillard]

I ran the example again and Infer seems smart enough to not report a leak there now.