[Android]: Database locale issues #423
Conversation
|
Would love to see this reviewed & merged soon if possible. We have many frustrated users that are receiving the issue in #422. |
android/src/main/java/com/reactnativecommunity/asyncstorage/ReactDatabaseSupplier.java
Outdated
Show resolved
Hide resolved
According to Android docs, one needs to be consistent when using NO_COLLATION flag with how database was created, so changing this during runtime makes no sense and can lead to some unexpected errors/behaviors (SQLite default collation rule is BINARY).
krizzu
force-pushed
the
fix/ensure-db-closed
branch
from
e2b1409
to
818f9cc
Compare
|
Thanks @tido64 for having a look 🙏 According to Android docs, one needs to be consistent with what collation rules were used when creating the database. That means, setting this flag to |
android/src/main/java/com/reactnativecommunity/asyncstorage/ReactDatabaseSupplier.java
Outdated
Show resolved
Hide resolved
Does this mean that the problem is that we tried to open a database that was already opened in a different locale? Wouldn't restarting the app already close the database? Or is the implementation such that it is possible to leak connections? |
I believe this is the case we see here.
This is exactly what I think is happening - changing locale restarts the activity, not the app process, so it leaks the db connection. |
The repro steps in #178 mentions killing the app though. I thought that would close the connection? |
This is the most confusing part of the whole issue. I also thought the connection will be closed, but apparently it's not 🤷♂️ Dunno if we're not missing something here. |
|
@tido64 I'll go ahead and merge this. Thanks for reviewing 🙏 |
Make sure that we close db connection when 'host' is destroyed and re-create it when needed
Summary:
Trying to address: #422 #178
Those issues we've seen are related to locale changing and database connection not being closed properly. I could not reproduce it myself, so those are just preventive measures. Can't tell if this would help for sure.
The fix is to close db connection when activity is destroyed - it'll be reestablished on next startup (in case of locale change, activity is recreated).
Another one is to disable localized collators for db, which is the most suggested fix forFailed to change locale for dberror.I've moved this fix behind a flag that you need to set in
gradle.propertiesin order to work.This has been dropped due to possibility of unexpected behavior. More info about being consistent with locale collation in android docs.
If anyone is able to constantly reproduce the issue, please do let me know. You can try out this branch before it's released by pointing your
package.jsondependency to: