Fix ownership of CursorWindows across processes.
Bug: 5332296
Ensure that there is always an owner for each CursorWindow
and that references to each window are acquired/released
appropriately at all times.
Added synchronization to CursorToBulkCursorAdaptor to
prevent the underlying Cursor and CursorWindow from being
remotely accessed in ways that might violate invariants,
resulting in leaks or other problems.
Ensured that CursorToBulkCursorAdaptor promptly releases
its references to the Cursor and CursorWindow when closed
so they don't stick around longer than they should, even
if the remote end hangs onto the IBulkCursor for some reason.
CursorWindow respects Parcelable.FLAG_WRITE_RETURN_VALUE
as an indication that one reference to the CursorWindow is
being released. Correspondingly, CursorToBulkCursorAdaptor
acquires a reference to the CursorWindow before returning
it to the caller. This change also prevents races from
resulting in the transfer of an invalid CursorWindow over
the wire.
Ensured that BulkCursorToCursorAdaptor promptly releases
its reference to the IBulkCursor when closed and throws
on attempts to access the cursor while closed.
Modified ContentProviderNative to handle both parts of
the wrapping and unwrapping of Cursors into IBulkCursors.
This makes it a lot easier to ensure that the right
things happen on both ends. Also, it turns out that
the only caller of IContentProvider.bulkQuery was
ContentProviderNative itself so there was no need
to support bulkQuery on ContentProviderProxy and it was
just getting in the way.
Implement CloseGuard on CursorWindow.
Change-Id: Ib3c8305d3cc62322f38a06698d404a2989bb6ef9
diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index 8057d4b..e452f1f 100644
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -22,10 +22,6 @@
import android.content.res.AssetFileDescriptor;
import android.content.res.Configuration;
import android.database.Cursor;
-import android.database.CursorToBulkCursorAdaptor;
-import android.database.CursorWindow;
-import android.database.IBulkCursor;
-import android.database.IContentObserver;
import android.database.SQLException;
import android.net.Uri;
import android.os.AsyncTask;
@@ -168,22 +164,9 @@
return ContentProvider.this;
}
- /**
- * Remote version of a query, which returns an IBulkCursor. The bulk
- * cursor should be wrapped with BulkCursorToCursorAdaptor before use.
- */
- public IBulkCursor bulkQuery(Uri uri, String[] projection,
- String selection, String[] selectionArgs, String sortOrder,
- IContentObserver observer, CursorWindow window) {
- enforceReadPermission(uri);
- Cursor cursor = ContentProvider.this.query(uri, projection,
- selection, selectionArgs, sortOrder);
- if (cursor == null) {
- return null;
- }
- return new CursorToBulkCursorAdaptor(cursor, observer,
- ContentProvider.this.getClass().getName(),
- hasWritePermission(uri), window);
+ @Override
+ public String getProviderName() {
+ return getContentProvider().getClass().getName();
}
public Cursor query(Uri uri, String[] projection,
diff --git a/core/java/android/content/ContentProviderNative.java b/core/java/android/content/ContentProviderNative.java
index 9a20951..064755e 100644
--- a/core/java/android/content/ContentProviderNative.java
+++ b/core/java/android/content/ContentProviderNative.java
@@ -20,6 +20,7 @@
import android.database.BulkCursorNative;
import android.database.BulkCursorToCursorAdaptor;
import android.database.Cursor;
+import android.database.CursorToBulkCursorAdaptor;
import android.database.CursorWindow;
import android.database.DatabaseUtils;
import android.database.IBulkCursor;
@@ -65,6 +66,13 @@
return new ContentProviderProxy(obj);
}
+ /**
+ * Gets the name of the content provider.
+ * Should probably be part of the {@link IContentProvider} interface.
+ * @return The content provider name.
+ */
+ public abstract String getProviderName();
+
@Override
public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
throws RemoteException {
@@ -98,33 +106,23 @@
}
String sortOrder = data.readString();
- IContentObserver observer = IContentObserver.Stub.
- asInterface(data.readStrongBinder());
+ IContentObserver observer = IContentObserver.Stub.asInterface(
+ data.readStrongBinder());
CursorWindow window = CursorWindow.CREATOR.createFromParcel(data);
- // Flag for whether caller wants the number of
- // rows in the cursor and the position of the
- // "_id" column index (or -1 if non-existent)
- // Only to be returned if binder != null.
- boolean wantsCursorMetadata = data.readInt() != 0;
+ Cursor cursor = query(url, projection, selection, selectionArgs, sortOrder);
+ if (cursor != null) {
+ CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor(
+ cursor, observer, getProviderName(), window);
+ final IBinder binder = adaptor.asBinder();
+ final int count = adaptor.count();
+ final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
+ adaptor.getColumnNames());
- IBulkCursor bulkCursor = bulkQuery(url, projection, selection,
- selectionArgs, sortOrder, observer, window);
- if (bulkCursor != null) {
- final IBinder binder = bulkCursor.asBinder();
- if (wantsCursorMetadata) {
- final int count = bulkCursor.count();
- final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
- bulkCursor.getColumnNames());
-
- reply.writeNoException();
- reply.writeStrongBinder(binder);
- reply.writeInt(count);
- reply.writeInt(index);
- } else {
- reply.writeNoException();
- reply.writeStrongBinder(binder);
- }
+ reply.writeNoException();
+ reply.writeStrongBinder(binder);
+ reply.writeInt(count);
+ reply.writeInt(index);
} else {
reply.writeNoException();
reply.writeStrongBinder(null);
@@ -324,92 +322,70 @@
return mRemote;
}
- // Like bulkQuery() but sets up provided 'adaptor' if not null.
- private IBulkCursor bulkQueryInternal(
- Uri url, String[] projection,
- String selection, String[] selectionArgs, String sortOrder,
- IContentObserver observer, CursorWindow window,
- BulkCursorToCursorAdaptor adaptor) throws RemoteException {
- Parcel data = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- try {
- data.writeInterfaceToken(IContentProvider.descriptor);
-
- url.writeToParcel(data, 0);
- int length = 0;
- if (projection != null) {
- length = projection.length;
- }
- data.writeInt(length);
- for (int i = 0; i < length; i++) {
- data.writeString(projection[i]);
- }
- data.writeString(selection);
- if (selectionArgs != null) {
- length = selectionArgs.length;
- } else {
- length = 0;
- }
- data.writeInt(length);
- for (int i = 0; i < length; i++) {
- data.writeString(selectionArgs[i]);
- }
- data.writeString(sortOrder);
- data.writeStrongBinder(observer.asBinder());
- window.writeToParcel(data, 0);
-
- // Flag for whether or not we want the number of rows in the
- // cursor and the position of the "_id" column index (or -1 if
- // non-existent). Only to be returned if binder != null.
- final boolean wantsCursorMetadata = (adaptor != null);
- data.writeInt(wantsCursorMetadata ? 1 : 0);
-
- mRemote.transact(IContentProvider.QUERY_TRANSACTION, data, reply, 0);
-
- DatabaseUtils.readExceptionFromParcel(reply);
-
- IBulkCursor bulkCursor = null;
- IBinder bulkCursorBinder = reply.readStrongBinder();
- if (bulkCursorBinder != null) {
- bulkCursor = BulkCursorNative.asInterface(bulkCursorBinder);
-
- if (wantsCursorMetadata) {
- int rowCount = reply.readInt();
- int idColumnPosition = reply.readInt();
- if (bulkCursor != null) {
- adaptor.set(bulkCursor, rowCount, idColumnPosition);
- }
- }
- }
- return bulkCursor;
- } finally {
- data.recycle();
- reply.recycle();
- }
- }
-
- public IBulkCursor bulkQuery(Uri url, String[] projection,
- String selection, String[] selectionArgs, String sortOrder, IContentObserver observer,
- CursorWindow window) throws RemoteException {
- return bulkQueryInternal(
- url, projection, selection, selectionArgs, sortOrder,
- observer, window,
- null /* BulkCursorToCursorAdaptor */);
- }
-
public Cursor query(Uri url, String[] projection, String selection,
String[] selectionArgs, String sortOrder) throws RemoteException {
- //TODO make a pool of windows so we can reuse memory dealers
CursorWindow window = new CursorWindow(false /* window will be used remotely */);
- BulkCursorToCursorAdaptor adaptor = new BulkCursorToCursorAdaptor();
- IBulkCursor bulkCursor = bulkQueryInternal(
- url, projection, selection, selectionArgs, sortOrder,
- adaptor.getObserver(), window,
- adaptor);
- if (bulkCursor == null) {
- return null;
+ try {
+ BulkCursorToCursorAdaptor adaptor = new BulkCursorToCursorAdaptor();
+ Parcel data = Parcel.obtain();
+ Parcel reply = Parcel.obtain();
+ try {
+ data.writeInterfaceToken(IContentProvider.descriptor);
+
+ url.writeToParcel(data, 0);
+ int length = 0;
+ if (projection != null) {
+ length = projection.length;
+ }
+ data.writeInt(length);
+ for (int i = 0; i < length; i++) {
+ data.writeString(projection[i]);
+ }
+ data.writeString(selection);
+ if (selectionArgs != null) {
+ length = selectionArgs.length;
+ } else {
+ length = 0;
+ }
+ data.writeInt(length);
+ for (int i = 0; i < length; i++) {
+ data.writeString(selectionArgs[i]);
+ }
+ data.writeString(sortOrder);
+ data.writeStrongBinder(adaptor.getObserver().asBinder());
+ window.writeToParcel(data, 0);
+
+ mRemote.transact(IContentProvider.QUERY_TRANSACTION, data, reply, 0);
+
+ DatabaseUtils.readExceptionFromParcel(reply);
+
+ IBulkCursor bulkCursor = BulkCursorNative.asInterface(reply.readStrongBinder());
+ if (bulkCursor != null) {
+ int rowCount = reply.readInt();
+ int idColumnPosition = reply.readInt();
+ adaptor.initialize(bulkCursor, rowCount, idColumnPosition);
+ } else {
+ adaptor.close();
+ adaptor = null;
+ }
+ return adaptor;
+ } catch (RemoteException ex) {
+ adaptor.close();
+ throw ex;
+ } catch (RuntimeException ex) {
+ adaptor.close();
+ throw ex;
+ } finally {
+ data.recycle();
+ reply.recycle();
+ }
+ } finally {
+ // We close the window now because the cursor adaptor does not
+ // take ownership of the window until the first call to onMove.
+ // The adaptor will obtain a fresh reference to the window when
+ // it is filled.
+ window.close();
}
- return adaptor;
}
public String getType(Uri url) throws RemoteException
diff --git a/core/java/android/content/IContentProvider.java b/core/java/android/content/IContentProvider.java
index 72bc9c2..2a67ff8 100644
--- a/core/java/android/content/IContentProvider.java
+++ b/core/java/android/content/IContentProvider.java
@@ -18,9 +18,6 @@
import android.content.res.AssetFileDescriptor;
import android.database.Cursor;
-import android.database.CursorWindow;
-import android.database.IBulkCursor;
-import android.database.IContentObserver;
import android.net.Uri;
import android.os.Bundle;
import android.os.IBinder;
@@ -36,13 +33,6 @@
* @hide
*/
public interface IContentProvider extends IInterface {
- /**
- * @hide - hide this because return type IBulkCursor and parameter
- * IContentObserver are system private classes.
- */
- public IBulkCursor bulkQuery(Uri url, String[] projection,
- String selection, String[] selectionArgs, String sortOrder, IContentObserver observer,
- CursorWindow window) throws RemoteException;
public Cursor query(Uri url, String[] projection, String selection,
String[] selectionArgs, String sortOrder) throws RemoteException;
public String getType(Uri url) throws RemoteException;
diff --git a/core/java/android/database/AbstractCursor.java b/core/java/android/database/AbstractCursor.java
index 3f23b89..ee6aec6 100644
--- a/core/java/android/database/AbstractCursor.java
+++ b/core/java/android/database/AbstractCursor.java
@@ -78,13 +78,11 @@
}
public void deactivate() {
- deactivateInternal();
+ onDeactivateOrClose();
}
- /**
- * @hide
- */
- public void deactivateInternal() {
+ /** @hide */
+ protected void onDeactivateOrClose() {
if (mSelfObserver != null) {
mContentResolver.unregisterContentObserver(mSelfObserver);
mSelfObserverRegistered = false;
@@ -108,7 +106,7 @@
public void close() {
mClosed = true;
mContentObservable.unregisterAll();
- deactivateInternal();
+ onDeactivateOrClose();
}
/**
diff --git a/core/java/android/database/AbstractWindowedCursor.java b/core/java/android/database/AbstractWindowedCursor.java
index bfc8123..5836265 100644
--- a/core/java/android/database/AbstractWindowedCursor.java
+++ b/core/java/android/database/AbstractWindowedCursor.java
@@ -19,6 +19,11 @@
/**
* A base class for Cursors that store their data in {@link CursorWindow}s.
* <p>
+ * The cursor owns the cursor window it uses. When the cursor is closed,
+ * its window is also closed. Likewise, when the window used by the cursor is
+ * changed, its old window is closed. This policy of strict ownership ensures
+ * that cursor windows are not leaked.
+ * </p><p>
* Subclasses are responsible for filling the cursor window with data during
* {@link #onMove(int, int)}, allocating a new cursor window if necessary.
* During {@link #requery()}, the existing cursor window should be cleared and
@@ -180,4 +185,25 @@
mWindow = null;
}
}
+
+ /**
+ * If there is a window, clear it.
+ * Otherwise, creates a local window.
+ * @hide
+ */
+ protected void clearOrCreateLocalWindow() {
+ 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 {
+ mWindow.clear();
+ }
+ }
+
+ /** @hide */
+ @Override
+ protected void onDeactivateOrClose() {
+ super.onDeactivateOrClose();
+ closeWindow();
+ }
}
diff --git a/core/java/android/database/BulkCursorNative.java b/core/java/android/database/BulkCursorNative.java
index 9925a9a..4fada8c 100644
--- a/core/java/android/database/BulkCursorNative.java
+++ b/core/java/android/database/BulkCursorNative.java
@@ -62,13 +62,13 @@
data.enforceInterface(IBulkCursor.descriptor);
int startPos = data.readInt();
CursorWindow window = getWindow(startPos);
+ reply.writeNoException();
if (window == null) {
reply.writeInt(0);
- return true;
+ } else {
+ reply.writeInt(1);
+ window.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
}
- reply.writeNoException();
- reply.writeInt(1);
- window.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
return true;
}
diff --git a/core/java/android/database/BulkCursorToCursorAdaptor.java b/core/java/android/database/BulkCursorToCursorAdaptor.java
index 9c1b26d..cbdd07fb 100644
--- a/core/java/android/database/BulkCursorToCursorAdaptor.java
+++ b/core/java/android/database/BulkCursorToCursorAdaptor.java
@@ -21,40 +21,24 @@
import android.util.Log;
/**
- * Adapts an {@link IBulkCursor} to a {@link Cursor} for use in the local
- * process.
+ * Adapts an {@link IBulkCursor} to a {@link Cursor} for use in the local process.
*
* {@hide}
*/
public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
private static final String TAG = "BulkCursor";
- private SelfContentObserver mObserverBridge;
+ private SelfContentObserver mObserverBridge = new SelfContentObserver(this);
private IBulkCursor mBulkCursor;
private int mCount;
private String[] mColumns;
private boolean mWantsAllOnMoveCalls;
- public void set(IBulkCursor bulkCursor) {
- mBulkCursor = bulkCursor;
-
- try {
- mCount = mBulkCursor.count();
- mWantsAllOnMoveCalls = mBulkCursor.getWantsAllOnMoveCalls();
-
- // Search for the rowID column index and set it for our parent
- mColumns = mBulkCursor.getColumnNames();
- mRowIdColumnIndex = findRowIdColumnIndex(mColumns);
- } catch (RemoteException ex) {
- Log.e(TAG, "Setup failed because the remote process is dead");
- }
- }
-
/**
- * Version of set() that does fewer Binder calls if the caller
- * already knows BulkCursorToCursorAdaptor's properties.
+ * Initializes the adaptor.
+ * Must be called before first use.
*/
- public void set(IBulkCursor bulkCursor, int count, int idIndex) {
+ public void initialize(IBulkCursor bulkCursor, int count, int idIndex) {
mBulkCursor = bulkCursor;
mColumns = null; // lazily retrieved
mCount = count;
@@ -80,31 +64,37 @@
*
* @return A SelfContentObserver hooked up to this Cursor
*/
- public synchronized IContentObserver getObserver() {
- if (mObserverBridge == null) {
- mObserverBridge = new SelfContentObserver(this);
- }
+ public IContentObserver getObserver() {
return mObserverBridge.getContentObserver();
}
+ private void throwIfCursorIsClosed() {
+ if (mBulkCursor == null) {
+ throw new StaleDataException("Attempted to access a cursor after it has been closed.");
+ }
+ }
+
@Override
public int getCount() {
+ throwIfCursorIsClosed();
return mCount;
}
@Override
public boolean onMove(int oldPosition, int newPosition) {
+ throwIfCursorIsClosed();
+
try {
// Make sure we have the proper window
if (mWindow != null) {
if (newPosition < mWindow.getStartPosition() ||
newPosition >= (mWindow.getStartPosition() + mWindow.getNumRows())) {
- mWindow = mBulkCursor.getWindow(newPosition);
+ setWindow(mBulkCursor.getWindow(newPosition));
} else if (mWantsAllOnMoveCalls) {
mBulkCursor.onMove(newPosition);
}
} else {
- mWindow = mBulkCursor.getWindow(newPosition);
+ setWindow(mBulkCursor.getWindow(newPosition));
}
} catch (RemoteException ex) {
// We tried to get a window and failed
@@ -126,44 +116,54 @@
// which is what actually makes the data set invalid.
super.deactivate();
- try {
- mBulkCursor.deactivate();
- } catch (RemoteException ex) {
- Log.w(TAG, "Remote process exception when deactivating");
+ if (mBulkCursor != null) {
+ try {
+ mBulkCursor.deactivate();
+ } catch (RemoteException ex) {
+ Log.w(TAG, "Remote process exception when deactivating");
+ }
}
- mWindow = null;
}
@Override
public void close() {
super.close();
- try {
- mBulkCursor.close();
- } catch (RemoteException ex) {
- Log.w(TAG, "Remote process exception when closing");
+
+ if (mBulkCursor != null) {
+ try {
+ mBulkCursor.close();
+ } catch (RemoteException ex) {
+ Log.w(TAG, "Remote process exception when closing");
+ } finally {
+ mBulkCursor = null;
+ }
}
- mWindow = null;
}
@Override
public boolean requery() {
- try {
- int oldCount = mCount;
- //TODO get the window from a pool somewhere to avoid creating the memory dealer
- mCount = mBulkCursor.requery(getObserver(), new CursorWindow(
- false /* the window will be accessed across processes */));
- if (mCount != -1) {
- mPos = -1;
- closeWindow();
+ throwIfCursorIsClosed();
- // super.requery() will call onChanged. Do it here instead of relying on the
- // observer from the far side so that observers can see a correct value for mCount
- // when responding to onChanged.
- super.requery();
- return true;
- } else {
- deactivate();
- return false;
+ try {
+ CursorWindow newWindow = new CursorWindow(false /* create a remote window */);
+ try {
+ mCount = mBulkCursor.requery(getObserver(), newWindow);
+ if (mCount != -1) {
+ mPos = -1;
+ closeWindow();
+
+ // super.requery() will call onChanged. Do it here instead of relying on the
+ // observer from the far side so that observers can see a correct value for mCount
+ // when responding to onChanged.
+ super.requery();
+ return true;
+ } else {
+ deactivate();
+ return false;
+ }
+ } finally {
+ // Don't take ownership of the window until the next call to onMove.
+ newWindow.close();
}
} catch (Exception ex) {
Log.e(TAG, "Unable to requery because the remote process exception " + ex.getMessage());
@@ -174,6 +174,8 @@
@Override
public String[] getColumnNames() {
+ throwIfCursorIsClosed();
+
if (mColumns == null) {
try {
mColumns = mBulkCursor.getColumnNames();
@@ -187,6 +189,8 @@
@Override
public Bundle getExtras() {
+ throwIfCursorIsClosed();
+
try {
return mBulkCursor.getExtras();
} catch (RemoteException e) {
@@ -198,6 +202,8 @@
@Override
public Bundle respond(Bundle extras) {
+ throwIfCursorIsClosed();
+
try {
return mBulkCursor.respond(extras);
} catch (RemoteException e) {
diff --git a/core/java/android/database/CrossProcessCursor.java b/core/java/android/database/CrossProcessCursor.java
index 77ba3a5..8e6a5aa 100644
--- a/core/java/android/database/CrossProcessCursor.java
+++ b/core/java/android/database/CrossProcessCursor.java
@@ -16,7 +16,7 @@
package android.database;
-public interface CrossProcessCursor extends Cursor{
+public interface CrossProcessCursor extends Cursor {
/**
* returns a pre-filled window, return NULL if no such window
*/
diff --git a/core/java/android/database/CursorToBulkCursorAdaptor.java b/core/java/android/database/CursorToBulkCursorAdaptor.java
index 8fa4d3b..a65b3b3 100644
--- a/core/java/android/database/CursorToBulkCursorAdaptor.java
+++ b/core/java/android/database/CursorToBulkCursorAdaptor.java
@@ -24,19 +24,37 @@
/**
* Wraps a BulkCursor around an existing Cursor making it remotable.
+ * <p>
+ * If the wrapped cursor is a {@link AbstractWindowedCursor} then it owns
+ * the cursor window. Otherwise, the adaptor takes ownership of the
+ * cursor itself and ensures it gets closed as needed during deactivation
+ * and requeries.
+ * </p>
*
* {@hide}
*/
public final class CursorToBulkCursorAdaptor extends BulkCursorNative
implements IBinder.DeathRecipient {
private static final String TAG = "Cursor";
- private final CrossProcessCursor mCursor;
- private CursorWindow mWindow;
+
+ private final Object mLock = new Object();
private final String mProviderName;
private ContentObserverProxy mObserver;
- private static final class ContentObserverProxy extends ContentObserver
- {
+ /**
+ * The cursor that is being adapted.
+ * This field is set to null when the cursor is closed.
+ */
+ private CrossProcessCursor mCursor;
+
+ /**
+ * The cursor window used by the cross process cursor.
+ * This field is always null for abstract windowed cursors since they are responsible
+ * for managing the lifetime of their window.
+ */
+ private CursorWindow mWindowForNonWindowedCursor;
+
+ private static final class ContentObserverProxy extends ContentObserver {
protected IContentObserver mRemote;
public ContentObserverProxy(IContentObserver remoteObserver, DeathRecipient recipient) {
@@ -70,7 +88,7 @@
}
public CursorToBulkCursorAdaptor(Cursor cursor, IContentObserver observer, String providerName,
- boolean allowWrite, CursorWindow window) {
+ CursorWindow window) {
try {
mCursor = (CrossProcessCursor) cursor;
if (mCursor instanceof AbstractWindowedCursor) {
@@ -81,90 +99,167 @@
+ providerName, new RuntimeException());
}
}
- windowedCursor.setWindow(window);
+ windowedCursor.setWindow(window); // cursor takes ownership of window
} else {
- mWindow = window;
+ mWindowForNonWindowedCursor = window; // we own the window
mCursor.fillWindow(0, window);
}
} catch (ClassCastException e) {
// TODO Implement this case.
+ window.close();
throw new UnsupportedOperationException(
"Only CrossProcessCursor cursors are supported across process for now", e);
}
mProviderName = providerName;
- createAndRegisterObserverProxy(observer);
+ synchronized (mLock) {
+ createAndRegisterObserverProxyLocked(observer);
+ }
}
-
+
+ private void closeCursorAndWindowLocked() {
+ if (mCursor != null) {
+ unregisterObserverProxyLocked();
+ mCursor.close();
+ mCursor = null;
+ }
+
+ if (mWindowForNonWindowedCursor != null) {
+ mWindowForNonWindowedCursor.close();
+ mWindowForNonWindowedCursor = null;
+ }
+ }
+
+ private void throwIfCursorIsClosed() {
+ if (mCursor == null) {
+ throw new StaleDataException("Attempted to access a cursor after it has been closed.");
+ }
+ }
+
+ @Override
public void binderDied() {
- mCursor.close();
- if (mWindow != null) {
- mWindow.close();
+ synchronized (mLock) {
+ closeCursorAndWindowLocked();
}
}
-
+
+ @Override
public CursorWindow getWindow(int startPos) {
- mCursor.moveToPosition(startPos);
-
- if (mWindow != null) {
- if (startPos < mWindow.getStartPosition() ||
- startPos >= (mWindow.getStartPosition() + mWindow.getNumRows())) {
- mCursor.fillWindow(startPos, mWindow);
- }
- return mWindow;
- } else {
- return ((AbstractWindowedCursor)mCursor).getWindow();
- }
- }
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
- public void onMove(int position) {
- mCursor.onMove(mCursor.getPosition(), position);
- }
+ mCursor.moveToPosition(startPos);
- public int count() {
- return mCursor.getCount();
- }
-
- public String[] getColumnNames() {
- return mCursor.getColumnNames();
- }
-
- public void deactivate() {
- maybeUnregisterObserverProxy();
- mCursor.deactivate();
- }
-
- public void close() {
- maybeUnregisterObserverProxy();
- mCursor.close();
- }
-
- public int requery(IContentObserver observer, CursorWindow window) {
- if (mWindow == null) {
- ((AbstractWindowedCursor)mCursor).setWindow(window);
- }
- try {
- if (!mCursor.requery()) {
- return -1;
+ final CursorWindow window;
+ if (mCursor instanceof AbstractWindowedCursor) {
+ window = ((AbstractWindowedCursor)mCursor).getWindow();
+ } else {
+ window = mWindowForNonWindowedCursor;
+ if (window != null
+ && (startPos < window.getStartPosition() ||
+ startPos >= (window.getStartPosition() + window.getNumRows()))) {
+ mCursor.fillWindow(startPos, window);
+ }
}
- } catch (IllegalStateException e) {
- IllegalStateException leakProgram = new IllegalStateException(
- mProviderName + " Requery misuse db, mCursor isClosed:" +
- mCursor.isClosed(), e);
- throw leakProgram;
+
+ // Acquire a reference before returning from this RPC.
+ // The Binder proxy will decrement the reference count again as part of writing
+ // the CursorWindow to the reply parcel as a return value.
+ if (window != null) {
+ window.acquireReference();
+ }
+ return window;
}
-
- if (mWindow != null) {
- mCursor.fillWindow(0, window);
- mWindow = window;
- }
- maybeUnregisterObserverProxy();
- createAndRegisterObserverProxy(observer);
- return mCursor.getCount();
}
+ @Override
+ public void onMove(int position) {
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ mCursor.onMove(mCursor.getPosition(), position);
+ }
+ }
+
+ @Override
+ public int count() {
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ return mCursor.getCount();
+ }
+ }
+
+ @Override
+ public String[] getColumnNames() {
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ return mCursor.getColumnNames();
+ }
+ }
+
+ @Override
+ public void deactivate() {
+ synchronized (mLock) {
+ if (mCursor != null) {
+ unregisterObserverProxyLocked();
+ mCursor.deactivate();
+ }
+ }
+ }
+
+ @Override
+ public void close() {
+ synchronized (mLock) {
+ closeCursorAndWindowLocked();
+ }
+ }
+
+ @Override
+ public int requery(IContentObserver observer, CursorWindow window) {
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ if (mCursor instanceof AbstractWindowedCursor) {
+ ((AbstractWindowedCursor) mCursor).setWindow(window);
+ } else {
+ if (mWindowForNonWindowedCursor != null) {
+ mWindowForNonWindowedCursor.close();
+ }
+ mWindowForNonWindowedCursor = window;
+ }
+
+ try {
+ if (!mCursor.requery()) {
+ return -1;
+ }
+ } catch (IllegalStateException e) {
+ IllegalStateException leakProgram = new IllegalStateException(
+ mProviderName + " Requery misuse db, mCursor isClosed:" +
+ mCursor.isClosed(), e);
+ throw leakProgram;
+ }
+
+ if (!(mCursor instanceof AbstractWindowedCursor)) {
+ if (window != null) {
+ mCursor.fillWindow(0, window);
+ }
+ }
+
+ unregisterObserverProxyLocked();
+ createAndRegisterObserverProxyLocked(observer);
+ return mCursor.getCount();
+ }
+ }
+
+ @Override
public boolean getWantsAllOnMoveCalls() {
- return mCursor.getWantsAllOnMoveCalls();
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ return mCursor.getWantsAllOnMoveCalls();
+ }
}
/**
@@ -173,7 +268,7 @@
* @param observer the IContentObserver that wants to monitor the cursor
* @throws IllegalStateException if an observer is already registered
*/
- private void createAndRegisterObserverProxy(IContentObserver observer) {
+ private void createAndRegisterObserverProxyLocked(IContentObserver observer) {
if (mObserver != null) {
throw new IllegalStateException("an observer is already registered");
}
@@ -182,7 +277,7 @@
}
/** Unregister the observer if it is already registered. */
- private void maybeUnregisterObserverProxy() {
+ private void unregisterObserverProxyLocked() {
if (mObserver != null) {
mCursor.unregisterContentObserver(mObserver);
mObserver.unlinkToDeath(this);
@@ -190,11 +285,21 @@
}
}
+ @Override
public Bundle getExtras() {
- return mCursor.getExtras();
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ return mCursor.getExtras();
+ }
}
+ @Override
public Bundle respond(Bundle extras) {
- return mCursor.respond(extras);
+ synchronized (mLock) {
+ throwIfCursorIsClosed();
+
+ return mCursor.respond(extras);
+ }
}
}
diff --git a/core/java/android/database/CursorWindow.java b/core/java/android/database/CursorWindow.java
index 2e3ef28..5a91b80 100644
--- a/core/java/android/database/CursorWindow.java
+++ b/core/java/android/database/CursorWindow.java
@@ -16,6 +16,8 @@
package android.database;
+import dalvik.system.CloseGuard;
+
import android.content.res.Resources;
import android.database.sqlite.SQLiteClosable;
import android.database.sqlite.SQLiteException;
@@ -48,6 +50,8 @@
private int mStartPos;
+ private final CloseGuard mCloseGuard = CloseGuard.get();
+
private static native int nativeInitializeEmpty(int cursorWindowSize, boolean localOnly);
private static native int nativeInitializeFromBinder(IBinder nativeBinder);
private static native void nativeDispose(int windowPtr);
@@ -91,6 +95,7 @@
throw new CursorWindowAllocationException("Cursor window allocation of " +
(sCursorWindowSize / 1024) + " kb failed. " + printStats());
}
+ mCloseGuard.open("close");
recordNewWindow(Binder.getCallingPid(), mWindowPtr);
}
@@ -102,11 +107,15 @@
throw new CursorWindowAllocationException("Cursor window could not be "
+ "created from binder.");
}
+ mCloseGuard.open("close");
}
@Override
protected void finalize() throws Throwable {
try {
+ if (mCloseGuard != null) {
+ mCloseGuard.warnIfOpen();
+ }
dispose();
} finally {
super.finalize();
@@ -114,6 +123,9 @@
}
private void dispose() {
+ if (mCloseGuard != null) {
+ mCloseGuard.close();
+ }
if (mWindowPtr != 0) {
recordClosingOfWindow(mWindowPtr);
nativeDispose(mWindowPtr);
@@ -677,6 +689,10 @@
public void writeToParcel(Parcel dest, int flags) {
dest.writeStrongBinder(nativeGetBinder(mWindowPtr));
dest.writeInt(mStartPos);
+
+ if ((flags & Parcelable.PARCELABLE_WRITE_RETURN_VALUE) != 0) {
+ releaseReference();
+ }
}
@Override
diff --git a/core/java/android/database/sqlite/SQLiteCursor.java b/core/java/android/database/sqlite/SQLiteCursor.java
index 81fe824..9d7e152 100644
--- a/core/java/android/database/sqlite/SQLiteCursor.java
+++ b/core/java/android/database/sqlite/SQLiteCursor.java
@@ -89,8 +89,6 @@
* @param query the {@link SQLiteQuery} object associated with this cursor object.
*/
public SQLiteCursor(SQLiteCursorDriver driver, String editTable, SQLiteQuery query) {
- // The AbstractCursor constructor needs to do some setup.
- super();
if (query == null) {
throw new IllegalArgumentException("query object cannot be null");
}
@@ -157,12 +155,7 @@
}
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 {
- mWindow.clear();
- }
+ clearOrCreateLocalWindow();
mWindow.setStartPosition(startPos);
int count = getQuery().fillWindow(mWindow);
if (startPos == 0) { // fillWindow returns count(*) only for startPos = 0
@@ -214,16 +207,9 @@
return mColumns;
}
- private void deactivateCommon() {
- if (false) Log.v(TAG, "<<< Releasing cursor " + this);
- closeWindow();
- if (false) Log.v("DatabaseWindow", "closing window in release()");
- }
-
@Override
public void deactivate() {
super.deactivate();
- deactivateCommon();
mDriver.cursorDeactivated();
}
@@ -231,7 +217,6 @@
public void close() {
super.close();
synchronized (this) {
- deactivateCommon();
mQuery.close();
mDriver.cursorClosed();
}
diff --git a/test-runner/src/android/test/mock/MockContentProvider.java b/test-runner/src/android/test/mock/MockContentProvider.java
index b63ff3d..e0ce322 100644
--- a/test-runner/src/android/test/mock/MockContentProvider.java
+++ b/test-runner/src/android/test/mock/MockContentProvider.java
@@ -21,16 +21,12 @@
import android.content.ContentProviderResult;
import android.content.ContentValues;
import android.content.Context;
-import android.content.EntityIterator;
import android.content.IContentProvider;
import android.content.OperationApplicationException;
import android.content.pm.PathPermission;
import android.content.pm.ProviderInfo;
import android.content.res.AssetFileDescriptor;
import android.database.Cursor;
-import android.database.CursorWindow;
-import android.database.IBulkCursor;
-import android.database.IContentObserver;
import android.net.Uri;
import android.os.Bundle;
import android.os.IBinder;
@@ -55,84 +51,75 @@
* IContentProvider that directs all calls to this MockContentProvider.
*/
private class InversionIContentProvider implements IContentProvider {
- @SuppressWarnings("unused")
+ @Override
public ContentProviderResult[] applyBatch(ArrayList<ContentProviderOperation> operations)
throws RemoteException, OperationApplicationException {
return MockContentProvider.this.applyBatch(operations);
}
- @SuppressWarnings("unused")
+ @Override
public int bulkInsert(Uri url, ContentValues[] initialValues) throws RemoteException {
return MockContentProvider.this.bulkInsert(url, initialValues);
}
- @SuppressWarnings("unused")
- public IBulkCursor bulkQuery(Uri url, String[] projection, String selection,
- String[] selectionArgs, String sortOrder, IContentObserver observer,
- CursorWindow window) throws RemoteException {
- throw new UnsupportedOperationException("Must not come here");
- }
-
- @SuppressWarnings("unused")
+ @Override
public int delete(Uri url, String selection, String[] selectionArgs)
throws RemoteException {
return MockContentProvider.this.delete(url, selection, selectionArgs);
}
- @SuppressWarnings("unused")
+ @Override
public String getType(Uri url) throws RemoteException {
return MockContentProvider.this.getType(url);
}
- @SuppressWarnings("unused")
+ @Override
public Uri insert(Uri url, ContentValues initialValues) throws RemoteException {
return MockContentProvider.this.insert(url, initialValues);
}
- @SuppressWarnings("unused")
+ @Override
public AssetFileDescriptor openAssetFile(Uri url, String mode) throws RemoteException,
FileNotFoundException {
return MockContentProvider.this.openAssetFile(url, mode);
}
- @SuppressWarnings("unused")
+ @Override
public ParcelFileDescriptor openFile(Uri url, String mode) throws RemoteException,
FileNotFoundException {
return MockContentProvider.this.openFile(url, mode);
}
- @SuppressWarnings("unused")
+ @Override
public Cursor query(Uri url, String[] projection, String selection, String[] selectionArgs,
String sortOrder) throws RemoteException {
return MockContentProvider.this.query(url, projection, selection,
selectionArgs, sortOrder);
}
- @SuppressWarnings("unused")
+ @Override
public int update(Uri url, ContentValues values, String selection, String[] selectionArgs)
throws RemoteException {
return MockContentProvider.this.update(url, values, selection, selectionArgs);
}
- /**
- * @hide
- */
- @SuppressWarnings("unused")
+ @Override
public Bundle call(String method, String request, Bundle args)
throws RemoteException {
return MockContentProvider.this.call(method, request, args);
}
+ @Override
public IBinder asBinder() {
throw new UnsupportedOperationException();
}
- @SuppressWarnings("unused")
+ @Override
public String[] getStreamTypes(Uri url, String mimeTypeFilter) throws RemoteException {
return MockContentProvider.this.getStreamTypes(url, mimeTypeFilter);
}
- @SuppressWarnings("unused")
+ @Override
public AssetFileDescriptor openTypedAssetFile(Uri url, String mimeType, Bundle opts)
throws RemoteException, FileNotFoundException {
return MockContentProvider.this.openTypedAssetFile(url, mimeType, opts);
diff --git a/test-runner/src/android/test/mock/MockIContentProvider.java b/test-runner/src/android/test/mock/MockIContentProvider.java
index 183be41..b7733a4 100644
--- a/test-runner/src/android/test/mock/MockIContentProvider.java
+++ b/test-runner/src/android/test/mock/MockIContentProvider.java
@@ -23,9 +23,6 @@
import android.content.IContentProvider;
import android.content.res.AssetFileDescriptor;
import android.database.Cursor;
-import android.database.CursorWindow;
-import android.database.IBulkCursor;
-import android.database.IContentObserver;
import android.net.Uri;
import android.os.Bundle;
import android.os.IBinder;
@@ -47,12 +44,6 @@
throw new UnsupportedOperationException("unimplemented mock method");
}
- public IBulkCursor bulkQuery(Uri url, String[] projection, String selection,
- String[] selectionArgs, String sortOrder, IContentObserver observer,
- CursorWindow window) {
- throw new UnsupportedOperationException("unimplemented mock method");
- }
-
@SuppressWarnings("unused")
public int delete(Uri url, String selection, String[] selectionArgs)
throws RemoteException {
diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContentProvider.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContentProvider.java
index 3835378..c91a3bf 100644
--- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContentProvider.java
+++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContentProvider.java
@@ -23,9 +23,6 @@
import android.content.OperationApplicationException;
import android.content.res.AssetFileDescriptor;
import android.database.Cursor;
-import android.database.CursorWindow;
-import android.database.IBulkCursor;
-import android.database.IContentObserver;
import android.net.Uri;
import android.os.Bundle;
import android.os.IBinder;
@@ -41,78 +38,84 @@
* TODO: never return null when the method is not supposed to. Return fake data instead.
*/
public final class BridgeContentProvider implements IContentProvider {
-
+ @Override
public ContentProviderResult[] applyBatch(ArrayList<ContentProviderOperation> arg0)
throws RemoteException, OperationApplicationException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public int bulkInsert(Uri arg0, ContentValues[] arg1) throws RemoteException {
// TODO Auto-generated method stub
return 0;
}
- public IBulkCursor bulkQuery(Uri arg0, String[] arg1, String arg2, String[] arg3,
- String arg4, IContentObserver arg5, CursorWindow arg6) throws RemoteException {
- // TODO Auto-generated method stub
- return null;
- }
-
+ @Override
public Bundle call(String arg0, String arg1, Bundle arg2) throws RemoteException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public int delete(Uri arg0, String arg1, String[] arg2) throws RemoteException {
// TODO Auto-generated method stub
return 0;
}
+ @Override
public String getType(Uri arg0) throws RemoteException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public Uri insert(Uri arg0, ContentValues arg1) throws RemoteException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public AssetFileDescriptor openAssetFile(Uri arg0, String arg1) throws RemoteException,
FileNotFoundException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public ParcelFileDescriptor openFile(Uri arg0, String arg1) throws RemoteException,
FileNotFoundException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public Cursor query(Uri arg0, String[] arg1, String arg2, String[] arg3, String arg4)
throws RemoteException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public int update(Uri arg0, ContentValues arg1, String arg2, String[] arg3)
throws RemoteException {
// TODO Auto-generated method stub
return 0;
}
+ @Override
public IBinder asBinder() {
// TODO Auto-generated method stub
return null;
}
+ @Override
public String[] getStreamTypes(Uri arg0, String arg1) throws RemoteException {
// TODO Auto-generated method stub
return null;
}
+ @Override
public AssetFileDescriptor openTypedAssetFile(Uri arg0, String arg1, Bundle arg2)
throws RemoteException, FileNotFoundException {
// TODO Auto-generated method stub