From 9463f297ef51bd056626d0fb0708c83509f7ebd9 Mon Sep 17 00:00:00 2001
From: Vasu Nori <vnori@google.com>
Date: Fri, 30 Apr 2010 12:22:18 -0700
Subject: [PATCH] sometimes database lock is not held before compiling a
 statement

indirectly uncovered by bug:2631971

Change-Id: I71059b6b62bc21612fc98689aefd4534884c8e7c
---
 .../database/sqlite/SQLiteCompiledSql.java    | 15 ++------
 .../database/sqlite/SQLiteDatabase.java       | 37 ++++++++++++-------
 .../sqlite/SQLiteDirectCursorDriver.java      |  5 ++-
 3 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/core/java/android/database/sqlite/SQLiteCompiledSql.java b/core/java/android/database/sqlite/SQLiteCompiledSql.java
index d5c6ad15882cd..16ff2ab6e3e95 100644
--- a/core/java/android/database/sqlite/SQLiteCompiledSql.java
+++ b/core/java/android/database/sqlite/SQLiteCompiledSql.java
@@ -78,20 +78,13 @@ import android.util.Log;
      *  existing compiled SQL program already around
      */
     private void compile(String sql, boolean forceCompilation) {
-        if (!mDatabase.isOpen()) {
-            throw new IllegalStateException("database " + mDatabase.getPath() + " already closed");
-        }
+        mDatabase.verifyLockOwner();
         // Only compile if we don't have a valid statement already or the caller has
         // explicitly requested a recompile.
         if (forceCompilation) {
-            mDatabase.lock();
-            try {
-                // Note that the native_compile() takes care of destroying any previously
-                // existing programs before it compiles.
-                native_compile(sql);
-            } finally {
-                mDatabase.unlock();
-            }
+            // Note that the native_compile() takes care of destroying any previously
+            // existing programs before it compiles.
+            native_compile(sql);
         }
     }
 
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index 3a83c0c9d29f9..e64e2c3176c9f 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -275,20 +275,22 @@ public class SQLiteDatabase extends SQLiteClosable {
                 // if it needs to be removed to accommodate a new entry,
                 //     close {@link SQLiteCompiledSql} represented by this entry, if not in use
                 //     and then let it be removed from the Map.
-                synchronized(mCompiledQueries) { // probably not necessary, but can't hurt
-                    if (this.size() <= mMaxSqlCacheSize) {
-                        // cache is not full. nothing needs to be removed
-                        return false;
-                    }
-                    // cache is full. eldest will be removed.
-                    SQLiteCompiledSql entry = eldest.getValue();
-                    if (!entry.isInUse()) {
-                        // this {@link SQLiteCompiledSql} is not in use. release it.
-                        entry.releaseSqlStatement();
-                    }
-                    // return true, so that this entry is removed automatically by the caller.
-                    return true;
+                // when this is called, the caller must be trying to add a just-compiled stmt
+                // to cache; i.e., caller should already have acquired database lock AND
+                // the lock on mCompiledQueries. do as assert of these two 2 facts.
+                verifyLockOwner();
+                if (this.size() <= mMaxSqlCacheSize) {
+                    // cache is not full. nothing needs to be removed
+                    return false;
                 }
+                // cache is full. eldest will be removed.
+                SQLiteCompiledSql entry = eldest.getValue();
+                if (!entry.isInUse()) {
+                    // this {@link SQLiteCompiledSql} is not in use. release it.
+                    entry.releaseSqlStatement();
+                }
+                // return true, so that this entry is removed automatically by the caller.
+                return true;
             }
         };
     /**
@@ -1979,6 +1981,15 @@ public class SQLiteDatabase extends SQLiteClosable {
         }
     }
 
+    /* package */ void verifyLockOwner() {
+        if (!isOpen()) {
+            throw new IllegalStateException("database " + getPath() + " already closed");
+        }
+        if (!isDbLockedByCurrentThread() && mLockingEnabled) {
+            throw new IllegalStateException("Don't have database lock!");
+        }
+    }
+
     /*
      * ============================================================================
      *
diff --git a/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java b/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
index 2144fc3f756ad..ac60b2706619e 100644
--- a/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
+++ b/core/java/android/database/sqlite/SQLiteDirectCursorDriver.java
@@ -39,9 +39,11 @@ public class SQLiteDirectCursorDriver implements SQLiteCursorDriver {
 
     public Cursor query(CursorFactory factory, String[] selectionArgs) {
         // Compile the query
-        SQLiteQuery query = new SQLiteQuery(mDatabase, mSql, 0, selectionArgs);
+        SQLiteQuery query = null;
 
         try {
+            mDatabase.lock();
+            query = new SQLiteQuery(mDatabase, mSql, 0, selectionArgs);
             // Arg binding
             int numArgs = selectionArgs == null ? 0 : selectionArgs.length;
             for (int i = 0; i < numArgs; i++) {
@@ -61,6 +63,7 @@ public class SQLiteDirectCursorDriver implements SQLiteCursorDriver {
         } finally {
             // Make sure this object is cleaned up if something happens
             if (query != null) query.close();
+            mDatabase.unlock();
         }
     }