2

I have one database helper class and three datasource classes for three tables in same database. The database is accessed in lots of places via AsyncTasks. I faced this "attempt to reopen an already-closed object..." problem, I searched around and found that dbhelper.getReadableDatabase() returns same object for already opened connection. I guessed that problem must be due to when two threads performing operations simultaneously and one of them finishes its task and calls close() the connection gets closed and running thread throws this exception. So to avoid close() I wrote following two methods:

public static synchronized void newOpenRequest() {
    requestsOpen++;
    Util.debuglog(TAG, "Open requests: " + requestsOpen);
}

public static synchronized boolean canClose() {
    requestsOpen--;
    Util.debuglog(TAG, "Open requests: " + requestsOpen);
    if(requestsOpen == 0)
        return true;
    return false;
}

In all of three datasource classes, when I do it in following manner:

private void openRead() {
    database = dbhelper.getReadableDatabase();
    DBHelper.newOpenRequest();
    Log.i(TAG, "Database opened.");
}

private void openWrite() {
    database = dbhelper.getWritableDatabase();
    DBHelper.newOpenRequest();
    Log.i(TAG, "Database opened.");
}

private void close() {
    if (DBHelper.canClose()) {
        dbhelper.close();
        Util.debuglog(TAG, "Database closed.");
    }
}

My LogCat output is as follows:

screen grab of partial logcat output

So as highlighted in black rectangle, total openRequests were 0, so database closed, normal but as highlighted in red rectangle, firstly openRequests were 0, so that time only database was supposed to get closed, but (my guess) what happened is canClose() returned true for a thread, and just before call to dbhelper.close(); another thread called open() (since openRequests = 1 is on LogCat just before close) and then first thread's close() invoked giving trouble to another running thread.

So looking for solution to avoid this concurrent access problem. Thank you.

Chilledrat
  • 2,593
  • 3
  • 28
  • 38
  • Is `requestsOpen` a `volatile` field? – Pedro Oliveira Oct 10 '14 at 14:18
  • @PedroOliveira no, but I guess use of synchronized methods will give effect of volatile here. – Swapnil Bhoite Oct 10 '14 at 16:43
  • I'm not sure. But since you can call close and open at the same time (since they are interdependently synchronized) the value can be changed at the same time by different tasks. Why don't you keep an instance of the database opened in a singleton and access it from the async tasks? – Pedro Oliveira Oct 10 '14 at 16:45
  • @PedroOliveira I would have to change lot of existing code, I tried that also upto some extent, I kept DBHelper singletone, moved methods open and close from datasource classes to it, but on calling dbhelper.close(), it was calling recursively itself, causing overflow, spent so much time examining cause, but could't. So implemented this logic. – Swapnil Bhoite Oct 10 '14 at 16:56
  • @PedroOliveira I'm thinking of combining above 3 things in a single synchronized function 1)newOpenRequest() 2)canClose() 3)I'll close db connection in same function right after checking `requestsOpen == 0. Do you think will it help? – Swapnil Bhoite Oct 10 '14 at 17:09
  • Yes I think so. Using only one synchronized method should work – Pedro Oliveira Oct 10 '14 at 17:09

1 Answers1

1

I have learned to never close the database in android. So maybe your fix is to not close the db. There is no point, keep it open during the entire life of your app. Android will release the resource when your app id destroyed.

You don't need to synchronize your database calls as sqlite can be thread safe.

Is Sqlite Database instance thread safe

DBOpenHelper works just fine:

public class DBOpenHelper extends SQLiteOpenHelper {

    private static final int DATABASE_VERSION = 31;

    private static DBOpenHelper mInstance;

    private static final String DATABASE_NAME = "thedb.db";

    public static DBOpenHelper getInstance(Context context) {

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

    private DBOpenHelper(Context context) {
        super(context, DATABASE_NAME, null, DATABASE_VERSION);
    }

}

sample using the db helper - close the cursor but not the db

SQLiteDatabase db = DBOpenHelper.getInstance(context).getWritableDatabase();

    Cursor cursor = null;
    try {
      cursor = db.query...

    }

    finally {
        cursor.close();

    }
Sakiboy
  • 7,252
  • 7
  • 52
  • 69
bsautner
  • 4,479
  • 1
  • 36
  • 50
  • Yeah, I read it is thread safe for same connection pool, but I'm getting problem due to the use of close only. Is it really good idea to keep connection open? Because my app also using service to handle incoming push messages from server, which again access db, so it will be like my connection will stay open forever. – Swapnil Bhoite Oct 10 '14 at 18:48
  • I have many android apps in production and we don't close the database. The OS releases the lock on the db file if the app is unloaded. Opening and closing is in unnecessary overhead and causes issues like the one you're describing. This post: http://stackoverflow.com/questions/4557154/android-sqlite-db-when-to-close talks about closing it on some onDestroy lifecycle method. The lesson I learned here is that an open connection isn't a bad thing, it's just a lock on a file. Keep it open forever imho. – bsautner Oct 10 '14 at 19:05