Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android not keeping inserts #601

Closed
craig-at-rsg opened this issue Dec 22, 2016 · 11 comments
Closed

Android not keeping inserts #601

craig-at-rsg opened this issue Dec 22, 2016 · 11 comments

Comments

@craig-at-rsg
Copy link
Contributor

craig-at-rsg commented Dec 22, 2016

I'm having a problem with Android not keeping inserts. This is a different problem from #193 because I don't use androidDatabaseImplementation: 2, and the data is lost without closing the app. Specifically, I insert a row and get back the inserted id, then the next insert to that table gets back the same insert id, and the inserted row is not available. I am using transactions for the inserts (1 insert per transaction), and the transactions appear successful.

One potential cause is that I'm opening the database read-only in some Android plugin code. My understanding was that read-only wouldn't affect a read-write connection, but maybe that's just for Sqlite itself and not for something that wraps it (Android code or this plugin).

What info would be useful in tracking this down? Or is there already a workaround (can I use androidLockWorkaround: 1 without androidDatabaseImplementation: 2)? The docs seem to say that won't help.

@brodycj
Copy link

brodycj commented Dec 22, 2016

Hi @craig-at-rsg,

Would you be able to make and post reproduction app for me? It might be easiest if you can use https://github.com/brodybits/cordova-sqlite-storage-starter-app as a starting point.

In case you are interested in private, commercial support please contact brodybits@litehelpers.net (I do remember we spoke before). Thanks and happy holidays!

@craig-at-rsg
Copy link
Contributor Author

craig-at-rsg commented Dec 22, 2016 via email

@brodycj
Copy link

brodycj commented Dec 22, 2016

Hi @craig-at-rsg,

To be honest I would not rule this out. The built-in (AOSP) SQLite database implementation is a real beast and has changed quite a bit from Android 1.x to Android 5.x or 6.x. I know since SQLCipher was based on an early version and I helped them fix a number of shortcomings. I am not 100% sure that it would really open a "read-only" connection as read-only.

For the future I will change the default implementation to build sqlite3 with improved thread safety. For now the version at https://github.com/brodybits/cordova-sqlite-ext-common already has this improved thread safety. It is still not officially published so I may do some force push but this is just something for you to try.

It may be safest for you to use androidDatabaseImplementation: 2, maybe with androidLockWorkaround: 1 so that you don't mix different sqlite builds.

Another alternative discussed in #309 may be to use a content provider, which you can access through both a plugin like https://github.com/phearme/cordova-ContentProviderPlugin and your own Java code. I found a couple nice looking tutorials at:

Please keep me posted.

@craig-at-rsg
Copy link
Contributor Author

Adding 'androidLockWorkaround: 1' seems to fix it, I'm planning on doing more testing with that and not 'androidDatabaseImplementation: 2' (we're close to a release and I want to make the smallest change possible that fixes this).

One more piece of info: it seems to be the last insert that gets lost; the app might insert 3 or 4 rows in a loop, and it seems to be only the last one that disappears. I assume this is a timing issue and that the last one is left uncommitted so it's most vulnerable, and that there's a small chance the others could get lost as well, so I probably still have a hole, but a much smaller one.

I did test with 'androidDatabaseImplementation: 2' and not 'androidLockWorkaround: 1', and I still saw the issue.

I'll have to think about a watertight solution for our next release. I'm not sure about using a content provider for Android - I have a lot of queries in Javascript, and I'd like to keep 1 implementation of these for both Android and iOS...

@brodycj
Copy link

brodycj commented Dec 23, 2016

Hi @craig-at-rsg,

Thanks for the info. Note that the plugin will ignore androidLockWorkaround: 1 unless you use androidDatabaseImplementation: 2. The androidLockWorkaround trick would simply close and reopen the database at the end of every transaction.

I am thinking of another trick for you to try: do not use either of these options but modify your JavaScript to close and reopen the database in the success callback of every write transaction. (You would also have to update the database handle in your own state.)

I can take another look at this after about 24 hours.

@brodycj
Copy link

brodycj commented Jan 4, 2017

Hi @craig-at-rsg,

Were you able to find a good solution?

I just updated the default Android database implementation to have better thread safety (NDK code built with -DSQLITE_THREADSAFE=1). You may want to give this a try.

@craig-at-rsg
Copy link
Contributor Author

Hi @brodybits,

I didn't try to come up with a good solution, I just used androidLockWorkaround: 1 and androidDatabaseImplementation: 2. Thanks for letting me know about the threadsafe config change - I'll give that a try without the lock workaround parameters.

@brodycj
Copy link

brodycj commented Jan 10, 2017

@craig-at-rsg did you get a chance to try the new version without the androidDatabaseImplementation: 2 and androidLockWorkaround: 1 settings? Is there any need to keep this open?

@craig-at-rsg
Copy link
Contributor Author

@brodybits I have been running with the new version, and I haven't seen a problem - I don't have a reason to keep this open. Thanks for all of your help on this.

@brodycj
Copy link

brodycj commented Jan 10, 2017

Thanks @craig-at-rsg for the confirmation.

@brodycj
Copy link

brodycj commented Jan 27, 2017

FYI this problem could be due to the "Multiple SQLite problem" described near the end of http://ericsink.com/entries/sqlite_android_n.html. I raised #626 to document this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants