IssueTracker

Project: platform/frameworks/support
Branch: androidx-main

commit e782987543a9f8ccd485e970ddc74564b24378db
Author: Vighnesh Raut <vighnesh.raut13@gmail.com>
Date:   Mon Jan 02 15:27:40 2023

    fix: tab row crashes when only 1 tab is added
   
    Bug: b/264018028
   
    Test: Added unit test
   
    Change-Id: I6381dbac304fc1d69d3708c6655f8b595668e93f

M       tv/tv-material/src/androidTest/java/androidx/tv/material/TabRowTest.kt
M       tv/tv-material/src/main/java/androidx/tv/material/TabRow.kt

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

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!