StrictMode complaining, but i'm not sure if i have anything to worry about.

[briandilley]

So i created a class called CursorListAdapter that extends from AbstractList and uses a Cursor as it's backing:

public class CursorListAdapter<T>
        extends AbstractList<T> {

    private Cursor cursor;
    private Func1<Cursor, T> mapper;

    public CursorListAdapter(Func1<Cursor, T> mapper) {
        this.mapper = mapper;
    }

    public CursorListAdapter(Cursor cursor, Func1<Cursor, T> mapper) {
        this.cursor = cursor;
        this.mapper = mapper;
    }

    public void setCursor(Cursor cursor) {
        this.cursor = cursor;
    }

    public synchronized void closeCursor() {
        if (cursor!=null && !cursor.isClosed()) {
            this.cursor.close();;
        }
    }

    public Cursor getCursor() {
        return this.cursor;
    }

    @Override
    public T get(int location) {
        if (cursor==null) {
            throw new IndexOutOfBoundsException("cursor is null");
        } else if (cursor.isClosed()) {
            throw new IndexOutOfBoundsException("cursor is closed");
        }
        if (!cursor.moveToPosition(location)) {
            throw new IndexOutOfBoundsException("moveToPosition failed");
        }
        return mapper.call(cursor);
    }

    @Override
    public int size() {
        return cursor!=null && !cursor.isClosed()
                ? cursor.getCount()
                : 0;
    }
}

and then i created an Observable.Operator to lift(...) Observable<SqlBrite.Query>s into an Observable of the given type:

public class CursorListAdapterOperator<T>
    implements Observable.Operator<List<T>, SqlBrite.Query> {

    private final Func1<Cursor, T> mapper;

    public CursorListAdapterOperator(Func1<Cursor, T> mapper) {
        this.mapper = mapper;
    }

    @Override
    public Subscriber<? super SqlBrite.Query> call(final Subscriber<? super List<T>> subscriber) {
        return new Subscriber<SqlBrite.Query>(subscriber) {

            final CursorListAdapter<T> list = new CursorListAdapter<>(mapper);

            @Override
            public void onNext(SqlBrite.Query query) {
                try {
                    list.closeCursor();
                    list.setCursor(query.run());
                    if (!subscriber.isUnsubscribed()) {
                        subscriber.onNext(list);
                    }
                } catch (Throwable e) {
                    Exceptions.throwIfFatal(e);
                    onError(OnErrorThrowable.addValueAsLastCause(e, query.toString()));
                }
            }

            @Override
            public void onCompleted() {
                subscriber.onCompleted();
                list.closeCursor();
            }

            @Override
            public void onError(Throwable e) {
                subscriber.onError(e);
                list.closeCursor();
            }
        };
    }
}

I see a StrictMode warning in my console about not calling close on the cursor and I'm wondering if that's something i need to worry about or not. Here's the warning:

    flipagram E/StrictMode: A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
    java.lang.Throwable: Explicit termination method 'close' not called
       at dalvik.system.CloseGuard.open(CloseGuard.java:180)
       at android.content.ContentResolver$CursorWrapperInner.<init>(ContentResolver.java:2507)
       at android.content.ContentResolver.query(ContentResolver.java:515)
       at android.content.ContentResolver.query(ContentResolver.java:434)
       at com.squareup.sqlbrite.BriteContentResolver$1.run(BriteContentResolver.java:83)
       at XXXX.CursorListAdapterOperator$1.onNext(CursorListAdapterOperator.java:41)
       at XXXX.CursorListAdapterOperator$1.onNext(CursorListAdapterOperator.java:32)
       at com.squareup.sqlbrite.BackpressureBufferLastOperator$BufferLastSubscriber.onNext(BackpressureBufferLastOperator.java:101)

I'm relatively new to RxJava and SqlBrite - but my Operator should be properly closing the Cursor right?

[JakeWharton]

You need to try / finally the calls to the downstream subscriber in onCompleted and onError at minimum. They might throw and leak the cursor.

That said, you want to be very careful about putting the Cursor into the stream directly (even if wrapped in this object) since it's very easy to create leaks and even easier to get a closed cursor. This is the whole reason the Query type exists. Calls to combineLatest hide the fact they buffer data and can return (in the sense of cursors) stale and thus closed instances. Not to mention the fact that the threading model prevents you from getting the convenience of try/finally blocks to always always always ensure the cursor is closed.

[briandilley]

I guess my problem then would be that It's hard\impossible to bind a RecyclerView to a database result in a way that doesn't involve bringing all of the records into memory... any ideas on that?

[briandilley]

