Skip to content

Commit 184b44e

Browse files
Jeff BrownAndroid (Google) Code Review
Jeff Brown
authored and
Android (Google) Code Review
committed
Merge "Fix ownership of CursorWindows across processes. Bug: 5332296"
2 parents 1d805c2 + d218365 commit 184b44e

File tree

14 files changed

+376
-310
lines changed

14 files changed

+376
-310
lines changed

‎core/java/android/content/ContentProvider.java‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@
2222
import android.content.res.AssetFileDescriptor;
2323
import android.content.res.Configuration;
2424
import android.database.Cursor;
25-
import android.database.CursorToBulkCursorAdaptor;
26-
import android.database.CursorWindow;
27-
import android.database.IBulkCursor;
28-
import android.database.IContentObserver;
2925
import android.database.SQLException;
3026
import android.net.Uri;
3127
import android.os.AsyncTask;
@@ -168,22 +164,9 @@ ContentProvider getContentProvider() {
168164
return ContentProvider.this;
169165
}
170166

171-
/**
172-
* Remote version of a query, which returns an IBulkCursor. The bulk
173-
* cursor should be wrapped with BulkCursorToCursorAdaptor before use.
174-
*/
175-
public IBulkCursor bulkQuery(Uri uri, String[] projection,
176-
String selection, String[] selectionArgs, String sortOrder,
177-
IContentObserver observer, CursorWindow window) {
178-
enforceReadPermission(uri);
179-
Cursor cursor = ContentProvider.this.query(uri, projection,
180-
selection, selectionArgs, sortOrder);
181-
if (cursor == null) {
182-
return null;
183-
}
184-
return new CursorToBulkCursorAdaptor(cursor, observer,
185-
ContentProvider.this.getClass().getName(),
186-
hasWritePermission(uri), window);
167+
@Override
168+
public String getProviderName() {
169+
return getContentProvider().getClass().getName();
187170
}
188171

189172
public Cursor query(Uri uri, String[] projection,

‎core/java/android/content/ContentProviderNative.java‎

Lines changed: 77 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import android.database.BulkCursorNative;
2121
import android.database.BulkCursorToCursorAdaptor;
2222
import android.database.Cursor;
23+
import android.database.CursorToBulkCursorAdaptor;
2324
import android.database.CursorWindow;
2425
import android.database.DatabaseUtils;
2526
import android.database.IBulkCursor;
@@ -65,6 +66,13 @@ static public IContentProvider asInterface(IBinder obj)
6566
return new ContentProviderProxy(obj);
6667
}
6768

69+
/**
70+
* Gets the name of the content provider.
71+
* Should probably be part of the {@link IContentProvider} interface.
72+
* @return The content provider name.
73+
*/
74+
public abstract String getProviderName();
75+
6876
@Override
6977
public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
7078
throws RemoteException {
@@ -98,33 +106,23 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
98106
}
99107

100108
String sortOrder = data.readString();
101-
IContentObserver observer = IContentObserver.Stub.
102-
asInterface(data.readStrongBinder());
109+
IContentObserver observer = IContentObserver.Stub.asInterface(
110+
data.readStrongBinder());
103111
CursorWindow window = CursorWindow.CREATOR.createFromParcel(data);
104112

105-
// Flag for whether caller wants the number of
106-
// rows in the cursor and the position of the
107-
// "_id" column index (or -1 if non-existent)
108-
// Only to be returned if binder != null.
109-
boolean wantsCursorMetadata = data.readInt() != 0;
110-
111-
IBulkCursor bulkCursor = bulkQuery(url, projection, selection,
112-
selectionArgs, sortOrder, observer, window);
113-
if (bulkCursor != null) {
114-
final IBinder binder = bulkCursor.asBinder();
115-
if (wantsCursorMetadata) {
116-
final int count = bulkCursor.count();
117-
final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
118-
bulkCursor.getColumnNames());
119-
120-
reply.writeNoException();
121-
reply.writeStrongBinder(binder);
122-
reply.writeInt(count);
123-
reply.writeInt(index);
124-
} else {
125-
reply.writeNoException();
126-
reply.writeStrongBinder(binder);
127-
}
113+
Cursor cursor = query(url, projection, selection, selectionArgs, sortOrder);
114+
if (cursor != null) {
115+
CursorToBulkCursorAdaptor adaptor = new CursorToBulkCursorAdaptor(
116+
cursor, observer, getProviderName(), window);
117+
final IBinder binder = adaptor.asBinder();
118+
final int count = adaptor.count();
119+
final int index = BulkCursorToCursorAdaptor.findRowIdColumnIndex(
120+
adaptor.getColumnNames());
121+
122+
reply.writeNoException();
123+
reply.writeStrongBinder(binder);
124+
reply.writeInt(count);
125+
reply.writeInt(index);
128126
} else {
129127
reply.writeNoException();
130128
reply.writeStrongBinder(null);
@@ -324,92 +322,70 @@ public IBinder asBinder()
324322
return mRemote;
325323
}
326324

327-
// Like bulkQuery() but sets up provided 'adaptor' if not null.
328-
private IBulkCursor bulkQueryInternal(
329-
Uri url, String[] projection,
330-
String selection, String[] selectionArgs, String sortOrder,
331-
IContentObserver observer, CursorWindow window,
332-
BulkCursorToCursorAdaptor adaptor) throws RemoteException {
333-
Parcel data = Parcel.obtain();
334-
Parcel reply = Parcel.obtain();
325+
public Cursor query(Uri url, String[] projection, String selection,
326+
String[] selectionArgs, String sortOrder) throws RemoteException {
327+
CursorWindow window = new CursorWindow(false /* window will be used remotely */);
335328
try {
336-
data.writeInterfaceToken(IContentProvider.descriptor);
337-
338-
url.writeToParcel(data, 0);
339-
int length = 0;
340-
if (projection != null) {
341-
length = projection.length;
342-
}
343-
data.writeInt(length);
344-
for (int i = 0; i < length; i++) {
345-
data.writeString(projection[i]);
346-
}
347-
data.writeString(selection);
348-
if (selectionArgs != null) {
349-
length = selectionArgs.length;
350-
} else {
351-
length = 0;
352-
}
353-
data.writeInt(length);
354-
for (int i = 0; i < length; i++) {
355-
data.writeString(selectionArgs[i]);
356-
}
357-
data.writeString(sortOrder);
358-
data.writeStrongBinder(observer.asBinder());
359-
window.writeToParcel(data, 0);
360-
361-
// Flag for whether or not we want the number of rows in the
362-
// cursor and the position of the "_id" column index (or -1 if
363-
// non-existent). Only to be returned if binder != null.
364-
final boolean wantsCursorMetadata = (adaptor != null);
365-
data.writeInt(wantsCursorMetadata ? 1 : 0);
366-
367-
mRemote.transact(IContentProvider.QUERY_TRANSACTION, data, reply, 0);
329+
BulkCursorToCursorAdaptor adaptor = new BulkCursorToCursorAdaptor();
330+
Parcel data = Parcel.obtain();
331+
Parcel reply = Parcel.obtain();
332+
try {
333+
data.writeInterfaceToken(IContentProvider.descriptor);
334+
335+
url.writeToParcel(data, 0);
336+
int length = 0;
337+
if (projection != null) {
338+
length = projection.length;
339+
}
340+
data.writeInt(length);
341+
for (int i = 0; i < length; i++) {
342+
data.writeString(projection[i]);
343+
}
344+
data.writeString(selection);
345+
if (selectionArgs != null) {
346+
length = selectionArgs.length;
347+
} else {
348+
length = 0;
349+
}
350+
data.writeInt(length);
351+
for (int i = 0; i < length; i++) {
352+
data.writeString(selectionArgs[i]);
353+
}
354+
data.writeString(sortOrder);
355+
data.writeStrongBinder(adaptor.getObserver().asBinder());
356+
window.writeToParcel(data, 0);
368357

369-
DatabaseUtils.readExceptionFromParcel(reply);
358+
mRemote.transact(IContentProvider.QUERY_TRANSACTION, data, reply, 0);
370359

371-
IBulkCursor bulkCursor = null;
372-
IBinder bulkCursorBinder = reply.readStrongBinder();
373-
if (bulkCursorBinder != null) {
374-
bulkCursor = BulkCursorNative.asInterface(bulkCursorBinder);
360+
DatabaseUtils.readExceptionFromParcel(reply);
375361

376-
if (wantsCursorMetadata) {
362+
IBulkCursor bulkCursor = BulkCursorNative.asInterface(reply.readStrongBinder());
363+
if (bulkCursor != null) {
377364
int rowCount = reply.readInt();
378365
int idColumnPosition = reply.readInt();
379-
if (bulkCursor != null) {
380-
adaptor.set(bulkCursor, rowCount, idColumnPosition);
381-
}
366+
adaptor.initialize(bulkCursor, rowCount, idColumnPosition);
367+
} else {
368+
adaptor.close();
369+
adaptor = null;
382370
}
371+
return adaptor;
372+
} catch (RemoteException ex) {
373+
adaptor.close();
374+
throw ex;
375+
} catch (RuntimeException ex) {
376+
adaptor.close();
377+
throw ex;
378+
} finally {
379+
data.recycle();
380+
reply.recycle();
383381
}
384-
return bulkCursor;
385382
} finally {
386-
data.recycle();
387-
reply.recycle();
388-
}
389-
}
390-
391-
public IBulkCursor bulkQuery(Uri url, String[] projection,
392-
String selection, String[] selectionArgs, String sortOrder, IContentObserver observer,
393-
CursorWindow window) throws RemoteException {
394-
return bulkQueryInternal(
395-
url, projection, selection, selectionArgs, sortOrder,
396-
observer, window,
397-
null /* BulkCursorToCursorAdaptor */);
398-
}
399-
400-
public Cursor query(Uri url, String[] projection, String selection,
401-
String[] selectionArgs, String sortOrder) throws RemoteException {
402-
//TODO make a pool of windows so we can reuse memory dealers
403-
CursorWindow window = new CursorWindow(false /* window will be used remotely */);
404-
BulkCursorToCursorAdaptor adaptor = new BulkCursorToCursorAdaptor();
405-
IBulkCursor bulkCursor = bulkQueryInternal(
406-
url, projection, selection, selectionArgs, sortOrder,
407-
adaptor.getObserver(), window,
408-
adaptor);
409-
if (bulkCursor == null) {
410-
return null;
383+
// We close the window now because the cursor adaptor does not
384+
// take ownership of the window until the first call to onMove.
385+
// The adaptor will obtain a fresh reference to the window when
386+
// it is filled.
387+
window.close();
411388
}
412-
return adaptor;
413389
}
414390

415391
public String getType(Uri url) throws RemoteException

‎core/java/android/content/IContentProvider.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818

1919
import android.content.res.AssetFileDescriptor;
2020
import android.database.Cursor;
21-
import android.database.CursorWindow;
22-
import android.database.IBulkCursor;
23-
import android.database.IContentObserver;
2421
import android.net.Uri;
2522
import android.os.Bundle;
2623
import android.os.IBinder;
@@ -36,13 +33,6 @@
3633
* @hide
3734
*/
3835
public interface IContentProvider extends IInterface {
39-
/**
40-
* @hide - hide this because return type IBulkCursor and parameter
41-
* IContentObserver are system private classes.
42-
*/
43-
public IBulkCursor bulkQuery(Uri url, String[] projection,
44-
String selection, String[] selectionArgs, String sortOrder, IContentObserver observer,
45-
CursorWindow window) throws RemoteException;
4636
public Cursor query(Uri url, String[] projection, String selection,
4737
String[] selectionArgs, String sortOrder) throws RemoteException;
4838
public String getType(Uri url) throws RemoteException;

‎core/java/android/database/AbstractCursor.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,11 @@ public int getColumnCount() {
7878
}
7979

8080
public void deactivate() {
81-
deactivateInternal();
81+
onDeactivateOrClose();
8282
}
8383

84-
/**
85-
* @hide
86-
*/
87-
public void deactivateInternal() {
84+
/** @hide */
85+
protected void onDeactivateOrClose() {
8886
if (mSelfObserver != null) {
8987
mContentResolver.unregisterContentObserver(mSelfObserver);
9088
mSelfObserverRegistered = false;
@@ -108,7 +106,7 @@ public boolean isClosed() {
108106
public void close() {
109107
mClosed = true;
110108
mContentObservable.unregisterAll();
111-
deactivateInternal();
109+
onDeactivateOrClose();
112110
}
113111

114112
/**

‎core/java/android/database/AbstractWindowedCursor.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
/**
2020
* A base class for Cursors that store their data in {@link CursorWindow}s.
2121
* <p>
22+
* The cursor owns the cursor window it uses. When the cursor is closed,
23+
* its window is also closed. Likewise, when the window used by the cursor is
24+
* changed, its old window is closed. This policy of strict ownership ensures
25+
* that cursor windows are not leaked.
26+
* </p><p>
2227
* Subclasses are responsible for filling the cursor window with data during
2328
* {@link #onMove(int, int)}, allocating a new cursor window if necessary.
2429
* During {@link #requery()}, the existing cursor window should be cleared and
@@ -180,4 +185,25 @@ protected void closeWindow() {
180185
mWindow = null;
181186
}
182187
}
188+
189+
/**
190+
* If there is a window, clear it.
191+
* Otherwise, creates a local window.
192+
* @hide
193+
*/
194+
protected void clearOrCreateLocalWindow() {
195+
if (mWindow == null) {
196+
// If there isn't a window set already it will only be accessed locally
197+
mWindow = new CursorWindow(true /* the window is local only */);
198+
} else {
199+
mWindow.clear();
200+
}
201+
}
202+
203+
/** @hide */
204+
@Override
205+
protected void onDeactivateOrClose() {
206+
super.onDeactivateOrClose();
207+
closeWindow();
208+
}
183209
}

‎core/java/android/database/BulkCursorNative.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,13 @@ public boolean onTransact(int code, Parcel data, Parcel reply, int flags)
6262
data.enforceInterface(IBulkCursor.descriptor);
6363
int startPos = data.readInt();
6464
CursorWindow window = getWindow(startPos);
65+
reply.writeNoException();
6566
if (window == null) {
6667
reply.writeInt(0);
67-
return true;
68+
} else {
69+
reply.writeInt(1);
70+
window.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
6871
}
69-
reply.writeNoException();
70-
reply.writeInt(1);
71-
window.writeToParcel(reply, Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
7272
return true;
7373
}
7474

0 commit comments

Comments
 (0)