Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

close() on anything other than database shouldn't acquire db lock.

Browse Source 
bug:2683001
implmentation details:
 1.close() on any sql statement is should simply queued up for finalization
    to be performed later. caller doesn't acquire database lock.
 2. the only effect of NOT close immediately is non-release of some memory.
 3. all such queued-up-finalizations are performed whenever there is
    another execute() of some sql statement on that database from ANY
    thread in the process.
 4. database close() automatically releases all unfinalized statements
    before closing the database.

Change-Id: If4c9c7fde6b8945a41abc6c8b992aa8c69854512
This commit is contained in:
Vasu Nori
2010-05-19 11:53:25 -07:00
parent 6b72dbe9c3
commit 6f37f83a48
8 changed files with 266 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
core/java/android/database/sqlite/SQLiteCompiledSql.java
View File

@@ -95,13 +95,8 @@ import android.util.Log;
if (SQLiteDebug.DEBUG_ACTIVE_CURSOR_FINALIZATION) {
Log.v(TAG, "closed and deallocated DbObj (id#" + nStatement +")");
}
try {
mDatabase.lock();
native_finalize();
nStatement = 0;
} finally {
mDatabase.unlock();
}
mDatabase.finalizeStatementLater(nStatement);
nStatement = 0;
}
}
@@ -159,5 +154,4 @@ import android.util.Log;
* @param sql The SQL to compile.
*/
private final native void native_compile(String sql);
private final native void native_finalize();
}

61
core/java/android/database/sqlite/SQLiteDatabase.java
View File

