n76 / Local-GSM-Backend

GSM LocationProvider backend for µg UnifiedNlp
Apache License 2.0
91 stars 23 forks source link

Using customized path for lacells.db makes copying new db impossible #81

Closed IzzySoft closed 8 years ago

IzzySoft commented 8 years ago

Almost, at least. Reproduced it 5 times now, no exception in between: while the new (replacement) lacells.db is still being copied, Local-GSM-Backend tries to open it, fails, and thus deletes it right away, see line 4 of the following logcat:

E/SQLiteLog(17275): (11) database corruption at line 54069 of [b3bb660af9]
E/SQLiteLog(17275): (11) database disk image is malformed
E/DefaultDatabaseErrorHandler(17275): Corruption reported by sqlite on database: /sdcard/.nogapps/lacells.db
E/DefaultDatabaseErrorHandler(17275): deleting the database file: /sdcard/.nogapps/lacells.db
E/SQLiteLog(17275): (14) cannot open file at line 30052 of [b3bb660af9]
E/SQLiteLog(17275): (14) os_unix.c:30052: (2) open(/sdcard/.nogapps/lacells.db) -
E/SQLiteDatabase(17275): Failed to open database '/sdcard/.nogapps/lacells.db'.
E/SQLiteDatabase(17275): android.database.sqlite.SQLiteCantOpenDatabaseException: unknown error (code 14): Could not open database
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnection.nativeOpen(Native Method)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnection.open(SQLiteConnection.java:209)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnection.open(SQLiteConnection.java:193)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnectionPool.openConnectionLocked(SQLiteConnectionPool.java:463)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnectionPool.open(SQLiteConnectionPool.java:185)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteConnectionPool.open(SQLiteConnectionPool.java:177)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteDatabase.openInner(SQLiteDatabase.java:806)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteDatabase.open(SQLiteDatabase.java:794)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:694)
E/SQLiteDatabase(17275):     at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:669)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.database.CellLocationDatabase.openDatabase(CellLocationDatabase.java:84)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.database.CellLocationDatabase.query(CellLocationDatabase.java:123)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.TelephonyHelper.legacyGetCellTowers(TelephonyHelper.java:197)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.TelephonyHelper.getTowerLocations(TelephonyHelper.java:240)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.TelephonyHelper.getLocationEstimate(TelephonyHelper.java:318)
E/SQLiteDatabase(17275):     at org.fitchfamily.android.gsmlocation.GsmService$2.run(GsmService.java:183)
E/SQLiteDatabase(17275):     at java.lang.Thread.run(Thread.java:818)
D/CountryDetector(  556): No listener is left

Only work-around is:

  1. opening expert settings
  2. changing the custom path (e.g. removing the last character or adding one – path doesn't have to be "correct")
  3. now copying the file and wait until copy is complete
  4. restore the correct custom path

Using the latest version (1.4.7) on CyanogenMod 12.1/Android 5.1 here.

May I propose an "exception handler" for that? I could think of e.g. remembering file size and timestamp on successful opening of the database – and then, when "corruption" is detected, compare that with the existing file. If it differs, sleep for .5..2s and check the file again; if size is growing, sleep again until it's no longer growing, then try opening it again. If it still fails, at least retry a few times more (the transfer might be stalled). If it starts growing again, reset the retry-counter. You could even make intervals and max number of retries (for the last step, i.e. how often to retry when the file is no longer growing) customizable.

If you end up really having to delete a corrupted database, at least give the user a clue: not everybody is used to logcat (it even took me a while to figure what's going on and why my network location had stopped working). E.g. placing a notification would be a good idea. You could even consider renaming the file instead of deleting it straight away: if then the transfer completes a little later, one at least could simply rename it back instead of starting over again and again :)

Thanks in advance (and apologies if I sounded grumpy – no grumpyness meant, just a report friendly asking for a fix :)

n76 commented 8 years ago

New database should be copied to some arbitrary filename and then renamed to lacells.db.new. If lacells.db.new exists and is accessible then the backend will close lacells.db, rename it to lacells.db.bak, rename lacells.db.new to lacells.db and then open it.

No fix, as this is the design and documented in the webpage on GitHub for this backend.

IzzySoft commented 8 years ago

OK, I see. But I wonder why simply copying it worked all those years, and now suddenly breaks. So several tools need to be updated: the lacells-creator is, after creating the DB file, simply pushing it to /sdcard/.nogapps, and "just placing the DB file into the directory for auto-sync" no longer works either.

Yes, it's possible working around it. But not very user-friendly (with things like simply using it with a sync tool being impossible).

Still, good to know. So I'll either revert to a previous version, or adjust my workflow to do everything manually again. I was one of those using a simply sync mechanism: device syncs automatically whenever connected via USB. Now I have to remember to transfer the database manually when a device is connected :(

Maybe you reconsider? Think of the not-so-tech-savvy users :)

n76 commented 8 years ago

I suspect the not-so-tech-savvy users are probably using the on-phone database generation facility. :)

IzzySoft commented 8 years ago

Well, that point goes to you. Still, it would be much easier if one could simply sync the created file.

Btw: Any intentions to fix the adb push part of lacells-creator then? As that won't work either :) Line 12 in lib/push then must rather read:

adb push "$LACELLS_DB" /sdcard/.nogapps/lacells.db.tmp
adb shell "mv /sdcard/.nogapps/lacells.db.tmp /sdcard/.nogapps/lacells.db.new"

Well – apart from the fact the target location might need some fine tuning :)