IssueTracker

since these are in public API (:/) we need to do this in 1.2

Opening diff shortly

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d576cbdc911cba16638a44fd8223391a90a07ef7
Author: Mike Nakhimovich <digitalbuddha@users.noreply.github.com>
Date:   Tue Aug 11 09:30:34 2020

    [GH] Hide deprecated internal API.
   
    ## Proposed Changes
   
    * `RoomDatabase.java` has protected `mCallbacks` field which is leaking in the API docs, we should @Hide it.
   
    ## Testing
   
    Test: Ran unit tests locally
   
    ## Issues Fixed
   
    Fixes: 76109329
   
    This is an imported pull request from https://github.com/androidx/androidx/pull/61.
   
    Resolves #61
    Github-Pr-Head-Sha: 6440daa3a63752c7f9d5ba2a390248cd85bc634f
    GitOrigin-RevId: fe92d8466a59b44b218b6ca3cbd57dcda17992f7
   
    Change-Id: Id599cdf5b02b32bdae0166266fb7da967598fe92

A       room/runtime/api/current.ignore
M       room/runtime/api/current.txt
M       room/runtime/api/public_plus_experimental_current.txt
M       room/runtime/api/restricted_current.txt
M       room/runtime/src/main/java/androidx/room/RoomDatabase.java

https://android-review.googlesource.com/1396827

Sure thing, I'll try to repro it and send you guys a link.

Can't seem to reproduce it in a clean codebase, despite it happening constantly in my real codebase... I don't really see any holes in the logic that RoomSQLiteQuery is using to lock the pool and recycle queries, either. I don't have enough time to screw around with it for now, but I'll come back to this and try to reproduce ASAP.

this is very troubling yet I cannot understand how this happens. I mean, we might have a bug in recycling queries, yet for a Single query, the instance we give to it does not get recycled until the query returns.
below is some sample code generated from a test that returns Single<List>.

@Override
  public Single<List<QueryTransactionTest.Entity1>> single() {
    final String _sql = "select * from Entity1";
    final RoomSQLiteQuery _statement = RoomSQLiteQuery.acquire(_sql, 0);
    return Single.fromCallable(new Callable<List<QueryTransactionTest.Entity1>>() {
      @Override
      public List<QueryTransactionTest.Entity1> call() throws Exception {
        final Cursor _cursor = __db.query(_statement);
        try {
          final int _cursorIndexOfId = _cursor.getColumnIndexOrThrow("id");
          final int _cursorIndexOfValue = _cursor.getColumnIndexOrThrow("value");
          final java.util.List<android.arch.persistence.room.integration.testapp.test.QueryTransactionTest.Entity1> _result = new java.util.ArrayList<android.arch.persistence.room.integration.testapp.test.QueryTransactionTest.Entity1>(_cursor.getCount());
          while(_cursor.moveToNext()) {
            final android.arch.persistence.room.integration.testapp.test.QueryTransactionTest.Entity1 _item;
            final int _tmpId;
            _tmpId = _cursor.getInt(_cursorIndexOfId);
            final java.lang.String _tmpValue;
            _tmpValue = _cursor.getString(_cursorIndexOfValue);
            _item = new android.arch.persistence.room.integration.testapp.test.QueryTransactionTest.Entity1(_tmpId,_tmpValue);
            _result.add(_item);
          }
          if(_result == null) {
            throw new EmptyResultSetException("Query returned empty result set: " + _statement.getSql());
          }
          return _result;
        } finally {
          _cursor.close();
          _statement.release();
        }
      }
    });
  }

An option could be that limit offset data source releasing queries that it wants to re-use. But if the count is coming from there, here is the generated count code from a query.

public int countItems() {
        final RoomSQLiteQuery sqLiteQuery = RoomSQLiteQuery.acquire(mCountQuery,
                mSourceQuery.getArgCount());
        sqLiteQuery.copyArgumentsFrom(mSourceQuery);
        Cursor cursor = mDb.query(sqLiteQuery);
        try {
            if (cursor.moveToFirst()) {
                return cursor.getInt(0);
            }
            return 0;
        } finally {
            cursor.close();
            sqLiteQuery.release();
        }
    }

it again obtains and releases the query in succession so I can't see how it happened to leak it.

I looked over the source that Room generated for my specific case and it all looked totally fine to me, too... I couldn't think of a way this would occur. However, the screenshot is pretty telling and it lines up with the exception I get. My temp "fix" involves just re-issuing the same query and it works fine (because the first failed attempt corrects the condition, despite eventually failing itself).

I'm swamped and under a major deadline for the next week or so, but I'll take a look at this again after that.

oh stupid me. the bug is actually right there in #8.

You are holding onto the Single so first time you call, it executes and releases the statement, second time you call, it does not allocate a new one :/

Basically this should never release a statement (unless we override finalize in that callable).
Thanks for the report. I'll figure out a way to audit all of these calls.

repro test:

@Test
    public void singleUser_keepSingleReference() throws InterruptedException {
        User[] users = TestUtil.createUsersArray(1, 2);
        mUserDao.insertAll(users);
        TestObserver<User> testObserver1 = new TestObserver<>();
        Single<User> userSingle1 = mUserDao.singleUserById(1);
        Disposable disposable1 = userSingle1.observeOn(mTestScheduler)
                .subscribeWith(testObserver1);
        drain();
        testObserver1.assertComplete();
        testObserver1.assertValue(users[0]);
        disposable1.dispose();

        // how get single for 2
        TestObserver<User> testObserver2 = new TestObserver<>();
        Single<User> userSingle2 = mUserDao.singleUserById(2);
        Disposable disposable2 = userSingle1.observeOn(mTestScheduler)
                .subscribeWith(testObserver2);
        drain();
        testObserver2.assertComplete();
        testObserver2.assertValue(users[1]);
        disposable2.dispose();

        // now re-use the first single
        TestObserver<User> testObserver3 = new TestObserver<>();
        Disposable disposable3 = userSingle1.observeOn(mTestScheduler)
                .subscribeWith(testObserver3);
        drain();
        testObserver3.assertComplete();
        testObserver3.assertValue(users[0]);
        disposable3.dispose();
    }

Solid, thanks!