Merge "Fix cursor window leak when query execution fails."

This commit is contained in:
Jeff Brown
2013-01-08 15:57:21 -08:00
committed by Android (Google) Code Review
6 changed files with 56 additions and 23 deletions

View File

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

View File

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

View File

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

View File

@@ -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 {

View File

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

View File

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