diff options
author | Vasu Nori <vnori@google.com> | 2010-02-18 13:35:42 -0800 |
---|---|---|
committer | Vasu Nori <vnori@google.com> | 2010-02-18 14:49:45 -0800 |
commit | e8de28415b4362824a52c180adf10dd882d12eaf (patch) | |
tree | 58762de7f639a841a955be6eedfd9d33e3c3eaa9 /core | |
parent | 6c2cc0d48ac51181d31017fbf8d735c9f91a1539 (diff) | |
download | frameworks_base-e8de28415b4362824a52c180adf10dd882d12eaf.zip frameworks_base-e8de28415b4362824a52c180adf10dd882d12eaf.tar.gz frameworks_base-e8de28415b4362824a52c180adf10dd882d12eaf.tar.bz2 |
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
Diffstat (limited to 'core')
4 files changed, 177 insertions, 3 deletions
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 @@ import android.util.Log; */ /* 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 @@ import android.util.Log; } } + /* 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 @@ public abstract class SQLiteProgram extends SQLiteClosable { // 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 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT 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 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT 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 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT 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 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT 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..af7ccce --- /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); + } +} |