diff options
author | Jeff Brown <jeffbrown@google.com> | 2013-01-07 17:15:12 -0800 |
---|---|---|
committer | Jeff Brown <jeffbrown@google.com> | 2013-01-08 15:32:50 -0800 |
commit | c21b5a019c1da00b6d861cd2859e3c349a44b3a7 (patch) | |
tree | 750072f97e644448c149c8d5447d514cca189ba0 | |
parent | f6f00b14fb4d2a247393fd3e6d45de077a8b4da9 (diff) | |
download | frameworks_base-c21b5a019c1da00b6d861cd2859e3c349a44b3a7.zip frameworks_base-c21b5a019c1da00b6d861cd2859e3c349a44b3a7.tar.gz frameworks_base-c21b5a019c1da00b6d861cd2859e3c349a44b3a7.tar.bz2 |
Fix cursor window leak when query execution fails.
Ensure that the Cursor object is closed if a query on a
content provider fails due to an error or is canceled during
execution. There are several places in the code where
similar problems can occur.
To further reduce the likelihood of leaks, close the cursor
window immediately when a query fails.
Bug: 7278577
Change-Id: I8c686c259de80a162b9086628a817d57f09fdd13
-rw-r--r-- | core/java/android/content/AsyncTaskLoader.java | 2 | ||||
-rw-r--r-- | core/java/android/content/ContentProviderNative.java | 22 | ||||
-rw-r--r-- | core/java/android/content/ContentResolver.java | 12 | ||||
-rw-r--r-- | core/java/android/content/CursorLoader.java | 11 | ||||
-rw-r--r-- | core/java/android/database/CursorToBulkCursorAdaptor.java | 5 | ||||
-rw-r--r-- | core/java/android/database/sqlite/SQLiteCursor.java | 29 |
6 files changed, 57 insertions, 24 deletions
diff --git a/core/java/android/content/AsyncTaskLoader.java b/core/java/android/content/AsyncTaskLoader.java index f9025d9..188c786 100644 --- a/core/java/android/content/AsyncTaskLoader.java +++ b/core/java/android/content/AsyncTaskLoader.java @@ -78,7 +78,7 @@ public abstract class AsyncTaskLoader<D> extends Loader<D> { // So we treat this case as an unhandled exception. throw ex; } - if (DEBUG) Slog.v(TAG, this + " <<< doInBackground (was canceled)"); + if (DEBUG) Slog.v(TAG, this + " <<< doInBackground (was canceled)", ex); return null; } } diff --git a/core/java/android/content/ContentProviderNative.java b/core/java/android/content/ContentProviderNative.java index 550a1c9..384ba57 100644 --- a/core/java/android/content/ContentProviderNative.java +++ b/core/java/android/content/ContentProviderNative.java @@ -113,13 +113,21 @@ abstract public class ContentProviderNative extends Binder implements IContentPr Cursor cursor = query(url, projection, selection, selectionArgs, sortOrder, cancellationSignal); if (cursor != null) { - CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor( - cursor, observer, getProviderName()); - BulkCursorDescriptor d = adaptor.getBulkCursorDescriptor(); - - reply.writeNoException(); - reply.writeInt(1); - d.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE); + try { + CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor( + cursor, observer, getProviderName()); + BulkCursorDescriptor d = adaptor.getBulkCursorDescriptor(); + cursor = null; + + reply.writeNoException(); + reply.writeInt(1); + d.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE); + } finally { + // Close cursor if an exception was thrown while constructing the adaptor. + if (cursor != null) { + cursor.close(); + } + } } else { reply.writeNoException(); reply.writeInt(0); diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index e34c827..86bf8c2 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -381,6 +381,7 @@ public abstract class ContentResolver { return null; } IContentProvider stableProvider = null; + Cursor qCursor = null; try { long startTime = SystemClock.uptimeMillis(); @@ -390,7 +391,6 @@ public abstract class ContentResolver { remoteCancellationSignal = unstableProvider.createCancellationSignal(); cancellationSignal.setRemote(remoteCancellationSignal); } - Cursor qCursor; try { qCursor = unstableProvider.query(uri, projection, selection, selectionArgs, sortOrder, remoteCancellationSignal); @@ -409,20 +409,26 @@ public abstract class ContentResolver { if (qCursor == null) { return null; } - // force query execution + + // Force query execution. Might fail and throw a runtime exception here. qCursor.getCount(); long durationMillis = SystemClock.uptimeMillis() - startTime; maybeLogQueryToEventLog(durationMillis, uri, projection, selection, sortOrder); - // Wrap the cursor object into CursorWrapperInner object + + // Wrap the cursor object into CursorWrapperInner object. CursorWrapperInner wrapper = new CursorWrapperInner(qCursor, stableProvider != null ? stableProvider : acquireProvider(uri)); stableProvider = null; + qCursor = null; return wrapper; } catch (RemoteException e) { // Arbitrary and not worth documenting, as Activity // Manager will kill this process shortly anyway. return null; } finally { + if (qCursor != null) { + qCursor.close(); + } if (unstableProvider != null) { releaseUnstableProvider(unstableProvider); } diff --git a/core/java/android/content/CursorLoader.java b/core/java/android/content/CursorLoader.java index 9f7a104..4e89dec 100644 --- a/core/java/android/content/CursorLoader.java +++ b/core/java/android/content/CursorLoader.java @@ -65,9 +65,14 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> { Cursor cursor = getContext().getContentResolver().query(mUri, mProjection, mSelection, mSelectionArgs, mSortOrder, mCancellationSignal); if (cursor != null) { - // Ensure the cursor window is filled - cursor.getCount(); - registerContentObserver(cursor, mObserver); + try { + // Ensure the cursor window is filled. + cursor.getCount(); + registerContentObserver(cursor, mObserver); + } catch (RuntimeException ex) { + cursor.close(); + throw ex; + } } return cursor; } finally { diff --git a/core/java/android/database/CursorToBulkCursorAdaptor.java b/core/java/android/database/CursorToBulkCursorAdaptor.java index 525be96..82a61d4 100644 --- a/core/java/android/database/CursorToBulkCursorAdaptor.java +++ b/core/java/android/database/CursorToBulkCursorAdaptor.java @@ -132,6 +132,11 @@ public final class CursorToBulkCursorAdaptor extends BulkCursorNative } } + /** + * Returns an object that contains sufficient metadata to reconstruct + * the cursor remotely. May throw if an error occurs when executing the query + * and obtaining the row count. + */ public BulkCursorDescriptor getBulkCursorDescriptor() { synchronized (mLock) { throwIfCursorIsClosed(); diff --git a/core/java/android/database/sqlite/SQLiteCursor.java b/core/java/android/database/sqlite/SQLiteCursor.java index b29897e..5a1a8e2 100644 --- a/core/java/android/database/sqlite/SQLiteCursor.java +++ b/core/java/android/database/sqlite/SQLiteCursor.java @@ -138,17 +138,26 @@ public class SQLiteCursor extends AbstractWindowedCursor { private void fillWindow(int requiredPos) { clearOrCreateWindow(getDatabase().getPath()); - if (mCount == NO_COUNT) { - int startPos = DatabaseUtils.cursorPickFillWindowStartPosition(requiredPos, 0); - mCount = mQuery.fillWindow(mWindow, startPos, requiredPos, true); - mCursorWindowCapacity = mWindow.getNumRows(); - if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "received count(*) from native_fill_window: " + mCount); + try { + if (mCount == NO_COUNT) { + int startPos = DatabaseUtils.cursorPickFillWindowStartPosition(requiredPos, 0); + mCount = mQuery.fillWindow(mWindow, startPos, requiredPos, true); + mCursorWindowCapacity = mWindow.getNumRows(); + if (Log.isLoggable(TAG, Log.DEBUG)) { + Log.d(TAG, "received count(*) from native_fill_window: " + mCount); + } + } else { + int startPos = DatabaseUtils.cursorPickFillWindowStartPosition(requiredPos, + mCursorWindowCapacity); + mQuery.fillWindow(mWindow, startPos, requiredPos, false); } - } else { - int startPos = DatabaseUtils.cursorPickFillWindowStartPosition(requiredPos, - mCursorWindowCapacity); - mQuery.fillWindow(mWindow, startPos, requiredPos, false); + } catch (RuntimeException ex) { + // Close the cursor window if the query failed and therefore will + // not produce any results. This helps to avoid accidentally leaking + // the cursor window if the client does not correctly handle exceptions + // and fails to close the cursor. + closeWindow(); + throw ex; } } |