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

Commit 5e89ae29 authored by Vasu Nori's avatar Vasu Nori
Browse files

fix compiled statement bugs, synchronization bugs

1. when LRU cache wants to remove the eldest entry, it may be finalizing
a statement that is still in use
2. a couple of synchronization issues.
3. a bit code refactoring in SQLiteCompiledSql
4. remove a bunch of unsed debug code from SQLiteDatabase

Change-Id: I45454dc2dbadd7d8842ba77dd2b0e9ff138ec6f4
parent e216dac4
Loading
Loading
Loading
Loading
+25 −36
Original line number Diff line number Diff line
@@ -31,12 +31,12 @@ import android.util.Log;
    private static final String TAG = "SQLiteCompiledSql";

    /** The database this program is compiled against. */
    /* package */ SQLiteDatabase mDatabase;
    /* package */ final SQLiteDatabase mDatabase;

    /**
     * Native linkage, do not modify. This comes from the database.
     */
    /* package */ int nHandle = 0;
    /* package */ final int nHandle;

    /**
     * Native linkage, do not modify. When non-0 this holds a reference to a valid
@@ -54,39 +54,14 @@ import android.util.Log;
    private boolean mInUse = false;

    /* package */ SQLiteCompiledSql(SQLiteDatabase db, String sql) {
        if (!db.isOpen()) {
            throw new IllegalStateException("database " + db.getPath() + " already closed");
        }
        db.verifyDbIsOpen();
        db.verifyLockOwner();
        mDatabase = db;
        mSqlStmt = sql;
        mStackTrace = new DatabaseObjectNotClosedException().fillInStackTrace();
        this.nHandle = db.mNativeHandle;
        compile(sql, true);
    }

    /**
     * Compiles the given SQL into a SQLite byte code program using sqlite3_prepare_v2(). If
     * this method has been called previously without a call to close and forCompilation is set
     * to false the previous compilation will be used. Setting forceCompilation to true will
     * always re-compile the program and should be done if you pass differing SQL strings to this
     * method.
     *
     * <P>Note: this method acquires the database lock.</P>
     *
     * @param sql the SQL string to compile
     * @param forceCompilation forces the SQL to be recompiled in the event that there is an
     *  existing compiled SQL program already around
     */
    private void compile(String sql, boolean forceCompilation) {
        mDatabase.verifyLockOwner();
        // Only compile if we don't have a valid statement already or the caller has
        // explicitly requested a recompile.
        if (forceCompilation) {
            // Note that the native_compile() takes care of destroying any previously
            // existing programs before it compiles.
        nHandle = db.mNativeHandle;
        native_compile(sql);
    }
    }

    /* package */ void releaseSqlStatement() {
        // Note that native_finalize() checks to make sure that nStatement is
@@ -102,7 +77,7 @@ import android.util.Log;
     */
    /* package */ synchronized boolean acquire() {
        if (mInUse) {
            // someone already has acquired it.
            // it is already in use.
            return false;
        }
        mInUse = true;
@@ -117,6 +92,13 @@ import android.util.Log;
        return mInUse;
    }

    /* package */ synchronized void releaseIfNotInUse() {
        // if it is not in use, release its memory from the database
        if (!isInUse()) {
            releaseSqlStatement();
        }
    }

    /**
     * Make sure that the native resource is cleaned up.
     */
@@ -125,10 +107,15 @@ import android.util.Log;
        try {
            if (nStatement == 0) return;
            // finalizer should NEVER get called
            // but if the database itself is not closed and is GC'ed, then
            // all sub-objects attached to the database could end up getting GC'ed too.
            // in that case, don't print any warning.
            if (!mInUse) {
                int len = mSqlStmt.length();
                Log.w(TAG, "Releasing statement in a finalizer. Please ensure " +
                        "that you explicitly call close() on your cursor: " +
                        mSqlStmt.substring(0, (len > 100) ? 100 : len), mStackTrace);
            }
            releaseSqlStatement();
        } finally {
            super.finalize();
@@ -140,6 +127,8 @@ import android.util.Log;
            StringBuilder buff = new StringBuilder();
            buff.append(" nStatement=");
            buff.append(nStatement);
            buff.append(", mInUse=");
            buff.append(mInUse);
            buff.append(", db=");
            buff.append(mDatabase.getPath());
            buff.append(", db_connectionNum=");
+35 −42
Original line number Diff line number Diff line
@@ -293,11 +293,7 @@ public class SQLiteDatabase extends SQLiteClosable {
                    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();
                }
                eldest.getValue().releaseIfNotInUse();
                // return true, so that this entry is removed automatically by the caller.
                return true;
            }
@@ -308,8 +304,7 @@ public class SQLiteDatabase extends SQLiteClosable {
     * SQL statement & schema.
     */
    public static final int MAX_SQL_CACHE_SIZE = 100;
    private int mCacheFullWarnings;
    private static final int MAX_WARNINGS_ON_CACHESIZE_CONDITION = 1;
    private boolean mCacheFullWarning;

    /** maintain stats about number of cache hits and misses */
    private int mNumCacheHits;
@@ -2092,12 +2087,6 @@ public class SQLiteDatabase extends SQLiteClosable {
        }
    }

    /*
     * ============================================================================
     *
     *       The following methods deal with compiled-sql cache
     * ============================================================================
     */
    /**
     * Adds the given SQL and its compiled-statement-id-returned-by-sqlite to the
     * cache of compiledQueries attached to 'this'.
@@ -2113,26 +2102,21 @@ public class SQLiteDatabase extends SQLiteClosable {
                return;
            }

            if (mCompiledQueries.size() == mMaxSqlCacheSize) {
            if (!isCacheFullWarningLogged() && mCompiledQueries.size() == mMaxSqlCacheSize) {
                /*
                 * cache size of {@link #mMaxSqlCacheSize} is not enough for this app.
                 * log a warning.
                 * chances are it is NOT using ? for bindargs - or cachesize is too small.
                 */
                if (++mCacheFullWarnings == MAX_WARNINGS_ON_CACHESIZE_CONDITION) {
                Log.w(TAG, "Reached MAX size for compiled-sql statement cache for database " +
                        getPath() + ". Use setMaxSqlCacheSize() to increase cachesize. ");
                }
                setCacheFullWarningLogged();
            } 
            /* add the given SQLiteCompiledSql compiledStatement to cache.
             * no need to worry about the cache size - because {@link #mCompiledQueries}
             * self-limits its size to {@link #mMaxSqlCacheSize}.
             */
            mCompiledQueries.put(sql, compiledStatement);
            if (SQLiteDebug.DEBUG_SQL_CACHE) {
                Log.v(TAG, "|adding_sql_to_cache|" + getPath() + "|" +
                        mCompiledQueries.size() + "|" + sql);
            }
        }
    }

