diff options
| author | Jeff Brown <jeffbrown@google.com> | 2011-10-07 13:29:37 -0700 |
|---|---|---|
| committer | Jeff Brown <jeffbrown@google.com> | 2011-10-07 13:31:00 -0700 |
| commit | 7ce745248d4de0e6543a559c93423df899832100 (patch) | |
| tree | 4cae60d6cc5a00379bbea921155579c3c4c3a31e /core/java/android/database/sqlite | |
| parent | d0ff68da6a606602235fb8749473999e3d1bde53 (diff) | |
Clean up CursorWindow lifetime.
Bug: 5332296
Removed dead code in SQLiteCursor related to the use of a background
query thread. This code could result in CursorWindows being modified
concurrently or used after free. This code is broken, unused and
is just in the way.
Added comments to explain how CursorWindow ownership is
supposed to work for AbstractWindowedCursors. (There are still cases
where cursor windows get dropped on the floor without being closed.
Those will be taken care of in a subsequent patch.)
Cleaned up SQLiteQuery.fillWindow to eliminate duplicate code and
remove bits that were only needed for background loading, like
returning -1.
Change-Id: I03e8e2e73ff0c11df76d63f57df4c5ada06ae1cb
Diffstat (limited to 'core/java/android/database/sqlite')
| -rw-r--r-- | core/java/android/database/sqlite/SQLiteCursor.java | 194 | ||||
| -rw-r--r-- | core/java/android/database/sqlite/SQLiteDatabase.java | 26 | ||||
| -rw-r--r-- | core/java/android/database/sqlite/SQLiteQuery.java | 30 |
3 files changed, 22 insertions, 228 deletions
diff --git a/core/java/android/database/sqlite/SQLiteCursor.java b/core/java/android/database/sqlite/SQLiteCursor.java index ea9346d93fbb..81fe8241106a 100644 --- a/core/java/android/database/sqlite/SQLiteCursor.java +++ b/core/java/android/database/sqlite/SQLiteCursor.java @@ -18,16 +18,11 @@ package android.database.sqlite; import android.database.AbstractWindowedCursor; import android.database.CursorWindow; -import android.database.DataSetObserver; -import android.os.Handler; -import android.os.Message; -import android.os.Process; import android.os.StrictMode; import android.util.Log; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.locks.ReentrantLock; /** * A Cursor implementation that exposes results from a query on a @@ -60,140 +55,8 @@ public class SQLiteCursor extends AbstractWindowedCursor { /** Used to find out where a cursor was allocated in case it never got released. */ private final Throwable mStackTrace; - - /** - * mMaxRead is the max items that each cursor window reads - * default to a very high value - */ - private int mMaxRead = Integer.MAX_VALUE; - private int mInitialRead = Integer.MAX_VALUE; - private int mCursorState = 0; - private ReentrantLock mLock = null; - private boolean mPendingData = false; /** - * support for a cursor variant that doesn't always read all results - * initialRead is the initial number of items that cursor window reads - * if query contains more than this number of items, a thread will be - * created and handle the left over items so that caller can show - * results as soon as possible - * @param initialRead initial number of items that cursor read - * @param maxRead leftover items read at maxRead items per time - * @hide - */ - public void setLoadStyle(int initialRead, int maxRead) { - mMaxRead = maxRead; - mInitialRead = initialRead; - mLock = new ReentrantLock(true); - } - - private void queryThreadLock() { - if (mLock != null) { - mLock.lock(); - } - } - - private void queryThreadUnlock() { - if (mLock != null) { - mLock.unlock(); - } - } - - - /** - * @hide - */ - final private class QueryThread implements Runnable { - private final int mThreadState; - QueryThread(int version) { - mThreadState = version; - } - private void sendMessage() { - if (mNotificationHandler != null) { - mNotificationHandler.sendEmptyMessage(1); - mPendingData = false; - } else { - mPendingData = true; - } - - } - public void run() { - // use cached mWindow, to avoid get null mWindow - CursorWindow cw = mWindow; - Process.setThreadPriority(Process.myTid(), Process.THREAD_PRIORITY_BACKGROUND); - // the cursor's state doesn't change - while (true) { - mLock.lock(); - try { - if (mCursorState != mThreadState) { - break; - } - - int count = getQuery().fillWindow(cw, mMaxRead, mCount); - // return -1 means there is still more data to be retrieved from the resultset - if (count != 0) { - if (count == NO_COUNT){ - mCount += mMaxRead; - if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "received -1 from native_fill_window. read " + - mCount + " rows so far"); - } - sendMessage(); - } else { - mCount += count; - if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "received all data from native_fill_window. read " + - mCount + " rows."); - } - sendMessage(); - break; - } - } else { - break; - } - } catch (Exception e) { - // end the tread when the cursor is close - break; - } finally { - mLock.unlock(); - } - } - } - } - - /** - * @hide - */ - protected class MainThreadNotificationHandler extends Handler { - public void handleMessage(Message msg) { - notifyDataSetChange(); - } - } - - /** - * @hide - */ - protected MainThreadNotificationHandler mNotificationHandler; - - public void registerDataSetObserver(DataSetObserver observer) { - super.registerDataSetObserver(observer); - if ((Integer.MAX_VALUE != mMaxRead || Integer.MAX_VALUE != mInitialRead) && - mNotificationHandler == null) { - queryThreadLock(); - try { - mNotificationHandler = new MainThreadNotificationHandler(); - if (mPendingData) { - notifyDataSetChange(); - mPendingData = false; - } - } finally { - queryThreadUnlock(); - } - } - - } - - /** * Execute a query and provide access to its result set through a Cursor * interface. For a query such as: {@code SELECT name, birth, phone FROM * myTable WHERE ... LIMIT 1,20 ORDER BY...} the column names (name, birth, @@ -293,36 +156,23 @@ public class SQLiteCursor extends AbstractWindowedCursor { return mCount; } - private void fillWindow (int startPos) { + private void fillWindow(int startPos) { if (mWindow == null) { // If there isn't a window set already it will only be accessed locally mWindow = new CursorWindow(true /* the window is local only */); } else { - mCursorState++; - queryThreadLock(); - try { - mWindow.clear(); - } finally { - queryThreadUnlock(); - } + mWindow.clear(); } mWindow.setStartPosition(startPos); - int count = getQuery().fillWindow(mWindow, mInitialRead, 0); - // return -1 means there is still more data to be retrieved from the resultset - if (count == NO_COUNT){ - mCount = startPos + mInitialRead; - if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "received -1 from native_fill_window. read " + mCount + " rows so far"); - } - Thread t = new Thread(new QueryThread(mCursorState), "query thread"); - t.start(); - } else if (startPos == 0) { // native_fill_window returns count(*) only for startPos = 0 + int count = getQuery().fillWindow(mWindow); + if (startPos == 0) { // fillWindow returns count(*) only for startPos = 0 if (Log.isLoggable(TAG, Log.DEBUG)) { Log.d(TAG, "received count(*) from native_fill_window: " + count); } mCount = count; } else if (mCount <= 0) { - throw new IllegalStateException("count should never be non-zero negative number"); + throw new IllegalStateException("Row count should never be zero or negative " + + "when the start position is non-zero"); } } @@ -366,11 +216,7 @@ public class SQLiteCursor extends AbstractWindowedCursor { private void deactivateCommon() { if (false) Log.v(TAG, "<<< Releasing cursor " + this); - mCursorState = 0; - if (mWindow != null) { - mWindow.close(); - mWindow = null; - } + closeWindow(); if (false) Log.v("DatabaseWindow", "closing window in release()"); } @@ -439,16 +285,12 @@ public class SQLiteCursor extends AbstractWindowedCursor { // This one will recreate the temp table, and get its count mDriver.cursorRequeried(this); mCount = NO_COUNT; - mCursorState++; - queryThreadLock(); try { mQuery.requery(); } catch (IllegalStateException e) { // for backwards compatibility, just return false Log.w(TAG, "requery() failed " + e.getMessage(), e); return false; - } finally { - queryThreadUnlock(); } } @@ -472,18 +314,9 @@ public class SQLiteCursor extends AbstractWindowedCursor { } @Override - public void setWindow(CursorWindow window) { - if (mWindow != null) { - mCursorState++; - queryThreadLock(); - try { - mWindow.close(); - } finally { - queryThreadUnlock(); - } - mCount = NO_COUNT; - } - mWindow = window; + public void setWindow(CursorWindow window) { + super.setWindow(window); + mCount = NO_COUNT; } /** @@ -521,11 +354,4 @@ public class SQLiteCursor extends AbstractWindowedCursor { super.finalize(); } } - - /** - * this is only for testing purposes. - */ - /* package */ int getMCount() { - return mCount; - } } diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java index 93a6ad3f55c1..896a75037e8f 100644 --- a/core/java/android/database/sqlite/SQLiteDatabase.java +++ b/core/java/android/database/sqlite/SQLiteDatabase.java @@ -1593,32 +1593,6 @@ public class SQLiteDatabase extends SQLiteClosable { } /** - * Runs the provided SQL and returns a cursor over the result set. - * The cursor will read an initial set of rows and the return to the caller. - * It will continue to read in batches and send data changed notifications - * when the later batches are ready. - * @param sql the SQL query. The SQL string must not be ; terminated - * @param selectionArgs You may include ?s in where clause in the query, - * which will be replaced by the values from selectionArgs. The - * values will be bound as Strings. - * @param initialRead set the initial count of items to read from the cursor - * @param maxRead set the count of items to read on each iteration after the first - * @return A {@link Cursor} object, which is positioned before the first entry. Note that - * {@link Cursor}s are not synchronized, see the documentation for more details. - * - * This work is incomplete and not fully tested or reviewed, so currently - * hidden. - * @hide - */ - public Cursor rawQuery(String sql, String[] selectionArgs, - int initialRead, int maxRead) { - SQLiteCursor c = (SQLiteCursor)rawQueryWithFactory( - null, sql, selectionArgs, null); - c.setLoadStyle(initialRead, maxRead); - return c; - } - - /** * Convenience method for inserting a row into the database. * * @param table the table to insert the row into diff --git a/core/java/android/database/sqlite/SQLiteQuery.java b/core/java/android/database/sqlite/SQLiteQuery.java index 06a41b28504c..7db0914140b2 100644 --- a/core/java/android/database/sqlite/SQLiteQuery.java +++ b/core/java/android/database/sqlite/SQLiteQuery.java @@ -30,6 +30,11 @@ import android.util.Log; public class SQLiteQuery extends SQLiteProgram { private static final String TAG = "SQLiteQuery"; + private static native int nativeFillWindow(int databasePtr, int statementPtr, int windowPtr, + int startPos, int offsetParam); + private static native int nativeColumnCount(int statementPtr); + private static native String nativeColumnName(int statementPtr, int columnIndex); + /** The index of the unbound OFFSET parameter */ private int mOffsetIndex = 0; @@ -68,19 +73,15 @@ public class SQLiteQuery extends SQLiteProgram { * @param window The window to fill into * @return number of total rows in the query */ - /* package */ int fillWindow(CursorWindow window, - int maxRead, int lastPos) { + /* package */ int fillWindow(CursorWindow window) { mDatabase.lock(mSql); long timeStart = SystemClock.uptimeMillis(); try { acquireReference(); try { window.acquireReference(); - // if the start pos is not equal to 0, then most likely window is - // too small for the data set, loading by another thread - // is not safe in this situation. the native code will ignore maxRead - int numRows = native_fill_window(window.mWindowPtr, window.getStartPosition(), - mOffsetIndex, maxRead, lastPos); + int numRows = nativeFillWindow(nHandle, nStatement, window.mWindowPtr, + window.getStartPosition(), mOffsetIndex); mDatabase.logTimeStat(mSql, timeStart); return numRows; } catch (IllegalStateException e){ @@ -111,7 +112,7 @@ public class SQLiteQuery extends SQLiteProgram { /* package */ int columnCountLocked() { acquireReference(); try { - return native_column_count(); + return nativeColumnCount(nStatement); } finally { releaseReference(); } @@ -127,17 +128,17 @@ public class SQLiteQuery extends SQLiteProgram { /* package */ String columnNameLocked(int columnIndex) { acquireReference(); try { - return native_column_name(columnIndex); + return nativeColumnName(nStatement, columnIndex); } finally { releaseReference(); } } - + @Override public String toString() { return "SQLiteQuery: " + mSql; } - + @Override public void close() { super.close(); @@ -153,11 +154,4 @@ public class SQLiteQuery extends SQLiteProgram { } compileAndbindAllArgs(); } - - private final native int native_fill_window(int windowPtr, - int startPos, int offsetParam, int maxRead, int lastPos); - - private final native int native_column_count(); - - private final native String native_column_name(int columnIndex); } |
