When should I close an SQLiteDatabase object?

Question

I kept getting this annoying runtime error for hours, which crashed my app:

java.lang.RuntimeException: An error occured while executing doInBackground().

Caused by: java.lang.IllegalStateException: attempt to re-open an already-closed object: SQLiteDatabase

After some debugging I found that it happens because I close my SQLiteDatabse object, in the onDestory() method. It happens when I call SQLiteOpenHelper.close() as well.

@Override
protected void onDestroy() {
    super.onDestroy();
    _cursor.close(); //is fine
    _db.close(); //causes error
    _databaseHelper.close(); //causes error too (probably calls db.close() internally..?)
    SharedFunctions.d("closed!"); //ignore this ugly thing
}

This brings up the two questions

Am I doing it right? (probably not)
When do I need to close a SQLiteDatabase object, if not in the onDestroy method?

Edit: The DB and the Helper classes are static:

public class MainActivity extends Activity {

    private Cursor _cursor = null;
    private MyCursorAdapter _myCursorAdapter = null;
    private ListView _listView = null;

    private static SalaryDatabaseHelper _databaseHelper = null;
    public static SQLiteDatabase db = null;

    ...

I initialize _databaseHelper in the onCreate() method:

//get database helper
        if(_databaseHelper == null)
            _databaseHelper = SalaryDatabaseHelper.getInstance(this);

db is initialized in an AsyncTask.doInBackground():

protected Boolean doInBackground(Integer... data)
{
    try {
        //get writable database
        if(db == null)
            db = SalaryDatabaseHelper.getDbInstance();

I use singletons for the helper class and the database class: (both accessed via the helper class)

class MyDatabaseHelper extends SQLiteOpenHelper{

    private static SalaryDatabaseHelper _instance = null;
    private static SQLiteDatabase _dbInstance = null;

    //singletons
    public static synchronized SalaryDatabaseHelper getInstance(Context context)
    {
        // Use the application context, which will ensure that you
        // don't accidentally leak an Activity's context.
        if (_instance == null)
            _instance = new SalaryDatabaseHelper(context.getApplicationContext());

        return _instance;
    }

    public static synchronized SQLiteDatabase getDbInstance() {
        if(_dbInstance == null)
            _dbInstance = _instance.getWritableDatabase();

        return _dbInstance;
    }

    ...

What is `_db`? What is `_databaseHelper`? Are these regular or `static` fields? Where is this exception being raised? — CommonsWare, Jun 15 '17 at 22:38

score 7 · Accepted Answer · answered Jun 15 '17 at 23:18

7

Your SQLiteOpenHelper instance is static, and therefore global in scope. With that in mind:

Am I doing it right? (probably not)

No.

When do I need to close a SQLiteDatabase object, if not in the onDestroy method?

Never. SQLite is transactional. There is no risk from failing to close the database.

_{Yes, this irks me too, but I have gone through the seven stages of grief over this, and I am on to "acceptance"}

In simpler scenarios, where there is a single component with access to the database, you might close it when that component is destroyed. In your case, your whole app, including background threads, have access to the database. In that case, you simply never close it.

answered Jun 15 '17 at 23:18

CommonsWare

986,068
189
2,389
2,491

So when does it get closed? only when the user manually terminates the app? Also, is it wrong to have SQLiteDatabase and SQLiteOpenHelper declared as static? – McLovin Jun 15 '17 at 23:21
@Pilpel: "So when does it get closed?" -- it doesn't, in terms of a `close()` method being called. Eventually, your process is terminated, and all of your objects are gone, including your `SQLiteOpenHelper` and `SQLiteDatabase`. "is it wrong to have SQLiteDatabase and SQLiteOpenHelper declared as static?" -- having a `static` `SQLiteOpenHelper`, or having some other singleton hold a `SQLiteOpenHelper`, is common. I would not have `SQLiteDatabase` as a field anywhere; call `getWritableDatabase()` on the helper to get it. – CommonsWare Jun 15 '17 at 23:28
@Pilpel: Also, you are not [implementing double-checked locking](https://stackoverflow.com/a/18093774/115145) correctly for your `static` `SQLiteOpenHelper`. If you make the field `static volatile` instead of just `static`, you should be OK. – CommonsWare Jun 15 '17 at 23:30
@CommonsWare: you wrote: "I would not have SQLiteDatabase as a field anywhere". Does that mean in the above example you will never have "db.query(...);" but always "databaseHelper.getWritableDatabase().query(...);"? And does that not have any relevant performance penalty? – j3App Nov 04 '18 at 20:05
@j3App: The code of the call to `getWritableDatabase()` will be a tiny fraction of the cost of the resulting database I/O. – CommonsWare Nov 04 '18 at 20:06

score 2 · Answer 2 · answered Jun 15 '17 at 23:51

Really, you don't have to close database connection. You can save database as a field in Application object.

Official documentation doesn't say anything about time of database closing. Also this question quotes an old (deleted now) post of Google engineer which says, that this approach is OK.

And in real applications it works well for years.

When should I close an SQLiteDatabase object?

2 Answers2

Linked