6

I discovered in my project that the close() function in the SQLiteDatabase implementation on Android throws a NullPointerException when running multiple threads that open the database, insert data, and then close the database. Everything runs smoothly unless I allow each thread to close() after inserting into the database.

Here is an example of what one of the writing threads might look like

        ContentValues values = new ContentValues();
        values.put(CallDetailsStorageConstants.COLUMN_NAME_REMOTE_NAME_IDX, (String) "");
        values.put(CallDetailsStorageConstants.COLUMN_NAME_REMOTE_MEETINGID_IDX, (String) "");

        CalculonDatabase callDetailsOpenHelper = CalculonDatabase.getInstance(mContext);
        SQLiteDatabase dbw = callDetailsOpenHelper.getWritableDatabase();

        long rowid = 0;
        try {
            rowid = dbw.insertWithOnConflict(CallDetailsTable.CALL_DETAILS_TABLE_NAME, null, values, SQLiteDatabase.CONFLICT_REPLACE);
        } catch (SQLiteConstraintException e) {
            e.printStackTrace();
        } finally {
            dbw.close();
        }

You can see that at the end a call to close() is made. When running multiple threads I get this exception.

11-03 03:39:26.128: E/AndroidRuntime(977): FATAL EXCEPTION: Thread-17
11-03 03:39:26.128: E/AndroidRuntime(977): java.lang.NullPointerException
11-03 03:39:26.128: E/AndroidRuntime(977):  at     android.database.sqlite.SQLiteStatement.releaseAndUnlock(SQLiteStatement.java:283)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteStatement.executeUpdateDelete(SQLiteStatement.java:96)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteDatabase.executeSql(SQLiteDatabase.java:1933)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteDatabase.execSQL(SQLiteDatabase.java:1864)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteDatabase.beginTransaction(SQLiteDatabase.java:636)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteDatabase.beginTransactionNonExclusive(SQLiteDatabase.java:551)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteStatement.acquireAndLock(SQLiteStatement.java:240)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteStatement.executeInsert(SQLiteStatement.java:111)
11-03 03:39:26.128: E/AndroidRuntime(977):  at android.database.sqlite.SQLiteDatabase.insertWithOnConflict(SQLiteDatabase.java:1737)
11-03 03:39:26.128: E/AndroidRuntime(977):  at com.smash.DBWritingThread.run(DBWritingThread.java:50)

My question then is why does this error happen only when using the close() function? Also, is it ok to use close() at a much later time or possibly never?

Any tips with this issue are very much appreciated.

BenMorel
  • 34,448
  • 50
  • 182
  • 322
user1028181
  • 61
  • 1
  • 2

4 Answers4

14

I have your answers!

Never close the db. It is that simple.

Make sure you only have one, repeat, *one Helper instance per application. Share it with all threads.

http://www.touchlab.co/blog/single-sqlite-connection/

Here's some more info on the internals of sqlite and locking:

http://www.touchlab.co/blog/android-sqlite-locking/

