Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2827d6d9 authored by Vasu Nori's avatar Vasu Nori
Browse files

for WAL to work, can't keep prepared SQL stmt_id in SQLiteStatement 

Some (including the Contacts app) do the following:
  1. Open database
  2. As part of database_connection.onCreate(),
       Create some SQLiteStatement objects to cache them in the process
  3. attach databases
WAL doesn't work with attached databases. so, apps doing the above
should enable WAL only if there are no attached databases.

But we would like to enable WAL automatically for all apps after step #1 above
and disable WAL if the app subsequently does 'attach database' SQL.

this works only if there are no SQLiteStatement objects created in step # 2,
because SQLiteStatements cwmaintain a hard-reference to the database connection
for life and also to the prepared SQL statement id.
It is quite difficult to disable WAL in step # 3
    if it is enabled in step # 1
    and then a connection pool gets used by step # 2

would make WAL disabling easier if SQLiteStatement refers to prepared SQL
statement id only when it is needed (during binding and execute calls)
and thus NOT tied to a spacific database conenction.

also, from the standpoint of not blocking readers, it helps NOT to have
SQLiteStatement be married to a database connection and prepared SQL statement
id for life.

Change-Id: I464d57042965a28d2bde88e0f44b66ec119b40dc
parent 7fbee2f7

core/java/android/database/sqlite/SQLiteDatabase.java

