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

Merge "Fix SQL crash and tests" into main

Browse Source
This commit is contained in:
Lee Shombert
2023-07-21 21:33:02 +00:00
committed by Android (Google) Code Review
parent ae67e9e87b 30fb30dfb5
commit f3032d18a7
4 changed files with 130 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
core/java/android/database/sqlite/SQLiteConnection.java
View File

@@ -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);
}
mPreparedStatementCache.resize(configuration.maxSqlCacheSize);
if (foreignKeyModeChanged) {
setForeignKeyModeFromConfiguration();
@@ -630,12 +625,12 @@ 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;
}
return mPreparedStatementCache.get(sql) != null;
}
/**
@@ -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) {
statement.mInUse = true;
return statement;
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.
}
} 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;
}
// 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);
}
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,9 +1153,7 @@ 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);
}
releasePreparedStatementLI(statement);
}
private void finalizePreparedStatement(PreparedStatement statement) {
@@ -1327,9 +1330,7 @@ public final class SQLiteConnection implements CancellationSignal.OnCancelListen
mRecentOperations.dump(printer);
if (verbose) {
synchronized (mCacheLock) {
mPreparedStatementCache.dump(printer);
}
mPreparedStatementCache.dump(printer);
}
}
@@ -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) {

11
core/java/android/database/sqlite/SQLiteConnectionPool.java
View File

@@ -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);
}
}
}

50
core/tests/coretests/src/android/database/DatabaseErrorHandlerTest.java
View File

@@ -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 (SQLiteException e) {
} catch (SQLiteDatabaseCorruptException e) {
// Expected result.
}
// 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();
@@ -122,4 +130,4 @@ public class DatabaseErrorHandlerTest extends AndroidTestCase {
}
}
}
}
}

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

@@ -914,6 +914,24 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
verifyLookasideStats(true);
}
void verifyLookasideStats(boolean expectDisabled) {
boolean dbStatFound = false;
SQLiteDebug.PagerStats info = SQLiteDebug.getDatabaseInfo();
for (SQLiteDebug.DbStats dbStat : info.dbStats) {
if (dbStat.dbName.endsWith(mDatabaseFile.getName()) && !dbStat.arePoolStats) {
dbStatFound = true;
Log.i(TAG, "Lookaside for " + dbStat.dbName + " " + dbStat.lookaside);
if (expectDisabled) {
assertTrue("lookaside slots count should be zero", dbStat.lookaside == 0);
} else {
assertTrue("lookaside slots count should be greater than zero",
dbStat.lookaside > 0);
}
}
}
assertTrue("No dbstat found for " + mDatabaseFile.getName(), dbStatFound);
}
@SmallTest
public void testOpenParamsSetLookasideConfigValidation() {
try {
@@ -930,24 +948,6 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
}
}
void verifyLookasideStats(boolean expectDisabled) {
boolean dbStatFound = false;
SQLiteDebug.PagerStats info = SQLiteDebug.getDatabaseInfo();
for (SQLiteDebug.DbStats dbStat : info.dbStats) {
if (dbStat.dbName.endsWith(mDatabaseFile.getName())) {
dbStatFound = true;
Log.i(TAG, "Lookaside for " + dbStat.dbName + " " + dbStat.lookaside);
if (expectDisabled) {
assertTrue("lookaside slots count should be zero", dbStat.lookaside == 0);
} else {
assertTrue("lookaside slots count should be greater than zero",
dbStat.lookaside > 0);
}
}
}
assertTrue("No dbstat found for " + mDatabaseFile.getName(), dbStatFound);
}
@LargeTest
public void testDefaultDatabaseErrorHandler() {
DefaultDatabaseErrorHandler errorHandler = new DefaultDatabaseErrorHandler();