@@ -230,8 +230,8 @@ public class SQLiteDatabase extends SQLiteClosable {
// lock acquistions of the database.
/* package */ static final String GET_LOCK_LOG_PREFIX = "GETLOCK:";
/** Used by native code, do not rename */
/* package */ int mNativeHandle = 0;
/** Used by native code, do not rename. make it volatile, so it is thread-safe. */
/* package */ volatile int mNativeHandle = 0;
/** Used to make temp table names unique */
/* package */ int mTempTableSequence = 0;
@@ -313,6 +313,9 @@ public class SQLiteDatabase extends SQLiteClosable {
private static final String LOG_SLOW_QUERIES_PROPERTY = "db.log.slow_query_threshold";
private final int mSlowQueryThreshold;
/** stores the list of statement ids that need to be finalized by sqlite */
private ArrayList<Integer> mClosedStatementIds = new ArrayList<Integer>();
/** {@link DatabaseErrorHandler} to be used when SQLite returns any of the following errors
* Corruption
* */
@@ -1778,6 +1781,7 @@ public class SQLiteDatabase extends SQLiteClosable {
}
logTimeStat(mLastSqlStatement, timeStart, GET_LOCK_LOG_PREFIX);
try {
closePendingStatements();
native_execSQL(sql);
} catch (SQLiteDatabaseCorruptException e) {
onCorruption();
@@ -2101,6 +2105,52 @@ public class SQLiteDatabase extends SQLiteClosable {
mMaxSqlCacheSize = cacheSize;
}
/* package */ void finalizeStatementLater(int id) {
if (!isOpen()) {
// database already closed. this statement will already have been finalized.
return;
}
synchronized(mClosedStatementIds) {
if (mClosedStatementIds.contains(id)) {
// this statement id is already queued up for finalization.
return;
}
mClosedStatementIds.add(id);
}
}
/**
* public visibility only for testing. otherwise, package visibility is sufficient
* @hide
*/
public void closePendingStatements() {
if (!isOpen()) {
// since this database is already closed, no need to finalize anything.
mClosedStatementIds.clear();
return;
}
verifyLockOwner();
/* to minimize synchronization on mClosedStatementIds, make a copy of the list */
ArrayList<Integer> list = new ArrayList<Integer>(mClosedStatementIds.size());
synchronized(mClosedStatementIds) {
list.addAll(mClosedStatementIds);
mClosedStatementIds.clear();
}
// finalize all the statements from the copied list
int size = list.size();
for (int i = 0; i < size; i++) {
native_finalize(list.get(i));
}
}
/**
* for testing only
* @hide
*/
public ArrayList<Integer> getQueuedUpStmtList() {
return mClosedStatementIds;
}
static class ActiveDatabases {
private static final ActiveDatabases activeDatabases = new ActiveDatabases();
private HashSet<WeakReference<SQLiteDatabase>> mActiveDatabases =
@@ -2310,4 +2360,11 @@ public class SQLiteDatabase extends SQLiteClosable {
* @return int value of SQLITE_DBSTATUS_LOOKASIDE_USED
*/
private native int native_getDbLookaside();
/**
* finalizes the given statement id.
*
* @param statementId statement to be finzlied by sqlite
*/
private final native void native_finalize(int statementId);
}

1
core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
View File

@@ -43,6 +43,7 @@ public class SQLiteDirectCursorDriver implements SQLiteCursorDriver {
try {
mDatabase.lock();
mDatabase.closePendingStatements();
query = new SQLiteQuery(mDatabase, mSql, 0, selectionArgs);
// Arg binding
int numArgs = selectionArgs == null ? 0 : selectionArgs.length;

7
core/java/android/database/sqlite/SQLiteProgram.java
View File

@@ -291,12 +291,7 @@ public abstract class SQLiteProgram extends SQLiteClosable {
if (!mDatabase.isOpen()) {
return;
}
mDatabase.lock();
try {
releaseReference();
} finally {
mDatabase.unlock();
}
releaseReference();
}
/**

4
core/java/android/database/sqlite/SQLiteStatement.java
View File

@@ -55,6 +55,7 @@ public class SQLiteStatement extends SQLiteProgram
acquireReference();
try {
mDatabase.closePendingStatements();
native_execute();
mDatabase.logTimeStat(mSql, timeStart);
} finally {
@@ -81,6 +82,7 @@ public class SQLiteStatement extends SQLiteProgram
acquireReference();
try {
mDatabase.closePendingStatements();
native_execute();
mDatabase.logTimeStat(mSql, timeStart);
return (mDatabase.lastChangeCount() > 0) ? mDatabase.lastInsertRow() : -1;
@@ -107,6 +109,7 @@ public class SQLiteStatement extends SQLiteProgram
acquireReference();
try {
mDatabase.closePendingStatements();
long retValue = native_1x1_long();
mDatabase.logTimeStat(mSql, timeStart);
return retValue;
@@ -133,6 +136,7 @@ public class SQLiteStatement extends SQLiteProgram
acquireReference();
try {
mDatabase.closePendingStatements();
String retValue = native_1x1_string();
mDatabase.logTimeStat(mSql, timeStart);
return retValue;

11
core/jni/android_database_SQLiteCompiledSql.cpp
View File

@@ -91,22 +91,11 @@ static void native_compile(JNIEnv* env, jobject object, jstring sqlString)
compile(env, object, GET_HANDLE(env, object), sqlString);
}
static void native_finalize(JNIEnv* env, jobject object)
{
int err;
sqlite3_stmt * statement = GET_STATEMENT(env, object);
if (statement != NULL) {
sqlite3_finalize(statement);
env->SetIntField(object, gStatementField, 0);
}
}
static JNINativeMethod sMethods[] =
{
/* name, signature, funcPtr */
{"native_compile", "(Ljava/lang/String;)V", (void *)native_compile},
{"native_finalize", "()V", (void *)native_finalize},
};
int register_android_database_SQLiteCompiledSql(JNIEnv * env)

13
core/jni/android_database_SQLiteDatabase.cpp
View File

@@ -215,6 +215,7 @@ static void enableSqlProfiling(JNIEnv* env, jobject object, jstring databaseName
static void dbclose(JNIEnv* env, jobject object)
{
sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
sqlite3_stmt * pStmt;
if (handle != NULL) {
// release the memory associated with the traceFuncArg in enableSqlTracing function
@@ -227,6 +228,10 @@ static void dbclose(JNIEnv* env, jobject object)
if (traceFuncArg != NULL) {
free(traceFuncArg);
}
// finalize all statements on this handle
while ((pStmt = sqlite3_next_stmt(handle, 0)) != 0 ) {
sqlite3_finalize(pStmt);
}
LOGV("Closing database: handle=%p\n", handle);
int result = sqlite3_close(handle);
if (result == SQLITE_OK) {
@@ -443,6 +448,13 @@ static jint native_releaseMemory(JNIEnv *env, jobject clazz)
return sqlite3_release_memory(SQLITE_SOFT_HEAP_LIMIT);
}
static void native_finalize(JNIEnv* env, jobject object, jint statementId)
{
if (statementId > 0) {
sqlite3_finalize((sqlite3_stmt *)statementId);
}
}
static JNINativeMethod sMethods[] =
{
/* name, signature, funcPtr */
@@ -456,6 +468,7 @@ static JNINativeMethod sMethods[] =
{"native_setLocale", "(Ljava/lang/String;I)V", (void *)native_setLocale},
{"native_getDbLookaside", "()I", (void *)native_getDbLookaside},
{"releaseMemory", "()I", (void *)native_releaseMemory},
{"native_finalize", "(I)V", (void *)native_finalize},
};
int register_android_database_SQLiteDatabase(JNIEnv *env)

186
core/tests/coretests/src/android/database/DatabaseGeneralTest.java
View File

@@ -1232,4 +1232,190 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
fail("unexpected");
}
}
/**
* test to make sure the statement finalizations are not done right away but
* piggybacked onto the next sql statement execution on the same database.
*/
@SmallTest
public void testStatementClose() {
mDatabase.execSQL("CREATE TABLE test (i int);");
// fill up statement cache in mDatabase\
int N = 26;
mDatabase.setMaxSqlCacheSize(N);
SQLiteStatement stmt;
int stmt0Id = 0;
for (int i = 0; i < N; i ++) {
stmt = mDatabase.compileStatement("insert into test values(" + i + ");");
stmt.executeInsert();
// keep track of 0th entry
if (i == 0) {
stmt0Id = stmt.getUniqueId();
}
stmt.close();
}
// add one more to the cache - and the above 'stmt0Id' should fall out of cache
SQLiteStatement stmt1 = mDatabase.compileStatement("select * from test where i = 1;");
stmt1.close();
// the above close() should have queuedUp the statement for finalization
ArrayList<Integer> statementIds = mDatabase.getQueuedUpStmtList();
assertTrue(statementIds.contains(stmt0Id));
// execute something to see if this statement gets finalized
mDatabase.execSQL("delete from test where i = 10;");
statementIds = mDatabase.getQueuedUpStmtList();
assertEquals(0, statementIds.size());
}
/**
* same as above - except that the statement to be finalized is from Thread # 1.
* and it is eventually finalized in Thread # 2 when it executes a sql statement.
* @throws InterruptedException
*/
@LargeTest
public void testStatementCloseDiffThread() throws InterruptedException {
mDatabase.execSQL("CREATE TABLE test (i int);");
// fill up statement cache in mDatabase in a thread
Thread t1 = new Thread() {
@Override public void run() {
int N = 26;
mDatabase.setMaxSqlCacheSize(N);
SQLiteStatement stmt;
for (int i = 0; i < N; i ++) {
stmt = mDatabase.compileStatement("insert into test values(" + i + ");");
stmt.executeInsert();
// keep track of 0th entry
if (i == 0) {
setStmt0Id(stmt.getUniqueId());
}
stmt.close();
}
}
};
t1.start();
// wait for the thread to finish
t1.join();
// add one more to the cache - and the above 'stmt0Id' should fall out of cache
// just for the heck of it, do it in a separate thread
Thread t2 = new Thread() {
@Override public void run() {
SQLiteStatement stmt1 = mDatabase.compileStatement(
"select * from test where i = 1;");
stmt1.close();
}
};
t2.start();
t2.join();
// close() in the above thread should have queuedUp the statement for finalization
ArrayList<Integer> statementIds = mDatabase.getQueuedUpStmtList();
assertTrue(getStmt0Id() > 0);
assertTrue(statementIds.contains(stmt0Id));
assertEquals(1, statementIds.size());
// execute something to see if this statement gets finalized
// again do it in a separate thread
Thread t3 = new Thread() {
@Override public void run() {
mDatabase.execSQL("delete from test where i = 10;");
}
};
t3.start();
t3.join();
// is the statement finalized?
statementIds = mDatabase.getQueuedUpStmtList();
assertEquals(0, statementIds.size());
}
private volatile int stmt0Id = 0;
private synchronized void setStmt0Id(int stmt0Id) {
this.stmt0Id = stmt0Id;
}
private synchronized int getStmt0Id() {
return this.stmt0Id;
}
/**
* same as above - except that the queue of statements to be finalized are finalized
* by database close() operation.
*/
@LargeTest
public void testStatementCloseByDbClose() throws InterruptedException {
mDatabase.execSQL("CREATE TABLE test (i int);");
// fill up statement cache in mDatabase in a thread
Thread t1 = new Thread() {
@Override public void run() {
int N = 26;
mDatabase.setMaxSqlCacheSize(N);
SQLiteStatement stmt;
for (int i = 0; i < N; i ++) {
stmt = mDatabase.compileStatement("insert into test values(" + i + ");");
stmt.executeInsert();
// keep track of 0th entry
if (i == 0) {
setStmt0Id(stmt.getUniqueId());
}
stmt.close();
}
}
};
t1.start();
// wait for the thread to finish
t1.join();
// add one more to the cache - and the above 'stmt0Id' should fall out of cache
// just for the heck of it, do it in a separate thread
Thread t2 = new Thread() {
@Override public void run() {
SQLiteStatement stmt1 = mDatabase.compileStatement(
"select * from test where i = 1;");
stmt1.close();
}
};
t2.start();
t2.join();
// close() in the above thread should have queuedUp the statement for finalization
ArrayList<Integer> statementIds = mDatabase.getQueuedUpStmtList();
assertTrue(getStmt0Id() > 0);
assertTrue(statementIds.contains(stmt0Id));
assertEquals(1, statementIds.size());
// close the database. native method dbclose() will finalize all statements
// before closing the database.
// again do it in a separate thread
Thread t3 = new Thread() {
@Override public void run() {
mDatabase.close();
}
};
t3.start();
t3.join();
// check mClosedStatementIds in mDatabase. it should still have 'stmt0Id'
statementIds = mDatabase.getQueuedUpStmtList();
assertTrue(statementIds.contains(stmt0Id));
// try to finalize the pending statements. and there should be no exceptions from anywhere
// just for the heck of it, do it in a separate thread
Thread t4 = new Thread() {
@Override public void run() {
try {
mDatabase.closePendingStatements();
} catch (Exception e) {
fail("not expected");
}
}
};
t4.start();
t4.join();
// mClosedStatementIds in mDatabase should be empty
statementIds = mDatabase.getQueuedUpStmtList();
assertEquals(0, statementIds.size());
}
}