Kevin Galligan
  • 16,159
  • 5
  • 42
  • 62
  • Thanks for the help. We are using one Helper instance. I forgot to clarify that the CalculonDatabase is an extension of the SQLiteOpenHelper and it is implemented as a Singleton (our one shared instance is grabbed with CalculonDatabase.getInstance()). All threads are using that to get an instance of the database, but the close() seems to still give us trouble. – user1028181 Nov 03 '11 at 18:27
  • Actually, looking at the first link you provided again, we have it implemented just like your example except we are closing the database when we write to it. Looks like we just have to stop calling close and trust the SQLite library to do its magic. Thanks a ton. – user1028181 Nov 03 '11 at 18:30
  • Just don't ever close it. You'll be fine. Its really just a file handle. Not a big deal. Your app will close it naturally when it shuts down. As for database issues, its basically impossible to corrupt a Sqlite database unless you have a device with hardware issues. – Kevin Galligan Nov 03 '11 at 18:31
  • If you don't close the db, and use transactions, then sqlite will grow a transaction table for every uncommitted transaction you have. this will cause your database to keep growing and growing. you should close your db instances if you are using transactions. The documentation from google is misleading about this. – Paul Nikonowicz Jan 04 '12 at 21:52
  • 1
    @Paul "then sqlite will grow a transaction table for every uncommitted transaction you have" You could try committing your transactions. Right? Please provide links, and go light on the down vote until you do. – Kevin Galligan Jan 08 '12 at 20:58
  • 1
    @Paul Please read this about how SQLite actually works. http://www.sqlite.org/atomiccommit.html When you commit your transaction, it does a whole bunch of very logical stuff to make sure the db file is content and happy. I don't see anything in there about your mysterious lingering transaction tables. Not to be a jerk, but I got down voted here, and I assume it was you. Please research before going nuts with the down vote. You might still be right, but I have a lot of stuff that says you're not, so post links and/or code examples. – Kevin Galligan Jan 08 '12 at 21:10
  • That's fair. Here is the best i can do:http://stackoverflow.com/questions/6115279/android-database-size-wont-decrease-on-htc-thunderbolt – Paul Nikonowicz Jan 23 '12 at 19:45
  • 1
    Tell me if I'm wrong, but did you just reference your own unreferenced answer? An answer to your own question, BTW? First of all, each call to getReadable/WritableDatabase returns THE SAME INSTANCE (unless the android source has changed since version 2.1 or so). Look at the code. Second, you haven't really "proven" anything. Again, lets create a test app and prove this is happening. – Kevin Galligan Jan 26 '12 at 00:55
  • ya, i did haha. but it's a history of a problem that i encountered and spent a few days debugging. I only believe it will happen if you use transactions though. Also, it doesn't happen on every kind of phone. It was a very frustrating problem. – Paul Nikonowicz Feb 02 '12 at 18:59
  • "Never close the db" and what about this exception I got because of that? 07-01 00:03:15.785: E/SQLiteDatabase(18077): android.database.sqlite.DatabaseObjectNotClosedException: Application did not close the cursor or database object that was opened here 07-01 00:03:15.785: E/SQLiteDatabase(18077): at android.database.sqlite.SQLiteDatabase.(SQLiteDatabase.java:2069) 07-01 00:03:15.785: E/SQLiteDatabase(18077): at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:1123) – User Jun 30 '13 at 22:04
  • You tried to open it more than once. "Make sure you only have one, repeat, *one Helper instance per application". – Kevin Galligan Jul 03 '13 at 00:00
  • To clarify, that exception *only* happens if you have an instance open, and try to open another one. Its possible that I'm wrong, but at this point, considering the number of applications I've worked on and the amount of DB stuff I've done in them, I'd be pretty shocked if you could show me how this happened while following my advice (strictly following my advice). – Kevin Galligan Jul 03 '13 at 00:01
4

You don't need to bother. See this discussion which includes confirmation that closing the database is done by the system as needed if/when the process is killed off.

The relevant quote, from a Google employee, is:

A content provider is created when its hosting process is created, and remains around for as long as the process does, so there is no need to close the database -- it will get closed as part of the kernel cleaning up the process's resources when the process is killed.

Graham Borland
  • 60,055
  • 21
  • 138
  • 179
  • Unless I'm mistaken, this doesn't apply to the asker's situation. The asker seems to be working directly with the SQLiteDB, and not necessarily from within a ContentProvider. If he's getting an instance of the DB in an activity or a service, when that component gets destroyed, it must manually release the DB instance, since the process in which the component was running is likely still running the rest of the app. – Chris Bye Nov 03 '11 at 17:36
  • 2
    It doesn't make a difference. The point is, it's safe for the DB to never be manually closed. The system will close it if needed. SQLite guarantees database integrity at any point, even if the device suddenly powers down without warning, so there's nothing important happening when you close anyway. Really, the best thing is to open the database connection once when your app starts up, and then forget about it. – Graham Borland Nov 03 '11 at 20:31
  • I agree, that is likely the better practice. however, if you don't want to create only a single databaseHelper(and instance), you'll need to be sure you close() before getting a new instance. It could be argued that having a monolithic singleton databaseHelper is worse than demanding that callers close() instances. It isn't necessarily the most well-founded position, but it could be argued. – Chris Bye Nov 03 '11 at 20:43
  • I get this exception when I don't close the db: 07-01 00:03:15.785: E/SQLiteDatabase(18077): android.database.sqlite.DatabaseObjectNotClosedException: Application did not close the cursor or database object that was opened here 07-01 00:03:15.785: E/SQLiteDatabase(18077): at android.database.sqlite.SQLiteDatabase.(SQLiteDatabase.java:2069) 07-01 00:03:15.785: E/SQLiteDatabase(18077): at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:1123) – User Jun 30 '13 at 22:04
1

If you are working directly on a SQLite database without resorting to a content provider, a pattern I have seen used is to close the database in Application.onTerminate, whereas the Application instance stores the singleton database "adapter", which would be an object containing a SQLiteDatabase and its SQLiteOpenHelper.

Giulio Piancastelli
  • 15,368
  • 5
  • 42
  • 62
  • 2
    Note that Application.onTerminate is *never* called in real life. Just saying you can save a few lines of code here. :) It's only there for testing purposes. – mreichelt Jul 24 '13 at 15:01
0

Looks similar to this issue: SQLiteDatabase close() function causing NullPointerException when multiple threads

You may not need to call database.close()

Folee
  • 179
  • 1
  • 1
  • 8