close() was never explicitly called on database

Question

I need to store sms in a sqlite database, when one is received. I'm using these classes:

DBHelper.java

public class DBHelper extends SQLiteOpenHelper {
    private static final String DB_NAME = "my_db";
    private static final int DB_VERSION = 1;

    public DBHelper(Context context, String name, CursorFactory factory, int version) {
        super(context, DB_NAME, factory, DB_VERSION);
    }

    public DBHelper(Context context) {
        super(context, DB_NAME, null, DB_VERSION);
    }

    public String getName(){
        return DB_NAME;
    }
    @Override
    public void onCreate(SQLiteDatabase db) {
        String sql="";
        sql+="CREATE TABLE sms ( ";
            sql+="_id INTEGER PRIMARY KEY AUTOINCREMENT, ";
        sql+="mit TEXT NOT NULL, ";
        sql+="dest TEXT NOT NULL, ";
        sql+="text TEXT NOT NULL, ";
        sql+="date DATETIME NOT NULL )";

        db.execSQL(sql);
    }

DBManager.java

public class DBManager {

private DBHelper dbh;
SQLiteDatabase dbr;
SQLiteDatabase dbw;

DBManager(DBHelper dbh){
    this.dbh=dbh;
    dbr=dbh.getReadableDatabase(); //line 24
    dbw=dbh.getWritableDatabase();
}

DBManager(Context ctx){
    this.dbh=new DBHelper(ctx);
    dbr=dbh.getReadableDatabase();
    dbw=dbh.getWritableDatabase();
}

public long insert(String table, ContentValues values){
    return dbw.insert(table, null, values);
}

public long insertSMS(Sms sms){
    if(sms.getSender().length()>0 && sms.getReceiver().length()>0 && sms.getText()!=null){
    String sql="INSERT INTO sms VALUES ( null, \""+sms.getSender()+"\", \""+sms.getReceiver()+"\", \""+sms.getText()+"\", datetime())";
    dbw.execSQL(sql);
    return 0;
    }
    return -1;
}

public Sms getSmsById(int id){
    String[] columns = { "mit","dest","text","date" }, selectionArgs={ Integer.toString(id) };
    String selection="_id = ?";
    Cursor c=dbr.query("sms", columns, selection, selectionArgs, null, null, "_id ASC");
    if(c.moveToFirst())
    return new Sms(c.getString(0),c.getString(1), c.getString(2),c.getString(3));
    return null;
}

public Sms[] getAllSms(){
        String[] columns = { "mittente","destinatario","testo","dataora" };
        Cursor c=dbr.query("sms", columns, null, null, null, null, "_id ASC");
        ArrayList<Sms> al=new ArrayList<Sms>();
        if(c.moveToFirst())
        do{
            al.add(new Sms(c.getString(0),c.getString(1),c.getString(2),c.getString(3)));
        }while(c.moveToNext());
        return (Sms[]) al.toArray();
    }

public void close(){
        if(dbh!=null)
        dbh.close();

        if(dbw!=null)
        dbw.close();

        if(dbr!=null)
        dbr.close();
    }}

SmsBR.java

public class SmsBR extends BroadcastReceiver {
        @Override
        public void onReceive(Context context, Intent intent) {               
                    Bundle bundle = intent.getExtras();
                    if (bundle != null) {
                        Object[] pdus = (Object[])bundle.get("pdus");
                        final SmsMessage[] messages = new SmsMessage[pdus.length];
                        for (int i = 0; i < pdus.length; i++) {
                            messages[i] = SmsMessage.createFromPdu((byte[])pdus[i]);
                        }
                        if (messages.length > 0) {
                            System.out.println("I received an sms from: "+messages[0].getOriginatingAddress());
                            DBManager dbm = new DBManager(context);
                            dbm.insertSMS(messages[0]);
                            dbm.close();
                        }}}}

But I get this error:

close() was never explicitly called on database '/data/data/it.giox.sms/databases/my_db' 
android.database.sqlite.DatabaseObjectNotClosedException: Application did not close the cursor or database object that was opened here
     at android.database.sqlite.SQLiteDatabase.<init>(SQLiteDatabase.java:1847)
     at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:820)
     at android.database.sqlite.SQLiteDatabase.openOrCreateDatabase(SQLiteDatabase.java:854)
     at android.database.sqlite.SQLiteDatabase.openOrCreateDatabase(SQLiteDatabase.java:847)
     at android.app.ContextImpl.openOrCreateDatabase(ContextImpl.java:549)
     at android.content.ContextWrapper.openOrCreateDatabase(ContextWrapper.java:203)
     at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:118)
     at android.database.sqlite.SQLiteOpenHelper.getReadableDatabase(SQLiteOpenHelper.java:187)
     at it.giox.sms.DBManager.<init>(DBManager.java:24)
     at it.giox.sms.DroidClient.<init>(DroidClient.java:29)
     at it.giox.sms.DroidWebServer.getClient(DroidWebServer.java:25)
     at it.giox.sms.WebServer.startServer(WebServer.java:101)
     at it.giox.sms.WebServer.run(WebServer.java:60)
     at java.lang.Thread.run(Thread.java:1019)

