-1

So Im trying to save data to database using separate thread but it causes an exception:

java.lang.IllegalStateException: database not open

I assume this is not a task for a Loader since I don't need any resulting cursor, it has just to save my data. The save to db code looks as follows:

Thread thread = new Thread(new Runnable() {
    @Override
    public void run() {
        dbHelper.saveMessages(messages);
    }
});
thread.setPriority(Thread.MIN_PRIORITY);
thread.start();

If I remove this DB operation out from a Thread everything works good but it causes UI lags. The same things happens if I'm trying to read from DB from a separate thread.
this is a part of a DB writing code:

    SQLiteDatabase db = dbAdapter.getWritableDatabase());
    try {
        if (existingMessage != null)
            insertId = db.update(Tables.Messages.TABLE_NAME, values, Tables.Messages.SERVER_ID + "=?", new String[]{String.valueOf(message.serverId)});
        else
            insertId = db.insertOrThrow(Tables.Messages.TABLE_NAME, null, values);
    } catch (Exception e) {
        e.printStackTrace();
    }

Actually exception happens at db.update() or at db.insertOrThrow() line. Its not guaranteed to throw exception but it does from time to time. It could write for instance 5 objects and then fail on 6th.
What am I doing wrong or whats wrong with SQLiteOpenHelper? The full exception log:

10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ java.lang.IllegalStateException: database not open
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at android.database.sqlite.SQLiteDatabase.queryWithFactory(SQLiteDatabase.java:1224)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1184)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1264)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at com.myapp.db.DatabaseHelper.saveMessages(DatabaseHelper.java:478)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at com.myapp.db.DatabaseHelper.requestMessages(DatabaseHelper.java:1245)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at com.myapp.activity.LoginActivity$9.run(LoginActivity.java:425)
10-30 22:34:52.746  10431-10536/com.myapp W/System.err﹕ at java.lang.Thread.run(Thread.java:1096)

UPDATE:
I'd discovered that its all about reading from db actually. In my code first I read from db to ensure there is no such record. After that I decide what to do - insert or update. And the issue happens with readable db. Right after db is opened for read and the next line should read from db it appears "not opened". And there is no thread for reading, everything happens in one thread: 1st read then write. However reading randomly fails from time to time. So its not writing as I mentioned before.

Stan
  • 6,511
  • 8
  • 55
  • 87
  • What's the exact stacktrace and where in your code it occurs? Post the `saveMessages()` code, too. – laalto Oct 30 '14 at 20:04
  • You're probably not multi-threading very carefully. Look at [this question](http://stackoverflow.com/questions/2493331/what-are-the-best-practices-for-sqlite-on-android/3689883). Some of its answers might help. – Reed Oct 30 '14 at 20:32
  • Actually I wonder why didn't you use AtomicInteger instead of locking on a fake object just for counter++ or --. Anyway I'm using Dmytro Danylyk's solution from the pointed thread. However my issue is not about mutli-threading since there is no other thread which uses db to read or write at same time. – Stan Oct 30 '14 at 20:50
  • This isn't the code that causes the exception. There's no `query()`in code but there is that in stacktrace. – laalto Oct 30 '14 at 21:45
  • @Stan, good point; I changed it. You shouldn't be using `getWritableDatabase()` if you are using that solution. You should use `openDatabase()` and `closeDatabase()`. Additionally, you could add a log message to the open and close methods to make sure you're not closing more than you're opening. – Reed Oct 31 '14 at 00:20
  • Also, I think [my solution](http://stackoverflow.com/a/26642293/802469) is better. It prevents the accidental direct access to `SQLiteOpenHelper`, uses a normal constructor, and doesn't require a `getInstance()` or `initialize()` call, making it safer and easier. It's partially just personal preference on code styles, though. – Reed Oct 31 '14 at 00:30
  • Do not make your own security Fail. Look at all the back doors on this code it's not immune to brute force – danny117 Oct 31 '14 at 16:23

3 Answers3

0

I see multiple issues with your implementation. First dbHelper can be used by another thread and could create a deadlock so you should guard it with lock. proceedToNextActivity() looks like does some work on UI, doing anything from background thread is simply wrong and will cause a crash.

If dbHelper don't need any UI element just pass a copy of messages to this thread and create dbHelper -> do some work -> close helper -> end thread. Hopefully this will solve your issue.

Hussain Mansoor
  • 2,934
  • 2
  • 27
  • 40
  • `proceedToNextActivity()` does nothing with UI and anyway this is not a part of a question. As for `dbHelper` it could be used by other thread theoretically but it does not really – Stan Oct 30 '14 at 20:25
0

Make an insert or update method. If the insert fails because of a duplicate key then update.

danny117
  • 5,581
  • 1
  • 26
  • 35
  • -1, This answer does not answer the question and does not address the `IllegalStateException` being thrown. This should be left as a comment, if anything. – Reed Oct 30 '14 at 23:49
  • I can't imagine a database without an insert_or_update method. You said it happens on the update most likely you haven't handled a duplicate key. Update fails then do an insert. – danny117 Oct 31 '14 at 16:21
0

Since I'm using the Dmytro Danylyk's way to work with DB (also mentioned on SO here) I made a mistake thinking that for reading purposes I should use readable DB returned by standard getReadableDatabase() method. Instead I should always use the writable one returned by openDatabase() method (which is thread safe).

Community
  • 1
  • 1
Stan
  • 6,511
  • 8
  • 55
  • 87