bug fix for 2419869. also included 2 unittests.
bug fix for 2419869 is the following
1. only one object can use the prepared statement object
(SQLiteCompiledSql in SQLIteProgram)
2. if two objects are requesting to use it, then create a new prepared
statement object for exclusive use by the newcomer and let it be
be finalized by the newcomer.
3. add mInUse flag to SQLiteCompiledSql - to be set when SQLiteProgram
requests it and to be released when that SQLiteProgram is done with it
a couple more changes included are
1. unitests to simulate bug # 2419869 (and the fix's repair to it)
2. better logging in SQLiteCloseable when it prints log messages
diff --git a/core/java/android/database/sqlite/SQLiteCompiledSql.java b/core/java/android/database/sqlite/SQLiteCompiledSql.java
index 79527b4..eb85822 100644
--- a/core/java/android/database/sqlite/SQLiteCompiledSql.java
+++ b/core/java/android/database/sqlite/SQLiteCompiledSql.java
@@ -44,6 +44,9 @@
*/
/* package */ int nStatement = 0;
+ /** when in cache and is in use, this member is set */
+ private boolean mInUse = false;
+
/* package */ SQLiteCompiledSql(SQLiteDatabase db, String sql) {
mDatabase = db;
this.nHandle = db.mNativeHandle;
@@ -92,6 +95,18 @@
}
}
+ /* package */ synchronized boolean isInUse() {
+ return mInUse;
+ }
+
+ /* package */ synchronized void acquire() {
+ mInUse = true;
+ }
+
+ /* package */ synchronized void release() {
+ mInUse = false;
+ }
+
/**
* Make sure that the native resource is cleaned up.
*/
diff --git a/core/java/android/database/sqlite/SQLiteProgram.java b/core/java/android/database/sqlite/SQLiteProgram.java
index 2d0aa39..2bb2f5d 100644
--- a/core/java/android/database/sqlite/SQLiteProgram.java
+++ b/core/java/android/database/sqlite/SQLiteProgram.java
@@ -58,32 +58,51 @@
// add it to the cache of compiled-sqls
db.addToCompiledQueries(sql, mCompiledSql);
+ mCompiledSql.acquire();
+ } else {
+ // it is already in compiled-sql cache.
+ if (mCompiledSql.isInUse()) {
+ // but the CompiledSql in cache is in use by some other SQLiteProgram object.
+ // we can't have two different SQLiteProgam objects can't share the same
+ // CompiledSql object. create a new one.
+ // finalize it when I am done with it in "this" object.
+ mCompiledSql = new SQLiteCompiledSql(db, sql);
+ } else {
+ // the CompiledSql in cache is NOT in use by any other SQLiteProgram object.
+ // it is safe to give it to this SQLIteProgram Object.
+ mCompiledSql.acquire();
+ }
}
nStatement = mCompiledSql.nStatement;
}
@Override
protected void onAllReferencesReleased() {
- releaseCompiledSqlIfInCache();
+ releaseCompiledSqlIfNotInCache();
mDatabase.releaseReference();
mDatabase.removeSQLiteClosable(this);
}
@Override
protected void onAllReferencesReleasedFromContainer() {
- releaseCompiledSqlIfInCache();
+ releaseCompiledSqlIfNotInCache();
mDatabase.releaseReference();
}
- private void releaseCompiledSqlIfInCache() {
+ private void releaseCompiledSqlIfNotInCache() {
if (mCompiledSql == null) {
return;
}
synchronized(mDatabase.mCompiledQueries) {
if (!mDatabase.mCompiledQueries.containsValue(mCompiledSql)) {
+ // it is NOT in compiled-sql cache. i.e., responsibility of
+ // release this statement is on me.
mCompiledSql.releaseSqlStatement();
mCompiledSql = null; // so that GC doesn't call finalize() on it
nStatement = 0;
+ } else {
+ // it is in compiled-sql cache. reset its CompiledSql#mInUse flag
+ mCompiledSql.release();
}
}
}
diff --git a/core/tests/coretests/src/android/database/DatabaseGeneralTest.java b/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
index ca650e0..e43031c 100644
--- a/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
+++ b/core/tests/coretests/src/android/database/DatabaseGeneralTest.java
@@ -944,6 +944,7 @@
Assert.assertNull(cur.getString(3));
Assert.assertEquals(1234, cur.getLong(4));
Assert.assertNull(cur.getString(5));
+ cur.close();
cv = new ContentValues();
cv.put("s", "two");
@@ -956,6 +957,7 @@
Assert.assertNull(cur.getString(3));
Assert.assertEquals(1234, cur.getLong(4));
Assert.assertNull(cur.getString(5));
+ cur.close();
cv = new ContentValues();
cv.put("t", "goodbye world");
@@ -975,6 +977,7 @@
Assert.assertEquals(2345, cur.getLong(3));
Assert.assertEquals(3456, cur.getLong(4));
Assert.assertEquals("tricky", cur.getString(5));
+ cur.close();
cv = new ContentValues();
cv.put("s", "three");
@@ -987,6 +990,7 @@
Assert.assertEquals("three", cur.getString(1));
Assert.assertEquals("hello world", cur.getString(2));
Assert.assertEquals(6789, cur.getLong(3));
+ cur.close();
ih.close();
}
diff --git a/core/tests/coretests/src/android/database/sqlite/SQLiteGeneralTest.java b/core/tests/coretests/src/android/database/sqlite/SQLiteGeneralTest.java
new file mode 100644
index 0000000..af7ccceb
--- /dev/null
+++ b/core/tests/coretests/src/android/database/sqlite/SQLiteGeneralTest.java
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2006 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.database.sqlite;
+
+import android.content.Context;
+import android.test.AndroidTestCase;
+import android.test.FlakyTest;
+import android.test.suitebuilder.annotation.LargeTest;
+
+import java.io.File;
+
+public class SQLiteGeneralTest extends AndroidTestCase {
+
+ private SQLiteDatabase mDatabase;
+ private File mDatabaseFile;
+ Boolean exceptionRecvd = false;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ exceptionRecvd = false;
+ File dbDir = getContext().getDir(this.getClass().getName(), Context.MODE_PRIVATE);
+ mDatabaseFile = new File(dbDir, "database_test.db");
+ if (mDatabaseFile.exists()) {
+ mDatabaseFile.delete();
+ }
+ mDatabase = SQLiteDatabase.openOrCreateDatabase(mDatabaseFile.getPath(), null);
+ assertNotNull(mDatabase);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ mDatabase.close();
+ mDatabaseFile.delete();
+ super.tearDown();
+ }
+
+ @LargeTest
+ public void testUseOfSameSqlStatementBy2Threads() throws Exception {
+ mDatabase.execSQL("CREATE TABLE test_pstmt (i INTEGER PRIMARY KEY, j text);");
+
+ // thread 1 creates a prepared statement
+ final String stmt = "SELECT * FROM test_pstmt WHERE i = ?";
+
+ // start 2 threads to do repeatedly execute "stmt"
+ // since these 2 threads are executing the same sql, they each should get
+ // their own copy and
+ // there SHOULD NOT be an error from sqlite: "prepared statement is busy"
+ class RunStmtThread extends Thread {
+ private static final int N = 1000;
+ @Override public void run() {
+ int i = 0;
+ try {
+ // execute many times
+ for (i = 0; i < N; i++) {
+ SQLiteStatement s1 = mDatabase.compileStatement(stmt);
+ s1.bindLong(1, i);
+ s1.execute();
+ s1.close();
+ }
+ } catch (SQLiteException e) {
+ fail("SQLiteException: " + e.getMessage());
+ return;
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("random unexpected exception: " + e.getMessage());
+ return;
+ }
+ }
+ }
+ RunStmtThread t1 = new RunStmtThread();
+ t1.start();
+ RunStmtThread t2 = new RunStmtThread();
+ t2.start();
+ while (t1.isAlive() || t2.isAlive()) {
+ Thread.sleep(1000);
+ }
+ }
+
+ @FlakyTest
+ public void testUseOfSamePreparedStatementBy2Threads() throws Exception {
+ mDatabase.execSQL("CREATE TABLE test_pstmt (i INTEGER PRIMARY KEY, j text);");
+
+ // thread 1 creates a prepared statement
+ final String stmt = "SELECT * FROM test_pstmt WHERE i = ?";
+ final SQLiteStatement s1 = mDatabase.compileStatement(stmt);
+
+ // start 2 threads to do repeatedly execute "stmt"
+ // since these 2 threads are executing the same prepared statement,
+ // should see an error from sqlite: "prepared statement is busy"
+ class RunStmtThread extends Thread {
+ private static final int N = 1000;
+ @Override public void run() {
+ int i = 0;
+ try {
+ // execute many times
+ for (i = 0; i < N; i++) {
+ s1.bindLong(1, i);
+ s1.execute();
+ }
+ } catch (SQLiteException e) {
+ // expect it
+ assertTrue(e.getMessage().contains("library routine called out of sequence:"));
+ exceptionRecvd = true;
+ return;
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("random unexpected exception: " + e.getMessage());
+ return;
+ }
+ }
+ }
+ RunStmtThread t1 = new RunStmtThread();
+ t1.start();
+ RunStmtThread t2 = new RunStmtThread();
+ t2.start();
+ while (t1.isAlive() || t2.isAlive()) {
+ Thread.sleep(1000);
+ }
+ assertTrue(exceptionRecvd);
+ }
+}
