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

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

5 files changed, 67 insertions, 16 deletions

diff --git a/core/java/android/database/sqlite/SQLiteCompiledSql.java b/core/java/android/database/sqlite/SQLiteCompiledSql.java
index 16ff2ab..9889a21 100644
--- a/core/java/android/database/sqlite/SQLiteCompiledSql.java
+++ b/core/java/android/database/sqlite/SQLiteCompiledSql.java
@@ -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();
 }
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index 47de286..e6f54e2 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -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);
 }
diff --git a/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java b/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
index ac60b27..be49257 100644
--- a/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
+++ b/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
@@ -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;
diff --git a/core/java/android/database/sqlite/SQLiteProgram.java b/core/java/android/database/sqlite/SQLiteProgram.java
index c37385c0..dc1a0ee 100644
--- a/core/java/android/database/sqlite/SQLiteProgram.java
+++ b/core/java/android/database/sqlite/SQLiteProgram.java
@@ -291,12 +291,7 @@ public abstract class SQLiteProgram extends SQLiteClosable {
         if (!mDatabase.isOpen()) {
             return;
         }
-        mDatabase.lock();
-        try {
-            releaseReference();
-        } finally {
-            mDatabase.unlock();
-        }
+        releaseReference();
     }
 
     /**
diff --git a/core/java/android/database/sqlite/SQLiteStatement.java b/core/java/android/database/sqlite/SQLiteStatement.java
index 47cca87..ba0e63b 100644
--- a/core/java/android/database/sqlite/SQLiteStatement.java
+++ b/core/java/android/database/sqlite/SQLiteStatement.java
@@ -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;