@@ -2150,24 +2134,13 @@ public class SQLiteDatabase extends SQLiteClosable {
     * From the compiledQueries cache, returns the compiled-statement-id for the given SQL.
     * Returns null, if not found in the cache.
     */
    /* package */ SQLiteCompiledSql getCompiledStatementForSql(String sql) {
        SQLiteCompiledSql compiledStatement = null;
        boolean cacheHit;
        synchronized(mCompiledQueries) {
            cacheHit = (compiledStatement = mCompiledQueries.get(sql)) != null;
        }
        if (cacheHit) {
            mNumCacheHits++;
        } else {
    /* package */ synchronized SQLiteCompiledSql getCompiledStatementForSql(String sql) {
        SQLiteCompiledSql compiledStatement = mCompiledQueries.get(sql);
        if (compiledStatement == null) {
            mNumCacheMisses++;
            return null;
        }

        if (SQLiteDebug.DEBUG_SQL_CACHE) {
            Log.v(TAG, "|cache_stats|" +
                    getPath() + "|" + mCompiledQueries.size() +
                    "|" + mNumCacheHits + "|" + mNumCacheMisses +
                    "|" + cacheHit + "|" + sql);
        }
        mNumCacheHits++;
        return compiledStatement;
    }

@@ -2203,6 +2176,26 @@ public class SQLiteDatabase extends SQLiteClosable {
        }
    }

    private synchronized boolean isCacheFullWarningLogged() {
        return mCacheFullWarning;
    }

    private synchronized void setCacheFullWarningLogged() {
        mCacheFullWarning = true;
    }

    private synchronized int getCacheHitNum() {
        return mNumCacheHits;
    }

    private synchronized int getCacheMissNum() {
        return mNumCacheMisses;
    }

    private synchronized int getCachesize() {
        return mCompiledQueries.size();
    }

    /* package */ void finalizeStatementLater(int id) {
        if (!isOpen()) {
            // database already closed. this statement will already have been finalized.
@@ -2484,16 +2477,16 @@ public class SQLiteDatabase extends SQLiteClosable {
                        }
                        if (pageCount > 0) {
                            dbStatsList.add(new DbStats(dbName, pageCount, db.getPageSize(),
                                    lookasideUsed, db.mNumCacheHits, db.mNumCacheMisses,
                                    db.mCompiledQueries.size()));
                                    lookasideUsed, db.getCacheHitNum(), db.getCacheMissNum(),
                                    db.getCachesize()));
                        }
                    }
                    // if there are pooled connections, return the cache stats for them also.
                    if (db.mConnectionPool != null) {
                        for (SQLiteDatabase pDb : db.mConnectionPool.getConnectionList()) {
                            dbStatsList.add(new DbStats("(pooled # " + pDb.mConnectionNum + ") "
                                    + lastnode, 0, 0, 0, pDb.mNumCacheHits, pDb.mNumCacheMisses,
                                    pDb.mCompiledQueries.size()));
                                    + lastnode, 0, 0, 0, pDb.getCacheHitNum(),
                                    pDb.getCacheMissNum(), pDb.getCachesize()));
                        }
                    }
                } catch (SQLiteException e) {