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

Commit d606b4bf authored by Vasu Nori's avatar Vasu Nori
Browse files

in finalize() methods, log warnings if db lock is going to be held.

this is to track bug:2463988 and bug:2457342
parent a834949a
Loading
Loading
Loading
Loading
+6 −17
Original line number Diff line number Diff line
@@ -62,9 +62,8 @@ public class SQLiteCursor extends AbstractWindowedCursor {
    /** A mapping of column names to column indices, to speed up lookups */
    private Map<String, Integer> mColumnNameMap;

    /** Used to find out where a cursor was allocated in case it never got
     * released. */
    private StackTraceElement[] mStackTraceElements;
    /** Used to find out where a cursor was allocated in case it never got released. */
    private Throwable mStackTrace;
    
    /** 
     *  mMaxRead is the max items that each cursor window reads 
@@ -208,11 +207,7 @@ public class SQLiteCursor extends AbstractWindowedCursor {
            String editTable, SQLiteQuery query) {
        // The AbstractCursor constructor needs to do some setup.
        super();

        if (SQLiteDebug.DEBUG_ACTIVE_CURSOR_FINALIZATION) {
            mStackTraceElements = new Exception().getStackTrace();
        }

        mStackTrace = new Exception().fillInStackTrace();
        mDatabase = db;
        mDriver = driver;
        mEditTable = editTable;
@@ -585,15 +580,9 @@ public class SQLiteCursor extends AbstractWindowedCursor {
            // if the cursor hasn't been closed yet, close it first
            if (mWindow != null) {
                close();
                Log.e(TAG, "Finalizing cursor that has not been deactivated or closed." 
                    + " database = " + mDatabase.getPath() + ", table = " + mEditTable
                    + ", query = " + mQuery.mSql);
                if (SQLiteDebug.DEBUG_ACTIVE_CURSOR_FINALIZATION) {
                    Log.d(TAG, "This cursor was created in:");
                    for (StackTraceElement ste : mStackTraceElements) {
                        Log.d(TAG, "      " + ste);
                    }
                }
                Log.e(TAG, "Finalizing a Cursor that has not been deactivated or closed. " +
                        "database = " + mDatabase.getPath() + ", table = " + mEditTable +
                        ", query = " + mQuery.mSql, mStackTrace);
                SQLiteDebug.notifyActiveCursorFinalized();
            } else {
                if (Config.LOGV) {
+5 −25
Original line number Diff line number Diff line
@@ -271,7 +271,8 @@ public class SQLiteDatabase extends SQLiteClosable {
    private String mTimeOpened = null;
    private String mTimeClosed = null;

    private final RuntimeException mLeakedException;
    /** Used to find out where this object was created in case it never got closed. */
    private Throwable mStackTrace = null;

    // System property that enables logging of slow queries. Specify the threshold in ms.
    private static final String LOG_SLOW_QUERIES_PROPERTY = "db.log.slow_query_threshold";
@@ -1726,13 +1727,8 @@ public class SQLiteDatabase extends SQLiteClosable {
    @Override
    protected void finalize() {
        if (isOpen()) {
            if (mPrograms.isEmpty()) {
                Log.e(TAG, "Leak found", mLeakedException);
            } else {
                IllegalStateException leakProgram = new IllegalStateException(
                        "mPrograms size " + mPrograms.size(), mLeakedException);
                Log.e(TAG, "Leak found", leakProgram);
            }
            Log.e(TAG, "close() was never explicitly called on database '" +
                    mPath + "' ", mStackTrace);
            closeClosable();
            onAllReferencesReleased();
        }
@@ -1753,9 +1749,7 @@ public class SQLiteDatabase extends SQLiteClosable {
        mFlags = flags;
        mPath = path;
        mSlowQueryThreshold = SystemProperties.getInt(LOG_SLOW_QUERIES_PROPERTY, -1);

        mLeakedException = new IllegalStateException(path +
            " SQLiteDatabase created and never closed");
        mStackTrace = new Exception().fillInStackTrace();
        mFactory = factory;
        dbopen(mPath, mFlags);
        if (SQLiteDebug.DEBUG_SQL_CACHE) {
@@ -1908,14 +1902,6 @@ public class SQLiteDatabase extends SQLiteClosable {
            return;
        }

        /* don't cache PRAGMA sql statements.
         * caching them makes sqlite return incorrect results on pragma sql execution!
         */
        String prefixSql = sql.substring(0, 6);
        if (prefixSql.toLowerCase().startsWith("pragma")) {
            return;
        }

        SQLiteCompiledSql compiledSql = null;
        synchronized(mCompiledQueries) {
            // don't insert the new mapping if a mapping already exists
@@ -1981,12 +1967,6 @@ public class SQLiteDatabase extends SQLiteClosable {
     * returns null, if not found in the cache.
     */
    /* package */ SQLiteCompiledSql getCompiledStatementForSql(String sql) {
        // don't look for PRAGMA sql statements in compiled-sql cache
        String prefixSql = sql.substring(0, 6);
        if (prefixSql.toLowerCase().startsWith("pragma")) {
            return null;
        }

        SQLiteCompiledSql compiledStatement = null;
        boolean cacheHit;
        synchronized(mCompiledQueries) {
+0 −7
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package android.database.sqlite;

import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase.CursorFactory;
import android.util.Log;

/**
 * A cursor driver that uses the given query directly.
@@ -26,7 +25,6 @@ import android.util.Log;
 * @hide
 */
public class SQLiteDirectCursorDriver implements SQLiteCursorDriver {
    private static String TAG = "SQLiteDirectCursorDriver";
    private String mEditTable; 
    private SQLiteDatabase mDatabase;
    private Cursor mCursor;
@@ -36,11 +34,6 @@ public class SQLiteDirectCursorDriver implements SQLiteCursorDriver {
    public SQLiteDirectCursorDriver(SQLiteDatabase db, String sql, String editTable) {
        mDatabase = db;
        mEditTable = editTable;
        //TODO remove all callers that end in ; and remove this check
        if (sql.charAt(sql.length() - 1) == ';') {
            Log.w(TAG, "Found SQL string that ends in ; -- " + sql);
            sql = sql.substring(0, sql.length() - 1);
        }
        mSql = sql;
    }

+17 −3
Original line number Diff line number Diff line
@@ -46,11 +46,23 @@ public abstract class SQLiteProgram extends SQLiteClosable {

    /* package */ SQLiteProgram(SQLiteDatabase db, String sql) {
        mDatabase = db;
        mSql = sql;
        mSql = sql.trim();
        db.acquireReference();
        db.addSQLiteClosable(this);
        this.nHandle = db.mNativeHandle;

        // shouldn't reuse compiled-plans of PRAGMA sql statements
        // because sqlite returns OLD values if compiled-pragma-statements are reused
        //TODO: remove this code when sqlite fixes it (and add tests too)
        String prefixSql = mSql.substring(0, 6);
        if (prefixSql.toLowerCase().startsWith("pragma")) {
            mCompiledSql = new SQLiteCompiledSql(db, sql);
            nStatement = mCompiledSql.nStatement;
            // since it is not in the cache, no need to acquire() it.
            return;
        }

        // it is not pragma
        mCompiledSql = db.getCompiledStatementForSql(sql);
        if (mCompiledSql == null) {
            // create a new compiled-sql obj
@@ -70,6 +82,8 @@ public abstract class SQLiteProgram extends SQLiteClosable {
                // CompiledSql object. create a new one.
                // finalize it when I am done with it in "this" object.
                mCompiledSql = new SQLiteCompiledSql(db, sql);

                // since it is not in the cache, no need to acquire() it.
            }
        }
        nStatement = mCompiledSql.nStatement;
@@ -95,9 +109,9 @@ public abstract class SQLiteProgram extends SQLiteClosable {
        synchronized(mDatabase.mCompiledQueries) {
            if (!mDatabase.mCompiledQueries.containsValue(mCompiledSql)) {
                // it is NOT in compiled-sql cache. i.e., responsibility of
                // release this statement is on me.
                // releasing this statement is on me.
                mCompiledSql.releaseSqlStatement();
                mCompiledSql = null; // so that GC doesn't call finalize() on it
                mCompiledSql = null;
                nStatement = 0;
            } else {
                // it is in compiled-sql cache. reset its CompiledSql#mInUse flag
+20 −0
Original line number Diff line number Diff line
@@ -1009,4 +1009,24 @@ public class DatabaseGeneralTest extends AndroidTestCase implements PerformanceT
         * mDatabase.close() in tearDown() should release it.
         */
    }

    @MediumTest
    public void testSemicolonsInStatements() throws Exception {
        mDatabase.execSQL("CREATE TABLE pragma_test (" +
                "i INTEGER DEFAULT 1234, " +
                "j INTEGER, " +
                "s TEXT DEFAULT 'hello', " +
                "t TEXT, " +
                "'select' TEXT DEFAULT \"hello\")");
        try {
            // ending the sql statement with  semicolons shouldn't be a problem.
            Cursor cur = mDatabase.rawQuery("PRAGMA database_list;", null);
            cur.close();
            // two semicolons in the statement shouldn't be a problem.
            cur = mDatabase.rawQuery("PRAGMA database_list;;", null);
            cur.close();
        } catch (Throwable t) {
            fail("unexpected, of course");
        }
    }
}