+14 −32
Original line number Diff line number Diff line
@@ -343,25 +343,14 @@ public class SQLiteDatabase extends SQLiteClosable { 


    private static final String MEMORY_DB_PATH = ":memory:";



    /**

     * @param closable

     */

    void addSQLiteClosable(SQLiteClosable closable) {

        lock();

        try {

    synchronized void addSQLiteClosable(SQLiteClosable closable) {

        // mPrograms is per instance of SQLiteDatabase and it doesn't actually touch the database

        // itself. so, there is no need to lock().

        mPrograms.put(closable, null);

        } finally {

            unlock();

        }

    }



    void removeSQLiteClosable(SQLiteClosable closable) {

        lock();

        try {

    synchronized void removeSQLiteClosable(SQLiteClosable closable) {

        mPrograms.remove(closable);

        } finally {

            unlock();

        }

    }



    @Override
@@ -1261,6 +1250,8 @@ public class SQLiteDatabase extends SQLiteClosable { 
     * statement and fill in those values with {@link SQLiteProgram#bindString}

     * and {@link SQLiteProgram#bindLong} each time you want to run the

     * statement. Statements may not return result sets larger than 1x1.

     *<p>

     * No two threads should be using the same {@link SQLiteStatement} at the same time.

     *

     * @param sql The raw SQL statement, may contain ? for unknown values to be

     *            bound later.
@@ -1269,19 +1260,7 @@ public class SQLiteDatabase extends SQLiteClosable { 
     */

    public SQLiteStatement compileStatement(String sql) throws SQLException {

        verifyDbIsOpen();

        String prefixSql = sql.trim().substring(0, 6);

        SQLiteDatabase db = this;

        // get a pooled database connection handle to use, if this is a query

        if (prefixSql.equalsIgnoreCase("SELECT")) {

            db = getDbConnection(sql);

        }

        db.lock();

        try {

            return new SQLiteStatement(db, sql);

        } finally {

            releaseDbConnection(db);

            db.unlock();

        }

        return new SQLiteStatement(this, sql);

    }



    /**
@@ -2354,7 +2333,10 @@ public class SQLiteDatabase extends SQLiteClosable { 
        return true;

    }



    private synchronized void disableWriteAheadLogging() {

    /**

     * package visibility only for testing purposes

     */

    /* package */ synchronized void disableWriteAheadLogging() {

        if (mConnectionPool == null) {

            return;

        }
@@ -2394,7 +2376,7 @@ public class SQLiteDatabase extends SQLiteClosable { 
        return this.mConnectionNum > 0;

    }



    private SQLiteDatabase getDbConnection(String sql) {

    /* package */ SQLiteDatabase getDbConnection(String sql) {

        verifyDbIsOpen();



        // use the current connection handle if

core/java/android/database/sqlite/SQLiteProgram.java

+39 −14
Original line number Diff line number Diff line
@@ -64,11 +64,16 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
    protected int nStatement = 0;



    /* package */ SQLiteProgram(SQLiteDatabase db, String sql) {

        mDatabase = db;

        this(db, sql, true);

    }



    /* package */ SQLiteProgram(SQLiteDatabase db, String sql, boolean compileFlag) {

        mSql = sql.trim();

        attachObjectToDatabase(db);

        if (compileFlag) {

            compileSql();

        }

    }



    private void compileSql() {

        if (nStatement > 0) {
@@ -139,6 +144,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
    private synchronized void attachObjectToDatabase(SQLiteDatabase db) {

        db.acquireReference();

        db.addSQLiteClosable(this);

        mDatabase = db;

        nHandle = db.mNativeHandle;

    }
@@ -147,6 +153,26 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
        mDatabase.releaseReference();

    }



    /* package */ synchronized void verifyDbAndCompileSql() {

        mDatabase.verifyDbIsOpen();

        // use pooled database connection handles for SELECT SQL statements

        SQLiteDatabase db = (getSqlStatementType(mSql) != SELECT_STMT) ? mDatabase

                : mDatabase.getDbConnection(mSql);

        if (!db.equals(mDatabase)) {

            // the database connection handle to be used is not the same as the one supplied

            // in the constructor. do some housekeeping.

            detachObjectFromDatabase();

            attachObjectToDatabase(db);

        }

        // compile the sql statement

        mDatabase.lock();

        try {

            compileSql();

        } finally {

            mDatabase.unlock();

        }

    }



    @Override

    protected void onAllReferencesReleased() {

        releaseCompiledSqlIfNotInCache();
@@ -159,7 +185,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
        mDatabase.releaseReference();

    }



    private void releaseCompiledSqlIfNotInCache() {

    /* package */ synchronized void releaseCompiledSqlIfNotInCache() {

        if (mCompiledSql == null) {

            return;

        }
@@ -222,7 +248,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
     */

    public void bindNull(int index) {

        synchronized (this) {

            mDatabase.verifyDbIsOpen();

            verifyDbAndCompileSql();

            acquireReference();

            try {

                native_bind_null(index);
@@ -241,7 +267,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
     */

    public void bindLong(int index, long value) {

        synchronized (this) {

            mDatabase.verifyDbIsOpen();

            verifyDbAndCompileSql();

            acquireReference();

            try {

                native_bind_long(index, value);
@@ -260,7 +286,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
     */

    public void bindDouble(int index, double value) {

        synchronized (this) {

            mDatabase.verifyDbIsOpen();

            verifyDbAndCompileSql();

            acquireReference();

            try {

                native_bind_double(index, value);
@@ -282,7 +308,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
            throw new IllegalArgumentException("the bind value at index " + index + " is null");

        }

        synchronized (this) {

            mDatabase.verifyDbIsOpen();

            verifyDbAndCompileSql();

            acquireReference();

            try {

                native_bind_string(index, value);
@@ -304,7 +330,7 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
            throw new IllegalArgumentException("the bind value at index " + index + " is null");

        }

        synchronized (this) {

            mDatabase.verifyDbIsOpen();

            verifyDbAndCompileSql();

            acquireReference();

            try {

                native_bind_blob(index, value);
@@ -319,6 +345,9 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
     */

    public void clearBindings() {

        synchronized (this) {

            if (this.nStatement == 0) {

                return;

            }

            mDatabase.verifyDbIsOpen();

            acquireReference();

            try {
@@ -334,14 +363,10 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
     */

    public void close() {

        synchronized (this) {

            if (nStatement == 0 || nHandle == 0 || !mDatabase.isOpen()) {

            if (nHandle == 0 || !mDatabase.isOpen()) {

                return;

            }

            releaseReference();

            // set all database objects to null/0, so that the user can't use a closed Object.

            mCompiledSql = null;

            nStatement = 0;

            nHandle = 0;

        }

    }
@@ -366,6 +391,6 @@ public abstract class SQLiteProgram extends SQLiteClosable { 
    protected final native void native_bind_double(int index, double value);

    protected final native void native_bind_string(int index, String value);

    protected final native void native_bind_blob(int index, byte[] value);

    private final native void native_clear_bindings();

    /* package */ final native void native_clear_bindings();

}

core/java/android/database/sqlite/SQLiteStatement.java

+7 −2
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import dalvik.system.BlockGuard; 
 * SQLiteStatement is not internally synchronized so code using a SQLiteStatement from multiple

 * threads should perform its own synchronization when using the SQLiteStatement.

 */

@SuppressWarnings("deprecation")

public class SQLiteStatement extends SQLiteProgram

{

    private static final boolean READ = true;
@@ -41,7 +42,7 @@ public class SQLiteStatement extends SQLiteProgram 
     * @param sql

     */

    /* package */ SQLiteStatement(SQLiteDatabase db, String sql) {

        super(db, sql);

        super(db, sql, false /* don't compile sql statement */);

    }



    /**
@@ -134,7 +135,7 @@ public class SQLiteStatement extends SQLiteProgram 
     * methods in this class.

     */

    private long acquireAndLock(boolean rwFlag) {

        mDatabase.verifyDbIsOpen();

        verifyDbAndCompileSql();

        if (rwFlag == WRITE) {

            BlockGuard.getThreadPolicy().onWriteToDisk();

        } else {
@@ -153,6 +154,10 @@ public class SQLiteStatement extends SQLiteProgram 
    private void releaseAndUnlock() {

        releaseReference();

        mDatabase.unlock();

        clearBindings();

        // release the compiled sql statement so that the caller's SQLiteStatement no longer

        // has a hard reference to a database object that may get deallocated at any point.

        releaseCompiledSqlIfNotInCache();

    }



    private final native void native_execute();

core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java

+26 −14
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 


    @SmallTest

    public void testEnableWriteAheadLogging() {

        mDatabase.disableWriteAheadLogging();

        assertNull(mDatabase.mConnectionPool);

        mDatabase.enableWriteAheadLogging();

        DatabaseConnectionPool pool = mDatabase.mConnectionPool;
@@ -280,6 +281,7 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 


    @SmallTest

    public void testLruCachingOfSqliteCompiledSqlObjs() {

        mDatabase.disableWriteAheadLogging();

        mDatabase.execSQL("CREATE TABLE test (i int, j int);");

        mDatabase.execSQL("insert into test values(1,1);");

        // set cache size
@@ -292,9 +294,10 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
        ArrayList<String> sqlStrings = new ArrayList<String>();

        SQLiteStatement stmt0 = null;

        for (int i = 0; i < N+1; i++) {

            String s = "select * from test where i = " + i;

            String s = "select * from test where i = " + i + " and j = ?";

            sqlStrings.add(s);

            SQLiteStatement c = mDatabase.compileStatement(s);

            c.bindLong(1, 1);

            stmtObjs.add(i, c.getSqlStatementId());

            if (i == 0) {

                // save thie SQLiteStatement obj. we want to make sure it is thrown out of
@@ -307,6 +310,7 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
        assertEquals(0, stmt0.getSqlStatementId());

        for (int i = 1; i < N+1; i++) {

            SQLiteCompiledSql compSql = mDatabase.getCompiledStatementForSql(sqlStrings.get(i));

            assertNotNull(compSql);

            assertTrue(stmtObjs.contains(compSql.nStatement));

        }

    }
@@ -317,9 +321,11 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
                "num1 INTEGER, num2 INTEGER, image BLOB);");

        final String statement = "DELETE FROM test WHERE _id=?;";

        SQLiteStatement statementDoNotClose = mDatabase.compileStatement(statement);

        // SQl statement is compiled only at find bind or execute call

        assertTrue(statementDoNotClose.getSqlStatementId() == 0);

        statementDoNotClose.bindLong(1, 1);

        assertTrue(statementDoNotClose.getSqlStatementId() > 0);

        int nStatement = statementDoNotClose.getSqlStatementId();

        assertTrue(statementDoNotClose.getSqlStatementId() == nStatement);

        /* do not close statementDoNotClose object.

         * That should leave it in SQLiteDatabase.mPrograms.

         * mDatabase.close() in tearDown() should release it.
@@ -332,24 +338,26 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
     */

    @SmallTest

    public void testStatementClose() {

        mDatabase.execSQL("CREATE TABLE test (i int);");

        mDatabase.execSQL("CREATE TABLE test (i int, j 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();

            stmt = mDatabase.compileStatement("insert into test values(" + i + ", ?);");

            stmt.bindLong(1, 1);

            // keep track of 0th entry

            if (i == 0) {

                stmt0Id = stmt.getSqlStatementId();

            }

            stmt.executeInsert();

            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;");

        SQLiteStatement stmt1 = mDatabase.compileStatement("select * from test where i = ?;");

        stmt1.bindLong(1, 1);

        stmt1.close();



        // the above close() should have queuedUp the statement for finalization
@@ -369,7 +377,7 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
     */

    @LargeTest

    public void testStatementCloseDiffThread() throws InterruptedException {

        mDatabase.execSQL("CREATE TABLE test (i int);");

        mDatabase.execSQL("CREATE TABLE test (i int, j int);");

        // fill up statement cache in mDatabase in a thread

        Thread t1 = new Thread() {

            @Override public void run() {
@@ -377,12 +385,13 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
                mDatabase.setMaxSqlCacheSize(N);

                SQLiteStatement stmt;

                for (int i = 0; i < N; i ++) {

                    stmt = mDatabase.compileStatement("insert into test values(" + i + ");");

                    stmt.executeInsert();

                    stmt = mDatabase.compileStatement("insert into test values(" + i + ", ?);");

                    stmt.bindLong(1,1);

                    // keep track of 0th entry

                    if (i == 0) {

                        setStmt0Id(stmt.getSqlStatementId());

                    }

                    stmt.executeInsert();

                    stmt.close();

                }

            }
@@ -396,7 +405,8 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
        Thread t2 = new Thread() {

            @Override public void run() {

                SQLiteStatement stmt1 = mDatabase.compileStatement(

                        "select * from test where i = 1;");

                        "select * from test where i = ?;");

                stmt1.bindLong(1, 1);

                stmt1.close();

            }

        };
@@ -438,7 +448,7 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
     */

    @LargeTest

    public void testStatementCloseByDbClose() throws InterruptedException {

        mDatabase.execSQL("CREATE TABLE test (i int);");

        mDatabase.execSQL("CREATE TABLE test (i int, j int);");

        // fill up statement cache in mDatabase in a thread

        Thread t1 = new Thread() {

            @Override public void run() {
@@ -446,12 +456,13 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
                mDatabase.setMaxSqlCacheSize(N);

                SQLiteStatement stmt;

                for (int i = 0; i < N; i ++) {

                    stmt = mDatabase.compileStatement("insert into test values(" + i + ");");

                    stmt.executeInsert();

                    stmt = mDatabase.compileStatement("insert into test values(" + i + ", ?);");

                    stmt.bindLong(1, 1);

                    // keep track of 0th entry

                    if (i == 0) {

                        setStmt0Id(stmt.getSqlStatementId());

                    }

                    stmt.executeInsert();

                    stmt.close();

                }

            }
@@ -465,7 +476,8 @@ public class SQLiteDatabaseTest extends AndroidTestCase { 
        Thread t2 = new Thread() {

            @Override public void run() {

                SQLiteStatement stmt1 = mDatabase.compileStatement(

                        "select * from test where i = 1;");

                        "select * from test where i = ?;");

                stmt1.bindLong(1, 1);

                stmt1.close();

            }

        };

core/tests/coretests/src/android/database/sqlite/SQLiteStatementTest.java

+158 −62

File changed.

Preview size limit exceeded, changes collapsed.