Android sqlite log reports and db.close()

Question

The log reports that the database or cursor was not closed. I basically have an activity with a custom surfaceview and uses a handler to report back to the activity. When I receive the msg I show an alertdialog and also update the database.

private Handler handler = new Handler() {
    public void handleMessage(Message msg) {
        switch(msg.what) {
        case 1:
            dh.open();
            dh.updateEvent("id", "name", "someone");
            dh.close();
            successAlert.show();
            break;
        case 2:
            failAlert.show();
            break;
        }
    }
};

Previously I did not have "dh.close()" and thats when the log reported database/cursor not closed. But ever since I added that in, it takes a really long time to complete. Once I get the message, the system seems to hang. Am I doing something wrong or does it usually take this long. I have also tried using a try block with a finally to close the db.

EDIT:

public class DatabaseHelper {

private Database dbHelper;
private SQLiteDatabase db;
private Context context;

public DatabaseHelper(Context context) {
    this.context = context;
    //database = new Database(context);
}

public void open() {
    dbHelper = new Database(context);
    db = dbHelper.getWritableDatabase();
}

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

public void updateEvent(int id, String name, int other) {
    ContentValues cv = new ContentValues();
    cv.put("id", id);
    cv.put("name", name);
    cv.put("other", other);
    db.update("stateTable", cv, "id=" + id, null);
}

public boolean checkState(int id) {
    db = dbHelper.getReadableDatabase();
    Cursor cursor = db.query("stateTable", null, null, null, null, null, null);
    cursor.moveToPosition(id - 1);
    int i = cursor.getInt(2);
    android.util.Log.d("semajhan", ": " + i);
    if (i == 1) {
        return true;
    } else {
        return false;
    }
}

}

Extended SQLiteOpenHelper:

public class Database extends SQLiteOpenHelper {

private static final String DATABASE_NAME = "events.db";
private static final int DATABASE_VERSION = 1;
private static final String TABLE_NAME = "stateTable";
private static final String ID = "id";
private static final String NAME = "name";
private static final String OTHER = "other";
private static final String DATABASE_CREATE = "CREATE TABLE stateTable (id INT, name TEXT, other INT)";
private static final String DATABASE_UPGRADE = "DROP TABLE IF EXISTS table";

public Database(Context context) {
    super(context, DATABASE_NAME, null, DATABASE_VERSION);
    // TODO Auto-generated constructor stub
}

@Override
public void onCreate(SQLiteDatabase db) {
    // TODO Auto-generated method stub
    db.execSQL(DATABASE_CREATE);
    // added initial values
}

@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
    // TODO Auto-generated method stub
    db.execSQL(DATABASE_UPGRADE);
    onCreate(db);
}

}

01-11 13:57:41.239: ERROR/ActivityManager(61): ANR in com.semajhan.soodles  (com.semajhan.soodles/.Level1)
01-11 13:57:41.239: ERROR/ActivityManager(61): Reason: keyDispatchingTimedOut
01-11 13:57:41.239: ERROR/ActivityManager(61): Load: 1.64 / 0.56 / 0.26
01-11 13:57:41.239: ERROR/ActivityManager(61): CPU usage from 35716ms to -1ms ago:
01-11 13:57:41.239: ERROR/ActivityManager(61):   44% 862/com.semajhan.soodles: 37% user + 7.2% kernel / faults: 853 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):   29% 61/system_server: 27% user + 1.9% kernel / faults: 142 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):   0.2% 731/com.android.quicksearchbox: 0% user + 0.2% kernel / faults: 30 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):   0.2% 707/com.android.launcher: 0.2% user + 0% kernel / faults: 30 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):   0.2% 801/com.svox.pico: 0.1% user + 0.1% kernel / faults: 363 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):   0% 117/com.android.systemui: 0% user + 0% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):   0% 41/adbd: 0% user + 0% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61): 99% TOTAL: 86% user + 13% kernel + 0% irq
01-11 13:57:41.239: ERROR/ActivityManager(61): CPU usage from 1969ms to 2620ms later:
01-11 13:57:41.239: ERROR/ActivityManager(61):   54% 61/system_server: 48% user + 6% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     40% 69/SurfaceFlinger: 40% user + 0% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     10% 92/InputDispatcher: 7.5% user + 3% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     1.5% 62/HeapWorker: 1.5% user + 0% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):   44% 862/com.semajhan.soodles: 32% user + 12% kernel / faults: 2 minor
01-11 13:57:41.239: ERROR/ActivityManager(61):     24% 874/Thread-13: 24% user + 0% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     23% 862/studios.soodles: 4.6% user + 18% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     1.5% 867/Compiler: 0% user + 1.5% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):   0.8% 731/com.android.quicksearchbox: 0% user + 0.8% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61):     0.8% 732/HeapWorker: 0% user + 0.8% kernel
01-11 13:57:41.239: ERROR/ActivityManager(61): 100% TOTAL: 76% user + 23% kernel