I guess if the Observable at the end of my chain emits the Query to the subscriber i could then just wrap the Cursor that comes from it - i think i see what you're getting at now.

[JakeWharton]

Yes. That would isolate interaction to a single, synchronous method where you can not only guarantee that it's always closed, but it frees you from having to worry about where threading happens in the chain and whether or not you are accidentally leaking an instance with an operator that buffers values.

[briandilley]

Cool, thanks for the help.

[ginosian]

@briandilley please can you provide corrected version of your code?
Thanks

[briandilley]

@ginosian we use the class below to wrap a Cursor in a List so that our RecyclerView can use it as if it were just a normal List (this makes testing easy as well since you can test with an ArrayList). The only other thing then is making sure that you close the Cursor when the Activity is destroyed (or whenever is appropriate). You can also call the replaceCursor method on the class below passing it a null and it will close the Cursor. So basically what we do is in our subscribe method we call query.run() on the main thread and pass the result (a Cursor) to this List object. The point that @JakeWharton was trying to make was that you don't want the Cursor object itself being held onto by something in RxJava, so it's better not to pass it down a stream. That's why he created the Query object, and when you're ready for the data you call run() on it to get the Cursor.

import android.database.ContentObserver;
import android.database.Cursor;
import android.database.DataSetObserver;
import android.net.Uri;
import android.util.SparseArray;

import java.util.AbstractList;

import rx.functions.Func1;

/**
 * An implementation of List that is backed by an Android Cursor.
 */
public class CursorListAdapter<T>
        extends AbstractList<T> {

    private static final String TAG = "FG/CursorListAdapter";

    private final Func1<Cursor, T> mapper;
    private final DataSetObserver dataSetObserver;
    private final ContentObserver contentObserver;

    private Cursor cursor;
    private SparseArray<T> cache; // TODO: replace with LRU

    public CursorListAdapter(Func1<Cursor, T> mapper) {
        this.mapper = mapper;
        this.dataSetObserver = new DataSetObserver() {
            @Override
            public void onChanged() {
                Log.d(TAG, "DataSetObserver.onChanged("+cursor.hashCode()+")");
                if (cache != null) {
                    cache.clear();
                }
            }
            @Override
            public void onInvalidated() {
                Log.d(TAG, "DataSetObserver.onInvalidated("+cursor.hashCode()+")");
                replaceCursor(null);
            }
        };
        this.contentObserver = new ContentObserver(null) {
            @Override
            public void onChange(boolean selfChange) {
                Log.d(TAG, "ContentObserver.onChanged("+cursor.hashCode()+")");
                if (cache != null) {
                    cache.clear();
                }
            }
            @Override
            public void onChange(boolean selfChange, Uri uri) {
                Log.d(TAG, "ContentObserver.onChanged("+cursor.hashCode()+")");
                if (cache != null) {
                    cache.clear();
                }
            }
        };
    }

    public synchronized Cursor getCursor() {
        return this.cursor;
    }

    public synchronized SparseArray<T> getCache() {
        return this.cache;
    }

    public synchronized void replaceCursor(Cursor cursor) {
        Cursor oldCursor = this.cursor;
        this.cursor = cursor;
        if (oldCursor != null) {
            oldCursor.unregisterDataSetObserver(dataSetObserver);
            oldCursor.unregisterContentObserver(contentObserver);
            if (!oldCursor.isClosed()) {
                oldCursor.close();
            }
        }
        if (cursor != null) {
            cache = new SparseArray<>(cursor.getCount());
            cursor.registerDataSetObserver(dataSetObserver);
            cursor.registerContentObserver(contentObserver);
        } else {
            cache = null;
        }
    }

    @Override
    public synchronized T get(int location) {
        T ret = cache.get(location);
        if (ret == null) {
            if (cursor == null) {
                throw new IndexOutOfBoundsException("cursor is null");
            } else if (cursor.isClosed()) {
                throw new IndexOutOfBoundsException("cursor is closed");
            } else if (!cursor.moveToPosition(location)) {
                throw new IndexOutOfBoundsException("moveToPosition failed: " + location);
            }
            ret = mapper.call(cursor);
            cache.put(location, ret);
        }
        return ret;
    }

    @Override
    public synchronized int size() {
        return cursor != null && !cursor.isClosed()
                ? cursor.getCount()
                : 0;
    }

    @Override
    public synchronized boolean isEmpty() {
        return cursor == null
                || cursor.isClosed()
                || cursor.getCount() <= 0;
    }
}

[ginosian]

@briandilley Thank you very much. Excellent explanation. I was looking for this kind of solution for almost 15 days.