Merge "Fix ownership of CursorWindows across processes. Bug: 5332296"

core/java/android/content/ContentProvider.java

@@ -22,10 +22,6 @@ import android.content.pm.ProviderInfo;
 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 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
             return ContentProvider.this;
         }
 
-        /**
+        @Override
-         * Remote version of a query, which returns an IBulkCursor. The bulk
+        public String getProviderName() {
-         * cursor should be wrapped with BulkCursorToCursorAdaptor before use.
+            return getContentProvider().getClass().getName();
-         */
-        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);
         }
 
         public Cursor query(Uri uri, String[] projection,

core/java/android/content/ContentProviderNative.java

@@ -20,6 +20,7 @@ import android.content.res.AssetFileDescriptor;
 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 @@ abstract public class ContentProviderNative extends Binder implements IContentPr
         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 @@ abstract public class ContentProviderNative extends Binder implements IContentPr
                     }
 
                     String sortOrder = data.readString();
-                    IContentObserver observer = IContentObserver.Stub.
+                    IContentObserver observer = IContentObserver.Stub.asInterface(
-                        asInterface(data.readStrongBinder());
+                            data.readStrongBinder());
                     CursorWindow window = CursorWindow.CREATOR.createFromParcel(data);
 
-                    // Flag for whether caller wants the number of
+                    Cursor cursor = query(url, projection, selection, selectionArgs, sortOrder);
-                    // rows in the cursor and the position of the
+                    if (cursor != null) {
-                    // "_id" column index (or -1 if non-existent)
+                        CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor(
-                    // Only to be returned if binder != null.
+                                cursor, observer, getProviderName(), window);
-                    boolean wantsCursorMetadata = data.readInt() != 0;
+                        final IBinder binder = adaptor.asBinder();
+                        final int count = adaptor.count();
+                        final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
+                                adaptor.getColumnNames());
 