The handler code looks OK, however we don't see how your `dh` is coded, so it's hard to say what could cause the delay. — Vit Khudenko, Jan 11 '11 at 18:11
Okay, where is Database coded in? Unless it's part of the adk and I don't see it anywhere... — sargas, Jan 11 '11 at 18:19

score 5 · Accepted Answer · answered Jan 11 '11 at 20:25

5

The first call to getReadableDatabase or getWritableDatabase of the SQLiteOpenHelper instance takes a really long time to complete. You shouldn't create a new Database object (your SQLiteOpenHelper instance) each time that you need to query the database. Try using the same Database instance within DatabaseHelper.

When using SQLiteOpenHelper, you don't want to close the SQLiteDatabase object for an SQLiteOpenHelper because it is shared; i.e. getWritableDatabase always returns the same SQLiteDatabase object.

Notice that your checkState method leaks a cursor. To help prevent cursor leaks, I always use a try-finally after obtaining a cursor. For example:

    db = dbHelper.getReadableDatabase();
    Cursor cursor = db.query("stateTable", null, null, null, null, null, null);
    try {
        cursor.moveToPosition(id - 1);
        int i = cursor.getInt(2);
        android.util.Log.d("semajhan", ": " + i);
        if (i == 1) {
                return true;
        } else {
                return false;
        }
    } finally {
        cursor.close();
    }

answered Jan 11 '11 at 20:25

Daniel Trebbien

38,421
18
121
193

I think I understand what you have tried to explain to me. In my DatabaseHelper class, I create a new instance of SQLiteOpenHelper each time which you are implying not to do. I cannot seem to figure out HOW exactly I would create an SQLiteOpenHelper instance ONCE within my DatabaseHelper class and use it from there. – semajhan Jan 11 '11 at 20:53
@semajhan: Add a private member variable to your activity for an object of type `DatabaseHelper`. Inside the `onCreate` method override of the activity you call DatabaseHelper#open. Inside the `onDestroy` method override of the activity you call DatabaseHelper#close. – Daniel Trebbien Jan 11 '11 at 21:26
@semajhan: Actually, I see that it's your `Handler` that needs access to the `DatabaseHelper` instance. Are you programming a service? If so, add the private member variable for an object of type `DatabaseHelper` to the service. Inside the `onCreate` method override of the service you call DatabaseHelper#open. Inside the `onDestroy` method override of the service you call DatabaseHelper#close. – Daniel Trebbien Jan 11 '11 at 21:32
@semajhan: I should also say that in this case, creating only one `SQLiteOpenHelper` instance is accomplished by creating only one `DatabaseHelper` instance. My two comments above are how to use only one `DatabaseHelper` instance. – Daniel Trebbien Jan 11 '11 at 21:37
I've narrowed down my problem to: how to access a single db from multiple activities. Since I have over 50 activities needing to update the db, i've come up with two solutions and they are to pass a handler to another activity with intents and the other is services. – semajhan Jan 11 '11 at 21:40
1

@semajhan: If you have 50 activities that need to access a single database, then I suggest writing a content provider. – Daniel Trebbien Jan 11 '11 at 23:48

score 3 · Answer 2 · edited May 23 '17 at 12:04