What can I change?

[EDIT] I've done this correction, but the error remains...

public Sms[] getAllSms(){
        String[] columns = { "mittente","destinatario","testo","dataora" };
        ArrayList<Sms> al=new ArrayList<Sms>(); Cursor c = null;
        try{
        c=dbr.query("sms", columns, null, null, null, null, "_id ASC");
                if(c.moveToFirst())
        do{
            al.add(new Sms(c.getString(0),c.getString(1),c.getString(2),c.getString(3)));
        }while(c.moveToNext());}
        finally{
        c.close();
        close();
        }

        return (Sms[]) al.toArray();
        }

[EDIT 2]

The problem is that in DBManager constructor I create a new SQLiteDatabase and then I should close() it. If I put close() in the constructor I've no that error, but the system tell me that the database is closed. I solved the problem creating a new instance of SQLiteDatabase every time I need it, and finally closing it. It works, but I think there are better solutions!

public Sms[] getAllSms(){
        SQLiteDatabase dbw;
                ...
                try{
                dbw=dbh.getWritableDatabase();
                ...
                } finally {dbw.close();}}

Please list DBManager.java line 24 and DroidClient.java line 29 — ghostbust555, Sep 15 '11 at 00:33
This is DBManager.java line 24: `dbr=dbh.getReadableDatabase();` and DroidClient.java line 29 is simply a new: `dbm=new DBManager(context)` — supergiox, Sep 15 '11 at 00:36
This post is the same as http://stackoverflow.com/questions/4464892/android-error-close-was-never-explicitly-called-on-database — ghostbust555, Sep 15 '11 at 00:39
@ghostbust555 Sorry, but I didn't understand what can be useful to me on that post — supergiox, Sep 15 '11 at 00:58

Nikolay Elenkov · Accepted Answer · 2011-09-15T09:08:30.417

2

You need to close your cursors once you are done with them. Use try/finally in getAllSms() and getSmsById() to close the cursor after you read the data.

edited Sep 15 '11 at 09:08

answered Sep 15 '11 at 01:51

Nikolay Elenkov

52,576
10
84
84

As you can see in the edited question I've done this correction (at this moment getSmsById() is commented) but nothing seems to have changed.. – supergiox Sep 15 '11 at 09:19
1

You have to close _all_ cursors you use. The stack trace should point to place where you opened them. Try to simplify your program further: you don't need to have separate read/write handles, use a single write handle (`getReadableDatabase()` does exactly the same thing as `getWritableDatabase()` except it _might_ work even if the disk is full) – Nikolay Elenkov Sep 15 '11 at 10:57
I followed your suggestion and now I close every cursor, but I've the same error on `dbw=dbh.getWritableDatabase();` – supergiox Sep 15 '11 at 22:32
You might consider calling `DBHelper.close()` in the `onDestroy()` of your activity. – Nikolay Elenkov Sep 16 '11 at 02:58
I solved the problem (see the edited question), but I don't like the solution – supergiox Sep 16 '11 at 11:55
3

This is wrong and will kill performance. Create the database in the activity's `onCreate()` and close it in `onDestroy()`. Creating, opening and closing every time you need it is bound to be slow, reuse your DBHelper instance as much as possible. – Nikolay Elenkov Sep 16 '11 at 12:12
But, in this way, I've to pass the `SQLiteDatabase` from the activity (in my case from a Service) to the classes who want to access the db? Or is there a better way? I created `DBManager` to simplify DB read/write operations – supergiox Sep 16 '11 at 12:33
3

Make `DBManager` a singleton. – Nikolay Elenkov Sep 16 '11 at 12:41

close() was never explicitly called on database

1 Answers1

Linked