-                    IBulkCursor bulkCursor = bulkQuery(url, projection, selection,
+                        reply.writeNoException();
-                            selectionArgs, sortOrder, observer, window);
+                        reply.writeStrongBinder(binder);
-                    if (bulkCursor != null) {
+                        reply.writeInt(count);
-                        final IBinder binder = bulkCursor.asBinder();
+                        reply.writeInt(index);
-                        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);
-                        }
                     } else {
                         reply.writeNoException();
                         reply.writeStrongBinder(null);
@@ -324,92 +322,70 @@ final class ContentProviderProxy implements IContentProvider
         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();
+        try {
-        IBulkCursor bulkCursor = bulkQueryInternal(
+            BulkCursorToCursorAdaptor adaptor = new BulkCursorToCursorAdaptor();
-            url, projection, selection, selectionArgs, sortOrder,
+            Parcel data = Parcel.obtain();
-            adaptor.getObserver(), window,
+            Parcel reply = Parcel.obtain();
-            adaptor);
+            try {
-        if (bulkCursor == null) {
+                data.writeInterfaceToken(IContentProvider.descriptor);
-            return null;
+
+                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

core/java/android/content/IContentProvider.java

@@ -18,9 +18,6 @@ package android.content;
 
 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 @@ import java.util.ArrayList;
  * @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;

core/java/android/database/AbstractCursor.java

@@ -78,13 +78,11 @@ public abstract class AbstractCursor implements CrossProcessCursor {
     }
 
     public void deactivate() {
-        deactivateInternal();
+        onDeactivateOrClose();
     }
 
-    /**
+    /** @hide */
-     * @hide
+    protected void onDeactivateOrClose() {
-     */
-    public void deactivateInternal() {
         if (mSelfObserver != null) {
             mContentResolver.unregisterContentObserver(mSelfObserver);
             mSelfObserverRegistered = false;
@@ -108,7 +106,7 @@ public abstract class AbstractCursor implements CrossProcessCursor {
     public void close() {
         mClosed = true;
         mContentObservable.unregisterAll();
-        deactivateInternal();
+        onDeactivateOrClose();
     }
 
     /**

core/java/android/database/AbstractWindowedCursor.java

@@ -19,6 +19,11 @@ package android.database;
 /**
  * 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 @@ public abstract class AbstractWindowedCursor extends AbstractCursor {
             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();
+    }
 }

core/java/android/database/BulkCursorNative.java

@@ -62,13 +62,13 @@ public abstract class BulkCursorNative extends Binder implements IBulkCursor
                     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;
                 }
 

core/java/android/database/BulkCursorToCursorAdaptor.java

@@ -21,40 +21,24 @@ import android.os.RemoteException;
 import android.util.Log;
 
 /**
- * Adapts an {@link IBulkCursor} to a {@link Cursor} for use in the local
+ * Adapts an {@link IBulkCursor} to a {@link Cursor} for use in the local process.
- * 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
+     * Initializes the adaptor.
-     * already knows BulkCursorToCursorAdaptor's properties.
+     * 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 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
      *
      * @return A SelfContentObserver hooked up to this Cursor
      */
-    public synchronized IContentObserver getObserver() {
+    public IContentObserver getObserver() {
-        if (mObserverBridge == null) {
-            mObserverBridge = new SelfContentObserver(this);
-        }
         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 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
         // which is what actually makes the data set invalid.
         super.deactivate();
 
-        try {
+        if (mBulkCursor != null) {
-            mBulkCursor.deactivate();
+            try {
-        } catch (RemoteException ex) {
+                mBulkCursor.deactivate();
-            Log.w(TAG, "Remote process exception when deactivating");
+            } catch (RemoteException ex) {
+                Log.w(TAG, "Remote process exception when deactivating");
+            }
         }
-        mWindow = null;
     }
     
     @Override
     public void close() {
         super.close();
-        try {
+
-            mBulkCursor.close();
+        if (mBulkCursor != null) {
-        } catch (RemoteException ex) {
+            try {
-            Log.w(TAG, "Remote process exception when closing");
+                mBulkCursor.close();
+            } catch (RemoteException ex) {
+                Log.w(TAG, "Remote process exception when closing");
+            } finally {
+                mBulkCursor = null;
+            }
         }
-        mWindow = null;        
     }
 
     @Override
     public boolean requery() {
-        try {
+        throwIfCursorIsClosed();
-            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();
 
-                // super.requery() will call onChanged. Do it here instead of relying on the
+        try {
-                // observer from the far side so that observers can see a correct value for mCount
+            CursorWindow newWindow = new CursorWindow(false /* create a remote window */);
-                // when responding to onChanged.
+            try {
-                super.requery();
+                mCount = mBulkCursor.requery(getObserver(), newWindow);
-                return true;
+                if (mCount != -1) {
-            } else {
+                    mPos = -1;
-                deactivate();
+                    closeWindow();
-                return false;
+
+                    // 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 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
 
     @Override
     public String[] getColumnNames() {
+        throwIfCursorIsClosed();
+
         if (mColumns == null) {
             try {
                 mColumns = mBulkCursor.getColumnNames();
@@ -187,6 +189,8 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
 
     @Override
     public Bundle getExtras() {
+        throwIfCursorIsClosed();
+
         try {
             return mBulkCursor.getExtras();
         } catch (RemoteException e) {
@@ -198,6 +202,8 @@ public final class BulkCursorToCursorAdaptor extends AbstractWindowedCursor {
 
     @Override
     public Bundle respond(Bundle extras) {
+        throwIfCursorIsClosed();
+
         try {
             return mBulkCursor.respond(extras);
         } catch (RemoteException e) {

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
      */

core/java/android/database/CursorToBulkCursorAdaptor.java

@@ -24,19 +24,37 @@ import android.util.Log;
 
 /**
  * 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 final class CursorToBulkCursorAdaptor extends BulkCursorNative
     }
 
     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 @@ public final class CursorToBulkCursorAdaptor extends BulkCursorNative
                                 + 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();
+        synchronized (mLock) {
-        if (mWindow != null) {
+            closeCursorAndWindowLocked();
-            mWindow.close();
         }
     }
-    
+
+    @Override
     public CursorWindow getWindow(int startPos) {
-        mCursor.moveToPosition(startPos);
+        synchronized (mLock) {
-        
+            throwIfCursorIsClosed();
-        if (mWindow != null) {
-            if (startPos < mWindow.getStartPosition() ||
-                    startPos >= (mWindow.getStartPosition() + mWindow.getNumRows())) {
-                mCursor.fillWindow(startPos, mWindow);
-            }            
-            return mWindow;
-        } else {
-            return ((AbstractWindowedCursor)mCursor).getWindow();
-        }
-    }
 
-    public void onMove(int position) {
+            mCursor.moveToPosition(startPos);
-        mCursor.onMove(mCursor.getPosition(), position);
-    }
 
-    public int count() {
+            final CursorWindow window;
-        return mCursor.getCount();
+            if (mCursor instanceof AbstractWindowedCursor) {
-    }
+                window = ((AbstractWindowedCursor)mCursor).getWindow();
-
+            } else {
-    public String[] getColumnNames() {
+                window = mWindowForNonWindowedCursor;
-        return mCursor.getColumnNames();
+                if (window != null
-    }
+                        && (startPos < window.getStartPosition() ||
-
+                                startPos >= (window.getStartPosition() + window.getNumRows()))) {
-    public void deactivate() {
+                    mCursor.fillWindow(startPos, window);
-        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;
             }
-        } catch (IllegalStateException e) {
+
-            IllegalStateException leakProgram = new IllegalStateException(
+            // Acquire a reference before returning from this RPC.
-                    mProviderName + " Requery misuse db, mCursor isClosed:" +
+            // The Binder proxy will decrement the reference count again as part of writing
-                    mCursor.isClosed(), e);
+            // the CursorWindow to the reply parcel as a return value.
-            throw leakProgram;
+            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 @@ public final class CursorToBulkCursorAdaptor extends BulkCursorNative
      * @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 @@ public final class CursorToBulkCursorAdaptor extends BulkCursorNative
     }
 
     /** 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 @@ public final class CursorToBulkCursorAdaptor extends BulkCursorNative
         }
     }
 
+    @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);
+        }
     }
 }

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 @@ public class CursorWindow extends SQLiteClosable implements Parcelable {
 
     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 @@ public class CursorWindow extends SQLiteClosable implements Parcelable {
             throw new CursorWindowAllocationException("Cursor window allocation of " +
                     (sCursorWindowSize / 1024) + " kb failed. " + printStats());
         }
+        mCloseGuard.open("close");
         recordNewWindow(Binder.getCallingPid(), mWindowPtr);
     }
 
@@ -102,11 +107,15 @@ public class CursorWindow extends SQLiteClosable implements Parcelable {
             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 @@ public class CursorWindow extends SQLiteClosable implements Parcelable {
     }
 
     private void dispose() {
+        if (mCloseGuard != null) {
+            mCloseGuard.close();
+        }
         if (mWindowPtr != 0) {
             recordClosingOfWindow(mWindowPtr);
             nativeDispose(mWindowPtr);
@@ -677,6 +689,10 @@ public class CursorWindow extends SQLiteClosable implements Parcelable {
     public void writeToParcel(Parcel dest, int flags) {
         dest.writeStrongBinder(nativeGetBinder(mWindowPtr));
         dest.writeInt(mStartPos);
+
+        if ((flags & Parcelable.PARCELABLE_WRITE_RETURN_VALUE) != 0) {
+            releaseReference();
+        }
     }
 
     @Override

core/java/android/database/sqlite/SQLiteCursor.java

@@ -89,8 +89,6 @@ public class SQLiteCursor extends AbstractWindowedCursor {
      * @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 @@ public class SQLiteCursor extends AbstractWindowedCursor {
     }
 
     private void fillWindow(int startPos) {
-        if (mWindow == null) {
+        clearOrCreateLocalWindow();
-            // 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();
-        }
         mWindow.setStartPosition(startPos);
         int count = getQuery().fillWindow(mWindow);
         if (startPos == 0) { // fillWindow returns count(*) only for startPos = 0
@@ -214,16 +207,9 @@ public class SQLiteCursor extends AbstractWindowedCursor {
         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 class SQLiteCursor extends AbstractWindowedCursor {
     public void close() {
         super.close();
         synchronized (this) {
-            deactivateCommon();
             mQuery.close();
             mDriver.cursorClosed();
         }

test-runner/src/android/test/mock/MockContentProvider.java

@@ -21,16 +21,12 @@ import android.content.ContentProviderOperation;
 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 @@ public class MockContentProvider extends ContentProvider {
      * 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")
+        @Override
-        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")
         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);
         }
 
-        /**
+        @Override
-         * @hide
-         */
-        @SuppressWarnings("unused")
         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);

test-runner/src/android/test/mock/MockIContentProvider.java

@@ -23,9 +23,6 @@ import android.content.EntityIterator;
 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 @@ public class MockIContentProvider implements IContentProvider {
         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 {

tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContentProvider.java

@@ -23,9 +23,6 @@ import android.content.IContentProvider;
 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 @@ import java.util.ArrayList;
  * 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,
+    @Override
-            String arg4, IContentObserver arg5, CursorWindow arg6) throws RemoteException {
-        // TODO Auto-generated method stub
-        return null;
-    }
-
     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