Skip to content

Commit 7ce7452

Browse files
author
Jeff Brown
committed
Clean up CursorWindow lifetime.
Bug: 5332296 Removed dead code in SQLiteCursor related to the use of a background query thread. This code could result in CursorWindows being modified concurrently or used after free. This code is broken, unused and is just in the way. Added comments to explain how CursorWindow ownership is supposed to work for AbstractWindowedCursors. (There are still cases where cursor windows get dropped on the floor without being closed. Those will be taken care of in a subsequent patch.) Cleaned up SQLiteQuery.fillWindow to eliminate duplicate code and remove bits that were only needed for background loading, like returning -1. Change-Id: I03e8e2e73ff0c11df76d63f57df4c5ada06ae1cb
1 parent d0ff68d commit 7ce7452

File tree

10 files changed

+186
-582
lines changed

10 files changed

+186
-582
lines changed

core/java/android/database/AbstractCursor.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ public byte[] getBlob(int column) {
6464
/* Methods that may optionally be implemented by subclasses */
6565

6666
/**
67-
* returns a pre-filled window, return NULL if no such window
67+
* If the cursor is backed by a {@link CursorWindow}, returns a pre-filled
68+
* window with the contents of the cursor, otherwise null.
69+
*
70+
* @return The pre-filled window that backs this cursor, or null if none.
6871
*/
6972
public CursorWindow getWindow() {
7073
return null;

core/java/android/database/AbstractWindowedCursor.java

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,22 @@
1818

1919
/**
2020
* A base class for Cursors that store their data in {@link CursorWindow}s.
21+
* <p>
22+
* Subclasses are responsible for filling the cursor window with data during
23+
* {@link #onMove(int, int)}, allocating a new cursor window if necessary.
24+
* During {@link #requery()}, the existing cursor window should be cleared and
25+
* filled with new data.
26+
* </p><p>
27+
* If the contents of the cursor change or become invalid, the old window must be closed
28+
* (because it is owned by the cursor) and set to null.
29+
* </p>
2130
*/
2231
public abstract class AbstractWindowedCursor extends AbstractCursor {
32+
/**
33+
* The cursor window owned by this cursor.
34+
*/
35+
protected CursorWindow mWindow;
36+
2337
@Override
2438
public byte[] getBlob(int columnIndex) {
2539
checkPosition();
@@ -126,25 +140,44 @@ protected void checkPosition() {
126140
public CursorWindow getWindow() {
127141
return mWindow;
128142
}
129-
143+
130144
/**
131-
* Set a new cursor window to cursor, usually set a remote cursor window
132-
* @param window cursor window
145+
* Sets a new cursor window for the cursor to use.
146+
* <p>
147+
* The cursor takes ownership of the provided cursor window; the cursor window
148+
* will be closed when the cursor is closed or when the cursor adopts a new
149+
* cursor window.
150+
* </p><p>
151+
* If the cursor previously had a cursor window, then it is closed when the
152+
* new cursor window is assigned.
153+
* </p>
154+
*
155+
* @param window The new cursor window, typically a remote cursor window.
133156
*/
134157
public void setWindow(CursorWindow window) {
135-
if (mWindow != null) {
136-
mWindow.close();
158+
if (window != mWindow) {
159+
closeWindow();
160+
mWindow = window;
137161
}
138-
mWindow = window;
139162
}
140-
163+
164+
/**
165+
* Returns true if the cursor has an associated cursor window.
166+
*
167+
* @return True if the cursor has an associated cursor window.
168+
*/
141169
public boolean hasWindow() {
142170
return mWindow != null;
143171
}
144172

145173
/**
146-
* This needs be updated in {@link #onMove} by subclasses, and
147-
* needs to be set to NULL when the contents of the cursor change.
174+
* Closes the cursor window and sets {@link #mWindow} to null.
175+
* @hide
148176
*/
149-
protected CursorWindow mWindow;
177+
protected void closeWindow() {
178+
if (mWindow != null) {
179+
mWindow.close();
180+
mWindow = null;
181+
}
182+
}
150183
}

core/java/android/database/BulkCursorToCursorAdaptor.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,7 @@ public boolean requery() {
154154
false /* the window will be accessed across processes */));
155155
if (mCount != -1) {
156156
mPos = -1;
157-
if (mWindow != null) {
158-
mWindow.close();
159-
mWindow = null;
160-
}
157+
closeWindow();
161158

162159
// super.requery() will call onChanged. Do it here instead of relying on the
163160
// observer from the far side so that observers can see a correct value for mCount

core/java/android/database/CursorWindow.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,12 @@ private CursorWindow(Parcel source) {
105105
}
106106

107107
@Override
108-
protected void finalize() {
109-
dispose();
108+
protected void finalize() throws Throwable {
109+
try {
110+
dispose();
111+
} finally {
112+
super.finalize();
113+
}
110114
}
111115

112116
private void dispose() {
@@ -145,21 +149,25 @@ public void clear() {
145149

146150
/**
147151
* Gets the start position of this cursor window.
148-
* The start position is the index of the first row that this window contains
152+
* <p>
153+
* The start position is the zero-based index of the first row that this window contains
149154
* relative to the entire result set of the {@link Cursor}.
155+
* </p>
150156
*
151-
* @return The start position.
157+
* @return The zero-based start position.
152158
*/
153159
public int getStartPosition() {
154160
return mStartPos;
155161
}
156162

157163
/**
158164
* Sets the start position of this cursor window.
159-
* The start position is the index of the first row that this window contains
165+
* <p>
166+
* The start position is the zero-based index of the first row that this window contains
160167
* relative to the entire result set of the {@link Cursor}.
168+
* </p>
161169
*
162-
* @param pos The new start position.
170+
* @param pos The new zero-based start position.
163171
*/
164172
public void setStartPosition(int pos) {
165173
mStartPos = pos;

core/java/android/database/CursorWrapper.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ public CursorWrapper(Cursor cursor) {
3333
}
3434

3535
/**
36-
* @return the wrapped cursor
36+
* Gets the underlying cursor that is wrapped by this instance.
37+
*
38+
* @return The wrapped cursor.
3739
*/
3840
public Cursor getWrappedCursor() {
3941
return mCursor;

0 commit comments

Comments
 (0)