SQLite Connection leaked although everything closed

Question

I found many stuff like close the connection and close the cursor, but I do all this stuff. Still the SQLite connection leaks and I get a warning like this:

A SQLiteConnection object for database was leaked!

I have a database manager this, which I call in my activities with the following code:

DatabaseManager dbm = new DatabaseManager(this);

The code of my database manager class follows now:

public class DatabaseManager {

    private static final int DATABASE_VERSION = 9;
    private static final String DATABASE_NAME = "MyApp";
    private Context context = null;
    private DatabaseHelper dbHelper = null;
    private SQLiteDatabase db = null;


    public static class DatabaseHelper extends SQLiteOpenHelper {

         public DatabaseHelper(Context context) {
             super(context, DATABASE_NAME, null, DATABASE_VERSION);
         }

         @Override
         public void onCreate(SQLiteDatabase db) {

                   //create database tables
         }

         @Override
         public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
                      //destroy and recreate them
         }

     }

     public DatabaseManager(Context ctx) {
         this.context = ctx;
     }

    private DatabaseManager open() throws SQLException {
        dbHelper = new DatabaseHelper(context);
        db = dbHelper.getWritableDatabase();

        if (!db.isReadOnly()) {
            db.execSQL("PRAGMA foreign_keys = ON;");
        }

        return this;
    }

    private void close() {
        dbHelper.close();
    }
}

When I call a database method, I do the following thing:

public Object getData() {

    open();

            //... database operations take place ...

    close();

    return data;
}

But as I said, I still get this SQLite connection leaked warning.

What am I doing wrong?

Sealer

I think You are only closing Your DBHelper, but not the Database itself – Sunlit

I think you should call db.close() too – Reconcile

Doesn't matter, if I do this or not. I will get the message anyways. But I read somewhere, that you don't need to do this, when you call the dbHelper.close() – Sealer

And why is the DatabaseHelper static? This might cause the leak because the DatabaseHelper still holds a connection to the db while there is no reference to the DatabasHelper left. – Eliza

I removed the static, but unfortunately it still leaks – Sealer

Is the close(); line always executed after a successful open(); line execution? Do you log the exceptions or just swallow them? – Dough

yes, it's always executed – Sealer

You should close resources in finaly block for example put this db.close() in finally block – Headship

Answer 1

The bolded font in the citation corresponds to this part in your code:

private DatabaseManager open() throws SQLException {
    dbHelper = new DatabaseHelper(context);
    db = dbHelper.getWritableDatabase();

from: http://www.androiddesignpatterns.com/2012/05/correctly-managing-your-sqlite-database.html

Approach #1: Use an Abstract Factory to Instantiate the SQLiteOpenHelper

Declare your database helper as a static instance variable and use the Abstract Factory pattern to guarantee the singleton property. The sample code below should give you a good idea on how to go about designing the DatabaseHelper class correctly.

The static factory getInstance method ensures that only one DatabaseHelper will ever exist at any given time. If the mInstance object has not been initialized, one will be created. If one has already been created then it will simply be returned.

You should not initialize your helper object using with new DatabaseHelper(context).
Instead, always use DatabaseHelper.getInstance(context), as it guarantees that only one database helper will exist across the entire application's lifecycle.

public static class DatabaseHelper extends SQLiteOpenHelper { 

  private static DatabaseHelper mInstance = null;

  private static final String DATABASE_NAME = "database_name";
  private static final String DATABASE_TABLE = "table_name";
  private static final int DATABASE_VERSION = 1;

  public static DatabaseHelper getInstance(Context ctx) {

    // Use the application context, which will ensure that you 
    // don't accidentally leak an Activity's context.
    // See this article for more information: http://bit.ly/6LRzfx
    if (mInstance == null) {
      mInstance = new DatabaseHelper(ctx.getApplicationContext());
    }
    return mInstance;
  }

  /**
   * Constructor should be private to prevent direct instantiation.
   * make call to static factory method "getInstance()" instead.
   */
  private DatabaseHelper(Context ctx) {
    super(ctx, DATABASE_NAME, null, DATABASE_VERSION);
  }
}

Arbe

Great answer. I ran into this problem while working with multiple IntentServices, all working simultaneously out of two different databases. I used this answer except with two separate Factory methods. Cleared up all the memory leak errors. It did add a half second or two to execution time, probably because I no longer have multiple database instances open at once. – Crabtree

I think the last edit to this answer made by @tdmsoares is erroneous, since DatabaseHelper was a static nested class, which can be instantiated. Or did I miss something to notice? – Owen

I get this warning in Android Studio: "Do not place Android context classes in static fields; this is a memory leak (and also breaks Instant Run)". – Heathenize

If Android Studio complains "Do not place Android context classes in static fields ..." Ignore it as long as we are using the applicationContext() we should be okay. See this SO answer – Ragan

Answer 2

The complete example of the above-accepted answer: It may help someone.

Helper Class:

public class DatabaseHelper extends SQLiteOpenHelper {

private static final String DATABASE_NAME = "sample.db";
private static final int DATABASE_VERSION = 1;

private static DatabaseHelper mInstance;

private DatabaseHelper(@Nullable Context context) {
    super(context, DATABASE_NAME, null, DATABASE_VERSION);
}

public static synchronized DatabaseHelper getInstance(Context context) {

    if (mInstance == null) {
        mInstance = new DatabaseHelper(context.getApplicationContext());
    }
    return mInstance;
}

@Override
public void onCreate(SQLiteDatabase db) {

    // create table stuff


}

@Override
public void onUpgrade(SQLiteDatabase db, int i, int i1) {

 // drop table stuff

    onCreate(db);
 }
}

Activity:

SQLiteDatabase database = DatabaseHelper.getInstance(getApplicationContext()).getWritableDatabase();

Cursor cursor = database.query("query");

if (cursor != null) {
   while (cursor.moveToNext()) {
    // stuff
     }
   cursor.close();
   database.close();
 }

Surpassing

Could you please help to solve this issue – Cant

Answer 3

private void method() {
        Cursor cursor = query();
        if (flag == false) {  // WRONG: return before close()
            return;
        }
        cursor.close();
   }

Good practice should be like this:

    private void method() {
        Cursor cursor = null;
        try {
            cursor = query();
        } finally {
            if (cursor != null)
                cursor.close();  // RIGHT: ensure resource is always recovered
        }
    }

Clepsydra