Fixed
Status Update
Comments
an...@gmail.com <an...@gmail.com> #2
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 3c819141d3be0a6194a6d02e5642942ac15d86af
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Mon Feb 04 10:39:54 2019
Use JetBrains's Kotlin metadata library.
Use kotlinx-metadata-jvm to read Kotlin's Metadata annotation
information. This library has less of an API surface than kotlin-metdata
meaning KotlinClassMetadataUtils only exposes what Room needs. However
it does allows us to fix the JVM descriptors (JvmDescriptorUtils) that
are needed to match annotation processing elements to kotlin metadata
elements.
Bug: 123767877
Test: ./gradlew room:room-compiler:test \
room:integration-tests:room-testapp-kotlin:cC \
room:integration-tests:room-testapp:cC
Change-Id: Iddbe3968865b32168144294e55765a77320a40da
M buildSrc/src/main/kotlin/androidx/build/dependencies/Dependencies.kt
M room/compiler/build.gradle
D room/compiler/src/main/kotlin/androidx/room/ext/KotlinMetadataElement.kt
M room/compiler/src/main/kotlin/androidx/room/ext/element_ext.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/JvmDescriptorUtils.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/KotlinClassMetadataUtils.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/KotlinMetadataElement.kt
M room/compiler/src/main/kotlin/androidx/room/processor/MethodProcessorDelegate.kt
M room/compiler/src/main/kotlin/androidx/room/processor/PojoProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/writer/DaoWriter.kt
A room/compiler/src/test/kotlin/androidx/room/kotlin/JvmDescriptorUtilsTest.kt
A room/compiler/src/test/kotlin/androidx/room/kotlin/KotlinMetadataElementTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt
A room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/AnswerConverter.kt
https://android-review.googlesource.com/894734
https://goto.google.com/android-sha1/3c819141d3be0a6194a6d02e5642942ac15d86af
Branch: androidx-master-dev
commit 3c819141d3be0a6194a6d02e5642942ac15d86af
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Mon Feb 04 10:39:54 2019
Use JetBrains's Kotlin metadata library.
Use kotlinx-metadata-jvm to read Kotlin's Metadata annotation
information. This library has less of an API surface than kotlin-metdata
meaning KotlinClassMetadataUtils only exposes what Room needs. However
it does allows us to fix the JVM descriptors (JvmDescriptorUtils) that
are needed to match annotation processing elements to kotlin metadata
elements.
Bug: 123767877
Test: ./gradlew room:room-compiler:test \
room:integration-tests:room-testapp-kotlin:cC \
room:integration-tests:room-testapp:cC
Change-Id: Iddbe3968865b32168144294e55765a77320a40da
M buildSrc/src/main/kotlin/androidx/build/dependencies/Dependencies.kt
M room/compiler/build.gradle
D room/compiler/src/main/kotlin/androidx/room/ext/KotlinMetadataElement.kt
M room/compiler/src/main/kotlin/androidx/room/ext/element_ext.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/JvmDescriptorUtils.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/KotlinClassMetadataUtils.kt
A room/compiler/src/main/kotlin/androidx/room/kotlin/KotlinMetadataElement.kt
M room/compiler/src/main/kotlin/androidx/room/processor/MethodProcessorDelegate.kt
M room/compiler/src/main/kotlin/androidx/room/processor/PojoProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/writer/DaoWriter.kt
A room/compiler/src/test/kotlin/androidx/room/kotlin/JvmDescriptorUtilsTest.kt
A room/compiler/src/test/kotlin/androidx/room/kotlin/KotlinMetadataElementTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt
A room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/vo/AnswerConverter.kt
yb...@google.com <yb...@google.com>
da...@google.com <da...@google.com> #3
It looks like Room 2.1.0 is not using androidx.sqlite:sqlite-framework:2.0.1 which should contain a fix for the situation you are just describing. The snippet of code you pasted for getWrappedDb() is different than what is in the head of the repo. See: https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/persistence/db-framework/src/main/java/androidx/sqlite/db/framework/FrameworkSQLiteOpenHelper.java#152
an...@gmail.com <an...@gmail.com> #4
This is a proper fix IMO. However, I can't find the package in http://maven.google.com/androidx/ . There I found only 2.0.0. Where else should I look? Also, the latest room 2.1.0-alpha4 does not include this version as well.
da...@google.com <da...@google.com> #5
It seems sqlite-framework:2.0.1 never made it to maven.google.com , we'll release it soon along with Room 2.1.0-alpha05 which should correctly depend on it. Sorry for the inconvenience.
an...@gmail.com <an...@gmail.com> #6
Thanks! I will definitely try the newest room version.
However, more I look at the code, more I believe the problem is elsewhere. If the migration fails, there will be no `SQLiteDatabase` and no `getWrappedDb(db)`. According to the code of `SQLiteOpenHelper`, there should be only one query before migration. The list of queries should be:
PRAGMA user_version;
DROP TABLE IF EXISTS `tracking_events` <--- this fails due closed database
DROP TABLE IF EXISTS `items` <-- never happens
More findings caught by my eye:
- I tried to make sure all DAO queries happen on a single thread. No success.
- The crash happens on a small number of users, but users get many crashes. I am afraid the user is unable to launch the app.
- Most crashes come from Samsung devices. But `SQLiteOpenHelper` class were not changed. All lines of stacktrace matches.
- `SQLiteOpenHelper` was constructed only once for `vinted_database.db`. However, meminfo shows something I don't understand:
DATABASES
pgsz dbsz Lookaside(b) cache Dbname
4 512 61 104/135/8 /data/user/0/lt.ito.md/databases/vinted_database.db
4 8 0/0/0 (attached) temp
4 512 25 35/14/2 /data/user/0/lt.ito.md/databases/vinted_database.db (3)
Looks like database was opened two times. There was opened more databases in my app, but no one appears in this list.
What else I could check?
However, more I look at the code, more I believe the problem is elsewhere. If the migration fails, there will be no `SQLiteDatabase` and no `getWrappedDb(db)`. According to the code of `SQLiteOpenHelper`, there should be only one query before migration. The list of queries should be:
PRAGMA user_version;
DROP TABLE IF EXISTS `tracking_events` <--- this fails due closed database
DROP TABLE IF EXISTS `items` <-- never happens
More findings caught by my eye:
- I tried to make sure all DAO queries happen on a single thread. No success.
- The crash happens on a small number of users, but users get many crashes. I am afraid the user is unable to launch the app.
- Most crashes come from Samsung devices. But `SQLiteOpenHelper` class were not changed. All lines of stacktrace matches.
- `SQLiteOpenHelper` was constructed only once for `vinted_database.db`. However, meminfo shows something I don't understand:
DATABASES
pgsz dbsz Lookaside(b) cache Dbname
4 512 61 104/135/8 /data/user/0/
4 8 0/0/0 (attached) temp
4 512 25 35/14/2 /data/user/0/
Looks like database was opened two times. There was opened more databases in my app, but no one appears in this list.
What else I could check?
da...@google.com <da...@google.com> #7
This type of behavior can also be seen when the corrupted database recovery mechanism is not correctly performed. Something that sqlite-framework:2.0.1 also fixed over its previous version.
See this patch and specifically the test in it:https://android.googlesource.com/platform/frameworks/support/+/611c90a4711670f8fe0d66ff2eaa8cb0266422d5
When the DB is corrupted (outside your control, and very little to do really) then the DB is immediately closed in hopes of creating a new DB (and a new object). What ends up happening is the corrupted object is still used causing the first db operation to fail (in this case a migration or that DROP TABLE IF EXISTS).
See this patch and specifically the test in it:
When the DB is corrupted (outside your control, and very little to do really) then the DB is immediately closed in hopes of creating a new DB (and a new object). What ends up happening is the corrupted object is still used causing the first db operation to fail (in this case a migration or that DROP TABLE IF EXISTS).
an...@gmail.com <an...@gmail.com> #8
That makes sense. I will update the app as soon as the new Room will be released and let you know if it helps.
Description
Version used: 2.1.0-alpha03
Devices/Android versions reproduced on: All devices. minSdk = 21. Most devices are Samsung (~90%)
I am getting tons of crashes and I am not able to reproduce them. Please see to attached stacktrace. I am using a default migration (drop everything -> recreate everything):
@Provides
@PerApplicationScope
fun provideDatabase(application: Application): VintedDatabase {
val builder = Room.databaseBuilder(
application,
VintedDatabase::class.java,
"vinted_database.db"
)
.fallbackToDestructiveMigration()
if (Debug.isDebuggerConnected()) {
builder.allowMainThreadQueries()
}
return builder.build()
}
The application is crashing on random DAO calls in various places in the app. There is a similar issue:
This generated code looks very stable. Also, everything at FrameworkSQLiteOpenHelper is synchronized, and I don't get how is possible to close DB during migration. But one thing caught my eye there:
FrameworkSQLiteDatabase getWrappedDb(SQLiteDatabase sqLiteDatabase) {
FrameworkSQLiteDatabase dbRef = mDbRef[0];
if (dbRef == null) {
dbRef = new FrameworkSQLiteDatabase(sqLiteDatabase);
mDbRef[0] = dbRef;
}
return mDbRef[0];
}
Just hypothetically, what if someone gets a DB through `FrameworkSQLiteOpenHelper.getWritableDatabase()` and closes immediate, like:
FrameworkSQLiteOpenHelper.getWritableDatabase().close()
Then DB instance will be closed and opening a new writable database gives a new SQLiteDatabase instance. But getWrappedDb returns an old one because the ref was not cleared because DB was closed directly instead of FrameworkSQLiteOpenHelper. Maybe some safety is needed here, like:
if (dbRef != null && dbRef.mDelegate != sqLiteDatabase) {
//try to close old instance anyway to avoid memory leak
dbRef.close()
dbRef = null
}
//rest code
Need to mention, DAO requests could be done on a few IO threads. Maybe Single thread environment would be better?