Fix SQL crash and tests (30fb30df) · Commits · e / os / android_frameworks_base

core/java/android/database/sqlite/SQLiteConnection.java

+73 −36

Original line number	Diff line number	Diff line
		@@ -113,9 +113,6 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		private final boolean mIsReadOnlyConnection;
		private PreparedStatement mPreparedStatementPool;

		// A lock access to the statement cache.
		private final Object mCacheLock = new Object();
		@GuardedBy("mCacheLock")
		private final PreparedStatementCache mPreparedStatementCache;

		// The recent operations log.
		@@ -596,9 +593,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		mConfiguration.updateParametersFrom(configuration);

		// Update prepared statement cache size.
		synchronized (mCacheLock) {
		mPreparedStatementCache.resize(configuration.maxSqlCacheSize);
		}

		if (foreignKeyModeChanged) {
		setForeignKeyModeFromConfiguration();
		@@ -630,13 +625,13 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		mOnlyAllowReadOnlyOperations = readOnly;
		}

		// Called by SQLiteConnectionPool only.
		// Returns true if the prepared statement cache contains the specified SQL.
		// Called by SQLiteConnectionPool only to decide if this connection has the desired statement
		// already prepared. Returns true if the prepared statement cache contains the specified SQL.
		// The statement may be stale, but that will be a rare occurrence and affects performance only
		// a tiny bit, and only when database schema changes.
		boolean isPreparedStatementInCache(String sql) {
		synchronized (mCacheLock) {
		return mPreparedStatementCache.get(sql) != null;
		}
		}

		/**
		* Gets the unique id of this connection.
		@@ -1070,28 +1065,41 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		/**
		* Return a {@link #PreparedStatement}, possibly from the cache.
		*/
		@GuardedBy("mCacheLock")
		private PreparedStatement acquirePreparedStatementLI(String sql) {
		++mPool.mTotalPrepareStatements;
		PreparedStatement statement = mPreparedStatementCache.get(sql);
		PreparedStatement statement = mPreparedStatementCache.getStatement(sql);
		long seqNum = mPreparedStatementCache.getLastSeqNum();

		boolean skipCache = false;
		if (statement != null) {
		if (!statement.mInUse) {
		if (statement.mSeqNum == seqNum) {
		// This is a valid statement. Claim it and return it.
		statement.mInUse = true;
		return statement;
		} else {
		// This is a stale statement. Remove it from the cache. Treat this as if the
		// statement was never found, which means we should not skip the cache.
		mPreparedStatementCache.remove(sql);
		statement = null;
		// Leave skipCache == false.
		}
		// The statement is already in the cache but is in use (this statement appears
		// to be not only re-entrant but recursive!). So prepare a new copy of the
		// statement but do not cache it.
		} else {
		// The statement is already in the cache but is in use (this statement appears to
		// be not only re-entrant but recursive!). So prepare a new copy of the statement
		// but do not cache it.
		skipCache = true;
		}
		}
		++mPool.mTotalPrepareStatementCacheMiss;
		final long statementPtr = nativePrepareStatement(mConnectionPtr, sql);
		final long statementPtr = mPreparedStatementCache.createStatement(sql);
		seqNum = mPreparedStatementCache.getLastSeqNum();
		try {
		final int numParameters = nativeGetParameterCount(mConnectionPtr, statementPtr);
		final int type = DatabaseUtils.getSqlStatementType(sql);
		final boolean readOnly = nativeIsReadOnly(mConnectionPtr, statementPtr);
		statement = obtainPreparedStatement(sql, statementPtr, numParameters, type, readOnly);
		statement = obtainPreparedStatement(sql, statementPtr, numParameters, type, readOnly,
		seqNum);
		if (!skipCache && isCacheable(type)) {
		mPreparedStatementCache.put(sql, statement);
		statement.mInCache = true;
		@@ -1112,15 +1120,12 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		* Return a {@link #PreparedStatement}, possibly from the cache.
		*/
		PreparedStatement acquirePreparedStatement(String sql) {
		synchronized (mCacheLock) {
		return acquirePreparedStatementLI(sql);
		}
		}

		/**
		* Release a {@link #PreparedStatement} that was originally supplied by this connection.
		*/
		@GuardedBy("mCacheLock")
		private void releasePreparedStatementLI(PreparedStatement statement) {
		statement.mInUse = false;
		if (statement.mInCache) {
		@@ -1148,10 +1153,8 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		* Release a {@link #PreparedStatement} that was originally supplied by this connection.
		*/
		void releasePreparedStatement(PreparedStatement statement) {
		synchronized (mCacheLock) {
		releasePreparedStatementLI(statement);
		}
		}

		private void finalizePreparedStatement(PreparedStatement statement) {
		nativeFinalizeStatement(mConnectionPtr, statement.mStatementPtr);
		@@ -1327,11 +1330,9 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		mRecentOperations.dump(printer);

		if (verbose) {
		synchronized (mCacheLock) {
		mPreparedStatementCache.dump(printer);
		}
		}
		}

		/**
		* Describes the currently executing operation, in the case where the
		@@ -1430,7 +1431,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		}

		private PreparedStatement obtainPreparedStatement(String sql, long statementPtr,
		int numParameters, int type, boolean readOnly) {
		int numParameters, int type, boolean readOnly, long seqNum) {
		PreparedStatement statement = mPreparedStatementPool;
		if (statement != null) {
		mPreparedStatementPool = statement.mPoolNext;
		@@ -1444,6 +1445,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		statement.mNumParameters = numParameters;
		statement.mType = type;
		statement.mReadOnly = readOnly;
		statement.mSeqNum = seqNum;
		return statement;
		}

		@@ -1461,10 +1463,10 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		return sql.replaceAll("[\\s]\\n+[\\s]", " ");
		}

		void clearPreparedStatementCache() {
		synchronized (mCacheLock) {
		mPreparedStatementCache.evictAll();
		}
		// Update the database sequence number. This number is stored in the prepared statement
		// cache.
		void setDatabaseSeqNum(long n) {
		mPreparedStatementCache.setDatabaseSeqNum(n);
		}

		/**
		@@ -1502,6 +1504,10 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		// True if the statement is in the cache.
		public boolean mInCache;

		// The database schema ID at the time this statement was created. The ID is left zero for
		// statements that are not cached. This value is meaningful only if mInCache is true.
		public long mSeqNum;

		// True if the statement is in use (currently executing).
		// We need this flag because due to the use of custom functions in triggers, it's
		// possible for SQLite calls to be re-entrant. Consequently we need to prevent
		@@ -1510,10 +1516,41 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
		}

		private final class PreparedStatementCache extends LruCache<String, PreparedStatement> {
		// The database sequence number. This changes every time the database schema changes.
		private long mDatabaseSeqNum = 0;

		// The database sequence number from the last getStatement() or createStatement()
		// call. The proper use of this variable depends on the caller being single threaded.
		private long mLastSeqNum = 0;

		public PreparedStatementCache(int size) {
		super(size);
		}

		public synchronized void setDatabaseSeqNum(long n) {
		mDatabaseSeqNum = n;
		}

		// Return the last database sequence number.
		public long getLastSeqNum() {
		return mLastSeqNum;
		}

		// Return a statement from the cache. Save the database sequence number for the caller.
		public synchronized PreparedStatement getStatement(String sql) {
		mLastSeqNum = mDatabaseSeqNum;
		return get(sql);
		}

		// Return a new native prepared statement and save the database sequence number for the
		// caller. This does not modify the cache in any way. However, by being synchronized,
		// callers are guaranteed that the sequence number did not change across the native
		// preparation step.
		public synchronized long createStatement(String sql) {
		mLastSeqNum = mDatabaseSeqNum;
		return nativePrepareStatement(mConnectionPtr, sql);
		}

		@Override
		protected void entryRemoved(boolean evicted, String key,
		PreparedStatement oldValue, PreparedStatement newValue) {

core/java/android/database/sqlite/SQLiteConnectionPool.java

+10 −1

Original line number	Diff line number	Diff line
		@@ -111,6 +111,13 @@ public final class SQLiteConnectionPool implements Closeable {
		@GuardedBy("mLock")
		private IdleConnectionHandler mIdleConnectionHandler;

		// The database schema sequence number. This counter is incremented every time a schema
		// change is detected. Every prepared statement records its schema sequence when the
		// statement is created. The prepared statement is not put back in the cache if the sequence
		// number has changed. The counter starts at 1, which allows clients to use 0 as a
		// distinguished value.
		private long mDatabaseSeqNum = 1;

		// whole execution time for this connection in milliseconds.
		private final AtomicLong mTotalStatementsTime = new AtomicLong(0);

		@@ -1127,10 +1134,12 @@ public final class SQLiteConnectionPool implements Closeable {
		}

		void clearAcquiredConnectionsPreparedStatementCache() {
		// Invalidate prepared statements that have an earlier schema sequence number.
		synchronized (mLock) {
		mDatabaseSeqNum++;
		if (!mAcquiredConnections.isEmpty()) {
		for (SQLiteConnection connection : mAcquiredConnections.keySet()) {
		connection.clearPreparedStatementCache();
		connection.setDatabaseSeqNum(mDatabaseSeqNum);
		}
		}
		}

core/tests/coretests/src/android/database/DatabaseErrorHandlerTest.java

+29 −21

Original line number	Diff line number	Diff line
		@@ -19,10 +19,13 @@ package android.database;
		import android.content.Context;
		import android.database.sqlite.SQLiteDatabase;
		import android.database.sqlite.SQLiteDiskIOException;
		import android.database.sqlite.SQLiteDatabaseCorruptException;

		import android.database.sqlite.SQLiteException;
		import android.test.AndroidTestCase;
		import android.util.Log;

		import java.util.concurrent.atomic.AtomicBoolean;
		import java.io.BufferedWriter;
		import java.io.File;
		import java.io.FileWriter;
		@@ -61,7 +64,6 @@ public class DatabaseErrorHandlerTest extends AndroidTestCase {
		assertTrue(mDatabaseFile.exists());
		}


		public void testDatabaseIsCorrupt() throws IOException {
		mDatabase.execSQL("create table t (i int);");
		// write junk into the database file
		@@ -70,30 +72,23 @@ public class DatabaseErrorHandlerTest extends AndroidTestCase {
		writer.close();
		assertTrue(mDatabaseFile.exists());
		// since the database file is now corrupt, doing any sql on this database connection
		// should trigger call to MyDatabaseCorruptionHandler.onCorruption
		// should trigger call to MyDatabaseCorruptionHandler.onCorruption. A corruption
		// exception will also be throws. This seems redundant.
		try {
		mDatabase.execSQL("select * from t;");
		fail("expected exception");
		} catch (SQLiteDiskIOException e) {
		/**
		* this test used to produce a corrupted db. but with new sqlite it instead reports
		* Disk I/O error. meh..
		* need to figure out how to cause corruption in db
		*/
		// expected
		if (mDatabaseFile.exists()) {
		mDatabaseFile.delete();
		} catch (SQLiteDatabaseCorruptException e) {
		// Expected result.
		}
		} catch (SQLiteException e) {

		}
		// database file should be gone
		// The database file should be gone.
		assertFalse(mDatabaseFile.exists());
		// after corruption handler is called, the database file should be free of
		// database corruption
		SQLiteDatabase db = SQLiteDatabase.openOrCreateDatabase(mDatabaseFile.getPath(), null,
		// After corruption handler is called, the database file should be free of
		// database corruption. Reopen it.
		mDatabase = SQLiteDatabase.openOrCreateDatabase(mDatabaseFile.getPath(), null,
		new MyDatabaseCorruptionHandler());
		assertTrue(db.isDatabaseIntegrityOk());
		assertTrue(mDatabase.isDatabaseIntegrityOk());
		// The teadDown() routine will close the database.
		}

		/**
		@@ -102,8 +97,21 @@ public class DatabaseErrorHandlerTest extends AndroidTestCase {
		* corrupt before deleting the file.
		*/
		public class MyDatabaseCorruptionHandler implements DatabaseErrorHandler {
		private final AtomicBoolean mEntered = new AtomicBoolean(false);
		public void onCorruption(SQLiteDatabase dbObj) {
		boolean databaseOk = dbObj.isDatabaseIntegrityOk();
		boolean databaseOk = false;
		if (!mEntered.get()) {
		// The integrity check can retrigger the corruption handler if the database is,
		// indeed, corrupted. Use mEntered to detect recursion and to skip retrying the
		// integrity check on recursion.
		mEntered.set(true);
		databaseOk = dbObj.isDatabaseIntegrityOk();
		}
		// At this point the database state has been detected and there is no further danger
		// of recursion. Setting mEntered to false allows this object to be reused, although
		// it is not obvious how such reuse would work.
		mEntered.set(false);

		// close the database
		try {
		dbObj.close();

core/tests/coretests/src/android/database/DatabaseGeneralTest.java

+17 −17

Original line number	Diff line number	Diff line
		@@ -914,27 +914,11 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
		verifyLookasideStats(true);
		}

		@SmallTest
		public void testOpenParamsSetLookasideConfigValidation() {
		try {
		SQLiteDatabase.OpenParams params = new SQLiteDatabase.OpenParams.Builder()
		.setLookasideConfig(-1, 0).build();
		fail("Negative slot size should be rejected");
		} catch (IllegalArgumentException expected) {
		}
		try {
		SQLiteDatabase.OpenParams params = new SQLiteDatabase.OpenParams.Builder()
		.setLookasideConfig(0, -10).build();
		fail("Negative slot count should be rejected");
		} catch (IllegalArgumentException expected) {
		}
		}

		void verifyLookasideStats(boolean expectDisabled) {
		boolean dbStatFound = false;
		SQLiteDebug.PagerStats info = SQLiteDebug.getDatabaseInfo();
		for (SQLiteDebug.DbStats dbStat : info.dbStats) {
		if (dbStat.dbName.endsWith(mDatabaseFile.getName())) {
		if (dbStat.dbName.endsWith(mDatabaseFile.getName()) && !dbStat.arePoolStats) {
		dbStatFound = true;
		Log.i(TAG, "Lookaside for " + dbStat.dbName + " " + dbStat.lookaside);
		if (expectDisabled) {
		@@ -948,6 +932,22 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
		assertTrue("No dbstat found for " + mDatabaseFile.getName(), dbStatFound);
		}

		@SmallTest
		public void testOpenParamsSetLookasideConfigValidation() {
		try {
		SQLiteDatabase.OpenParams params = new SQLiteDatabase.OpenParams.Builder()
		.setLookasideConfig(-1, 0).build();
		fail("Negative slot size should be rejected");
		} catch (IllegalArgumentException expected) {
		}
		try {
		SQLiteDatabase.OpenParams params = new SQLiteDatabase.OpenParams.Builder()
		.setLookasideConfig(0, -10).build();
		fail("Negative slot count should be rejected");
		} catch (IllegalArgumentException expected) {
		}
		}

		@LargeTest
		public void testDefaultDatabaseErrorHandler() {
		DefaultDatabaseErrorHandler errorHandler = new DefaultDatabaseErrorHandler();