I wonder if the underlying problem regarding the cursor not being closed is the same as in this answer: Activities need to have an onDestroy method that closes the DatabaseHelper. (All I can tell right now is that you don't show an onDestroy method, but this is clearly just a portion of your code.) I also blogged about this problem here.

You also wrote that "But ever since I added [dh.close()], it takes a really long time to complete." Are you doing a lot of writes to the database? Maybe you need to use transactions to flush those writes periodically. If you're only reading from the database, then I have no idea why the close call would take a long time to complete. But without seeing more of your code, these are only guesses.

~~EDIT: ccheneson's advice about calling db.close() before calling dbHelper.getReadableDatabase() is worth following. However, you need to check that db isn't null. This changes the checkState to:~~

public boolean checkState(int id) {
    if (db != null) {
        db.close();
    }
    db = dbHelper.getReadableDatabase();
    // ... etc ....

If that turns out to be the real problem, please accept ccheneson's answer, not mine.

(Above edit was incorrect; I should have known better! See below instead.)

EDIT 1: I'm not sure why you are re-opening the database at all in this method. Assuming that your DatabaseHelper.open method is invoked, you have a readable and writeable database handle. checkState doesn't need to re-open it to perform reads.

EDIT 2: However, SQLiteOpenHelper.getReadableDatabase will almost always return the same database handle that was returned from SQLiteOpenHelper.getWriteableDatabase. You do not need to close that database handle explicitly; the SQLiteOpenHelper will close it for you. You do need to be sure that SQLiteOpenHelper.close is invoked when your activity is destroyed (as I wrote above.)

I'm checking your blog and link right now. I'm only writing one row of 3 columns: an INT, TEXT, and another INT. Being pretty new to SQL but IMHO writing just 1 row of 3 cols should be very fast. — semajhan, Jan 11 '11 at 20:05
So basically when I create an instance of SQLiteOpenHelper I have already "opened" the database for read and write? And for reading I do not need to explicitly call db.getReadableDatabase()? — semajhan, Jan 11 '11 at 20:30
@semajhan: Not quite. The database isn't opened until you've called `db.getWriteableDatabase` or `db.getReadableDatabase`. But calling `db.getWriteableDatabase` allows you to read from **and** write to the database. Hence I don't think you need to call `db.getReadableDatabase` **after** you've already called `db.getWriteableDatabase`. — Dan Breslau, Jan 11 '11 at 20:52
Ah, that makes sense. So I would never need to call getReadableDatabase() if I have already called getWritableDatabase(). But say I AM calling getReadableDatabase() after getWritableDatabase(), does that mean I am opening the db twice and causing a leak maybe? — semajhan, Jan 11 '11 at 20:55
@semajhan: No, it does not cause a leak. `SQLiteOpenHelper` keeps a reference to the database handle that it's given you. If you re-open the database for reading or writing, it will simply return that handle (assuming no errors: you have appropriate permissions, the disk is not full, etc.) In fact, if you look at the source code, you can see that calling `getReadableDatabase` will often give you a database that's already writeable! (See http://android.git.kernel.org/?p=platform/frameworks/base.git;a=blob_plain;f=core/java/android/database/sqlite/SQLiteOpenHelper.java;hb=HEAD ) — Dan Breslau, Jan 11 '11 at 21:00

score 1 · Answer 3 · answered Jan 11 '11 at 18:32

1

public void close() {
    dbHelper.close(); // to change to db.close(); ?
}

I think here you would want to close the database handle db instead of the dbHelper

Also in you checkState method:

public boolean checkState(int id) {
    db = dbHelper.getReadableDatabase();
    Cursor cursor = db.query("stateTable", null, null, null, null, null, null);
    cursor.moveToPosition(id - 1);
    int i = cursor.getInt(2);
    android.util.Log.d("semajhan", ": " + i);
    if (i == 1) {
        return true;
    } else {
        return false;
    }
}

You would want to close the cursor like:

int i = cursor.getInt(2);
cursor.close();
android.util.Log.d("semajhan", ": " + i);

answered Jan 11 '11 at 18:32

ccheneson

49,072
8
63
68

The thing is, I have already done all that. In my older code, I had the handle to db close instead of dbHelper. And in my checkState method I had also called cursor.close(). But that resulted in a leak. Once I switched from closing db to dbHelper, the leak went away but I got some freaky error in my log. Just a bunch of numbers and giving me a percentage: user=72% kernel=28% something like that. – semajhan Jan 11 '11 at 18:36
1

Also, when you call `db = dbHelper.getReadableDatabase();`, try closing the db first: `db.close(); db = dbHelper.getReadableDatabase();`. After that, no other idea :) – ccheneson Jan 11 '11 at 18:49
Closing the db first results in a force close =[ – semajhan Jan 11 '11 at 18:58
Look at the LogCat under DDMS to see if there is any stack trace that can help us debug – ccheneson Jan 11 '11 at 19:02
Found DDMS but in the LogCat I don't see anything related to "stack trace". – semajhan Jan 11 '11 at 19:18
You can go straight to LogCat by going to Window -> Show View -> Other -> Under Android, select LogCat and you will see the LogCat window appear. Run your project and you will see stack trace in case of errors – ccheneson Jan 11 '11 at 19:24
I don't see any "caused by" clauses only a Reason: KeyDispatchingTimedOut. – semajhan Jan 11 '11 at 19:31
ccheneson's comment about `db.close()` is correct as far as it goes, but you probably want to change it to `if (db != null) {db.close();}` By the way, I think it's probably not a good idea to use the same `db` member variable for both the readable and writeable database handles. Why not get a single, writeable database handle at the start of your activity, and leave it at that? – Dan Breslau Jan 11 '11 at 20:06
I read somewhere here that it was good practice to open the DB ALAP and close it ASAP. I'm guessing this is not the case? – semajhan Jan 11 '11 at 20:13
@ccheneson: `SQLiteOpenHelper` maintains a reference to the database handle that it's opened. Any database handle that's opened through `SQLiteOpenHelper` should be closed by calling `SQLiteOpenHelper.close()`. – Dan Breslau Jan 11 '11 at 20:19
For the record, my comment 3 comments above (mentioning `if (db != null) {db.close();}`) was incorrect. My answer, elsewhere on this page, is more correct. – Dan Breslau Jan 11 '11 at 20:56

Android sqlite log reports and db.close()

3 Answers3

(Above edit was incorrect; I should have known better! See below instead.)

Linked