Random sqlite3 library routine called out of sequence error in MultiThreadReadWriteTest #176
Comments
|
As promised in #175, I put the full log of the NOTE (update): I just noticed that the In addition, I noticed that sometimes we get This error was not always in the same place. For example:
I will look at these cases early next week and propose some changes with additional safeguards. |
|
Hi @brodybits It is certainly possible there could be an issue within that specific test, there is certainly a lot going on there. That said, I haven't been able to reproduce it. What device or emulator were you running that produced this error? |
At least it seems to be a valuable test!
I am not surprised:
It was on my Motorola Moto G which is my own dual-sim mobile, and I only I do kind-of remember this test failing a few times when I was working on I am planning to take another look at this test tomorrow night after my |
|
Just rebuilt the libraries with the latest changes included and ran the test suite OK (with no failures). I noticed (again) the I do find it especially strange that some of these test functions could show an exception thrown during an openDatabase/openOrCreateDatabase function call but still all the tests are passing OK. |
|
When I changed When I ran it, the it eventually died (second When I uninstalled, did a clean build/install of the test suite, and ran it, it passed the first 3 then died on the second UPDATED: This was on my Motorola Moto G. I will try this on a simulator when I get a chance. Also, I did take a look through |
|
When I tried the same changes to I got the following error in logcat: But strangely enough:
When I ran the test suite the second time on the emulator, it failed the first test and then died pretty fast on the first instance of the I will take a closer look at the errors observed so far within the next few days, and also want to see if the same test would crash when testing with the standard (built-in) Android database library as well. |
|
Here is how I fixed the crashing, after trial-and-error, on both the emulator and my device: The following changes to --- a/src/main/java/net/zetetic/tests/MultiThreadReadWriteTest.java
+++ b/src/main/java/net/zetetic/tests/MultiThreadReadWriteTest.java
@@ -3,6 +3,7 @@ package net.zetetic.tests;
import android.util.Log;
import net.sqlcipher.Cursor;
import net.sqlcipher.database.SQLiteDatabase;
+import net.sqlcipher.database.SQLiteException;
import net.zetetic.ZeteticApplication;
import java.io.File;
@@ -25,6 +26,7 @@ public class MultiThreadReadWriteTest extends SQLCipherTest {
return executor.run();
}
+ // NOTE: this function _could_ but is NOT EXPECTED to throw:
synchronized SQLiteDatabase getDatabase(DatabaseAccessType accessType) {
if (accessType == DatabaseAccessType.InstancePerRequest) {
return SQLiteDatabase.openOrCreateDatabase(databaseFile, password, null);
@@ -59,14 +61,29 @@ public class MultiThreadReadWriteTest extends SQLCipherTest {
public void run() {
android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND);
Log.i(TAG, String.format("writer thread %d beginning", id));
+
+ // NOTE: _could_ but is NOT EXPECTED to throw:
SQLiteDatabase writer = getDatabase(accessType);
- writer.execSQL("create table if not exists t1(a,b)");
- for (int index = 0; index < size; index++) {
+
+ try {
+ // XXX TBD fix indent:
+ writer.execSQL("create table if not exists t1(a,b)");
+ for (int index = 0; index < size; index++) {
Log.i(TAG, String.format("writer thread %d - insert data for row:%d", id, index));
writer.execSQL("insert into t1(a,b) values(?, ?)",
new Object[]{"one for the money", "two for the show"});
+ }
+
+ } catch (Exception ex){
+ Log.e(TAG, "Writer test failed with exception", ex);
+ }
+
+ try {
+ closeDatabase(writer, accessType);
+ } catch (Exception ex){
+ Log.e(TAG, "Writer test failed with exception", ex);
}
- closeDatabase(writer, accessType);
+
Log.i(TAG, String.format("writer thread %d terminating", id));
}
}
@@ -86,6 +108,22 @@ public class MultiThreadReadWriteTest extends SQLCipherTest {
android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND);
Log.i(TAG, String.format("reader thread %d beginning", id));
SQLiteDatabase reader = getDatabase(accessType);
+
int currentCount = 0;
+ // NOTE: _could_ but is NOT EXPECTED to throw:
int updatedCount = getCurrentTableCount(reader);
- while (currentCount == 0 || currentCount != updatedCount) {
+
+ try {
+ // XXX TBD fix indent:
+ while (currentCount == 0 || currentCount != updatedCount) {
logRecordsBetween(reader, currentCount, updatedCount);
currentCount = updatedCount;
updatedCount = getCurrentTableCount(reader);
+ }
+ } catch (Exception ex){
+ Log.e(TAG, "Reader test failed with exception", ex);
}
+
closeDatabase(reader, accessType);
Log.i(TAG, String.format("reader thread %d terminating", id));
}then my tests are passing OK, BOTH in emulator and on my device. The first instance of the I then made some quick adaptations to --- a/src/main/java/net/zetetic/tests/MultiThreadReadWriteTest.java
+++ b/src/main/java/net/zetetic/tests/MultiThreadReadWriteTest.java
@@ -1,8 +1,12 @@
package net.zetetic.tests;
import android.util.Log;
-import net.sqlcipher.Cursor;
-import net.sqlcipher.database.SQLiteDatabase;
+//import net.sqlcipher.Cursor;
+//import net.sqlcipher.database.SQLiteDatabase;
+//import net.sqlcipher.database.SQLiteException;
+import android.database.Cursor;
+import android.database.sqlite.SQLiteDatabase;
+import android.database.sqlite.SQLiteException;
import net.zetetic.ZeteticApplication;
import java.io.File;
@@ -19,18 +23,21 @@ public class MultiThreadReadWriteTest extends SQLCipherTest {
int threadCount = 40;
@Override
- public boolean execute(SQLiteDatabase database) {
+ public boolean execute(net.sqlcipher.database.SQLiteDatabase database) {
database.close();
executor = new ThreadExecutor(threadCount, recordsPerThread, DatabaseAccessType.Singleton);
return executor.run();
}
+ // NOTE: this function _could_ but is NOT EXPECTED to throw:
synchronized SQLiteDatabase getDatabase(DatabaseAccessType accessType) {
if (accessType == DatabaseAccessType.InstancePerRequest) {
- return SQLiteDatabase.openOrCreateDatabase(databaseFile, password, null);
+ //return SQLiteDatabase.openOrCreateDatabase(databaseFile, password, null);
+ return SQLiteDatabase.openOrCreateDatabase(databaseFile, null);
} else if (accessType == DatabaseAccessType.Singleton) {
if (instance == null) {
- instance = SQLiteDatabase.openOrCreateDatabase(databaseFile, password, null);
+ //instance = SQLiteDatabase.openOrCreateDatabase(databaseFile, password, null);
+ instance = SQLiteDatabase.openOrCreateDatabase(databaseFile, null);
}
return instance;
}Without my fixes, it would crash (in the first instance, if I am not mistaken). When I re-enabled my fixes to catch the exceptions, I had to add one more try-catch block in MultiThreadReadWriteTest.Reader.logRecordsBetween() to fix the crash (again, tried this only on my device so far): synchronized void logRecordsBetween(SQLiteDatabase reader, int start, int end) {
if(!reader.isOpen()) return;
- Cursor results = reader.rawQuery("select rowid, * from t1 where rowid between ? and ?",
+
+ Cursor results = null;
+ try {
+ results = reader.rawQuery("select rowid, * from t1 where rowid between ? and ?",
new String[]{String.valueOf(start), String.valueOf(end)});
+ } catch (Exception ex){
+ Log.e(TAG, "Reader test failed with exception", ex);
+ }
+
if (results != null) {
Log.i(TAG, String.format("reader thread %d - writing results %d to %d", id, start, end));So I think we can make the following conclusions:
At this point, what I am thinking is to make a version of the In general, I think we should try to reflect the normal Android database API as closely as possible. It is very good that this project does actually implement some of the normal Android interfaces (such as Cursor), and am thinking it would be cool to possibly implement even more. Any feedback from the project owners would be highly appreciated. |
|
I just tried porting a small part of the test suite to use the built-in Android database library instead of sqlcipher, with the following tests:
The first time it would run OK (only ran on my device so far). If I click the button to run again, it crashed. Then when I ran it yet again (by reopening and running, if I am not mistaken), it crashed with the following in the log: Also strangely enough, I do see in logcat that a couple other tests actually passed before the program crashed (trace from other programs filtered out by hand): I hadn't seen any |
|
@brodybits I don't think we've ever tried to compare the test suite against the standard SQLite
Fair enough, though that is really a misgiving of the state of the Java
That is an interesting idea. We could possibly push it down into the
Would you like to submit a pull request for the adjustments to the By the way - thank you for the thoughtful feedback on the test suite, we don't tend to receive much discussion on this! |
After a long day of trial-and-error I have now discovered that in the stock Android database Java classes, SQLiteDatabase.rawQuery() can throw SQLiteException, and both SQLiteDatabase.execSQL() & SQLiteDatabase.rawQuery() can throw IllegalStateException, however this is neither declared nor documented by the upstream Android project. I have reproduced/demonstrated these cases in the following project: https://github.com/brodybits/repeat-multi-threaded-sqlcipher-android-tests-without-sqlcipher and raised a issue on the Android project here: https://code.google.com/p/android/issues/detail?id=175351 I also submitted https://code.google.com/p/android/issues/detail?id=175353 that the SQLite database functions should always throw an instance or subclass of SQLiteException, to make it easier for the developer to check that all possible exceptions are handled at compile-time.
Right. This is really the fault of the (sqlite) database classes in the Android project and not the sqlcipher project. In general we do want to avoid diverging too far away from the stock Android database API, though just adding the throws declarations would not make the API of this project any less restrictive. I will look at this when I get a chance. Maybe, someday, they will fix this problem in the Android project, could be in a different way.
What I would really like to see is better abstraction, where someone has an Android database interface that exports at least the most important SQL functions regardless of which implementation s/he is actually using. Also perhaps an abstract Android database factory interface. Maybe someday I will look at this further. Unfortunately I have very limited experience dealing with annotations (came from C/C++ background).
Absolutely, maybe tomorrow. One more thing I would like to mention: taking another look at the error trace that I originally reported, it looks like an aberration (corner-case) to me since I am getting ready to finish after 12 hours of work today. |
This is great - thanks! 👍
Agreed - I just put together the
Ah, interesting. We should make sure this check is in place elsewhere.
Sounds like a good time to break! |
I already looked through the public |
|
I started preparing the changes to submit for When I ran the test suite again it passed OK. For the record, I am keeping the changes to the test suite where I saw this error in: https://github.com/brodybits/sqlcipher-android-tests/tree/fix-multi-thread-test-part-1 I will try to make a reproducible scenario for this, cannot promise when. Additional note: yes this error would be caught if the application catches |
…LiteDatabase.yieldIfContended* against closed database; some missing Javadoc in SQLiteDatabase.java
|
Thanks @developernotes for merging the fixes. I would still like to get to the bottom of the actual error that is thrown in the native code, someday. |
|
Any one tell me that how can i resolve this issue ... |
|
What version of SQLCipher for Android are you using? What Android OS version(s)/devices do you receive the error on? Are you able to reproduce the above error with a new test case in the SQLCipher for Android test suite? |
|
3.5.4 version of SQLCipher, Device is one plus 1 java.lang.RuntimeException: Unable to start activity ComponentInfo{com.girnar.hondadealer/com.girnar.lmsapp.ActionBarHomeActivity}: net.sqlcipher.database.SQLiteMisuseException: library routine called out of sequence: , while compiling: SELECT * FROM leadsTable WHERE nextScheduledDate < '2017-06-21' AND NOT Status = 'INVALID' AND NOT Status = 'INVALID' AND NOT Status = 'INVALID' |
|
Can you try upgrading to SQLCipher 3.5.7 (the latest) to see if the issue persists? Can you reproduce the issue within the test suite? |
|
Hello @mahendramahi - it might also be a good idea to take a look at issue #252 (comment) - it describes a scenario where a multithreaded application (i.e. with database operations occurring asynchronously) could use a database handle that had been closed, resulting in a misuse error. |
|
Thank you @sjlombardo and @developernotes 👍 |
I encountered this error one time when testing the changes in #175:
Second test round with the changes in #175 passed OK. I am very suspicious that this is an intermittent problem lurking in the library, and/or perhaps in the test.
This is caused by attempting to call
sqlite3_prepare16_v2()[innet_sqlcipher_database_SQLiteCompiledSql.cpp] with a sqite3 handle that is closed or invalid. For reference: http://stackoverflow.com/questions/8372871/library-routine-called-out-of-sequence-sqlite3-prepare-v2create-tableThe solution is to check that the database connection is open and valid before attempting to prepare (compile) the statement in native code. By "database connection", I am not sure yet whether this means that we should simply verify that the Java SQLiteDatabase is in the open state or that we should also check the internal db handle. I hope it will be enough to simply check the state of the SQLiteDatabase object.
The text was updated successfully, but these errors were encountered: