Closed Bug 741224 Opened 9 years ago Closed 9 years ago

Can't open tabs.db for writing, browser.db error code = 5, Gecko won't start up

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 15
Tracking Status
firefox14 --- fixed
blocking-fennec1.0 --- beta+

People

(Reporter: joe, Assigned: lucasr)

References

Details

(Whiteboard: [has patch][inbound])

Attachments

(2 files, 4 obsolete files)

Fennec hung on me for one reason or another, and I killed it. Now Gecko won't start up, and I get the following errors in my adb lolcat:

I/SqliteDatabaseCpp(26427): sqlite returned: error code = 5, msg = statement aborts at 1: [PRAGMA journal_mode=TRUNCATE] , db=/data/data/org.mozilla.fennec/files/mozilla/kfkd3xxm.default/browser.db
E/SQLiteOpenHelper(26427): Couldn't open /data/data/org.mozilla.fennec/files/mozilla/kfkd3xxm.default/tabs.db for writing (will try read-only):
E/SQLiteOpenHelper(26427): android.database.sqlite.SQLiteDatabaseLockedException: error code 5: database is locked
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteStatement.native_1x1_string(Native Method)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteStatement.simpleQueryForString(SQLiteStatement.java:161)
E/SQLiteOpenHelper(26427): 	at android.database.DatabaseUtils.stringForQuery(DatabaseUtils.java:735)
E/SQLiteOpenHelper(26427): 	at android.database.DatabaseUtils.stringForQuery(DatabaseUtils.java:723)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteDatabase.setJournalMode(SQLiteDatabase.java:1060)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:996)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteDatabase.openOrCreateDatabase(SQLiteDatabase.java:1051)
E/SQLiteOpenHelper(26427): 	at android.app.ContextImpl.openOrCreateDatabase(ContextImpl.java:770)
E/SQLiteOpenHelper(26427): 	at android.content.ContextWrapper.openOrCreateDatabase(ContextWrapper.java:221)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:157)
E/SQLiteOpenHelper(26427): 	at android.database.sqlite.SQLiteOpenHelper.getReadableDatabase(SQLiteOpenHelper.java:231)
E/SQLiteOpenHelper(26427): 	at org.mozilla.fennec.db.TabsProvider.getReadableDatabase(TabsProvider.java:247)
E/SQLiteOpenHelper(26427): 	at org.mozilla.fennec.db.TabsProvider.query(TabsProvider.java:510)
E/SQLiteOpenHelper(26427): 	at android.content.ContentProvider$Transport.query(ContentProvider.java:178)
E/SQLiteOpenHelper(26427): 	at android.content.ContentResolver.query(ContentResolver.java:310)
E/SQLiteOpenHelper(26427): 	at org.mozilla.gecko.TabsAccessor$2.doInBackground(TabsAccessor.java:120)
E/SQLiteOpenHelper(26427): 	at org.mozilla.gecko.TabsAccessor$2.doInBackground(TabsAccessor.java:109)
E/SQLiteOpenHelper(26427): 	at org.mozilla.gecko.GeckoAsyncTask$BackgroundTaskRunnable.run(GeckoAsyncTask.java:61)
E/SQLiteOpenHelper(26427): 	at android.os.Handler.handleCallback(Handler.java:605)
E/SQLiteOpenHelper(26427): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/SQLiteOpenHelper(26427): 	at android.os.Looper.loop(Looper.java:137)
E/SQLiteOpenHelper(26427): 	at org.mozilla.gecko.GeckoBackgroundThread.run(GeckoBackgroundThread.java:31)
E/GeckoConsole(26427): [JavaScript Error: "Couldn't play common dialog event sound: TypeError: Cc['@mozilla.org/sound;1'] is undefined" {file: "resource://gre/modules/CommonDialog.jsm" line: 213}]
W/SQLiteOpenHelper(26427): Opened /data/data/org.mozilla.fennec/files/mozilla/kfkd3xxm.default/tabs.db in read-only mode
blocking-fennec1.0: --- → ?
To be clear: I killed it using the Android "Nightly is not responding, do you want to wait or close it" dialog.
Moving over to Fennec.

Database corruption can occur whenever you kill a process. In this case Fennec should probably try clearing locks, and in the case of tabs fall back on recreating the DB.
Component: Firefox Sync: Backend → General
Product: Mozilla Services → Fennec Native
QA Contact: sync-backend → general
Summary: Can't open tabs.db for writing, browser.db corrupt, Gecko won't start up → Can't open tabs.db for writing, browser.db corrupt, Fennec won't start up
Summary: Can't open tabs.db for writing, browser.db corrupt, Fennec won't start up → Can't open tabs.db for writing, tabs.db corrupt, Fennec won't start up
Summary: Can't open tabs.db for writing, tabs.db corrupt, Fennec won't start up → Can't open tabs.db for writing, browser.db error code = 5, Gecko won't start up
Assignee: nobody → sriram
blocking-fennec1.0: ? → beta+
Attached patch WIP (obsolete) — Splinter Review
Is this idea right? When I get a DatabaseLocked exception, I delete the database, remove the mapping so that new DatabaseHelper is stored in the map, and then try getting a ReadableDatabase (which will create a new database and re-create tabs and indices (hopefully)).

If this approach is right, I'll extend it to getWritableDatabase() too.
Attachment #612286 - Flags: feedback?(rnewman)
I think there are two bugs here: firstly it can't open browser.db at all, and secondly it can't open tabs.db for writing. (Note that it successfully opens it for reading, which is the actual call it's trying to make anyway.)

Probably they're related, but not necessarily.

I think the former is a dupe of Bug 722055; consider duping these or reopening that bug and splitting.

(Joe, do you have earlier logs than you posted in Comment 0?)

Sriram, your patch will *probably* fix the latter, but in a very destructive way that wouldn't work for browser.db except in extreme circumstances (which would warrant a resync, too).

And I wouldn't be super keen on landing it without a test. (It should be straightforward to provoke a busted DB: http://www.sqlite.org/howtocorrupt.html)

I encourage you to explore solutions that remove the lock and roll back using the transaction log: e.g.,

http://random.kakaopor.hu/how-to-repair-an-sqlite-database

and also to consider whether there isn't an alternative explanation for what's going on:

http://www.sqlite.org/lockingv3.html

I don't think that this is a bug which would benefit from rushing to a solution without thorough testing. You might find that you blow away user data more frequently than you ought.
Comment on attachment 612286 [details] [diff] [review]
WIP

Review of attachment 612286 [details] [diff] [review]:
-----------------------------------------------------------------

Giving f- until this has had more analysis.
Attachment #612286 - Flags: feedback?(rnewman) → feedback-
Searching around the big web, I finally found this guy:

http://developer.android.com/reference/android/database/sqlite/SQLiteDatabase.html#setLockingEnabled%28boolean%29

It states:
"This is pretty expensive, so if you know that your DB will only be used by a single thread then you should set this to false. The default is true."

Since every DB access goes through ContentProvider, I think we are ensuring thread safety there. Aren't we? If so, we should probably turn off setLockingEnabled(). This "might" gives us performance win too.
(In reply to Sriram Ramasubramanian [:sriram] from comment #6)

> Since every DB access goes through ContentProvider, I think we are ensuring
> thread safety there. Aren't we? If so, we should probably turn off
> setLockingEnabled(). This "might" gives us performance win too.

If you can guarantee it (by never allowing the DB handle and the bridge to escape), then absolutely. Good spot!
Attached patch Patch (obsolete) — Splinter Review
Is that all the patch it needs? I've no clue about ContentProviders and the way we handle databases. I just built TabsProvider based on BrowserProvider. I don't know what all will this affect.
Attachment #612286 - Attachment is obsolete: true
Attachment #612943 - Flags: review?(rnewman)
Comment on attachment 612943 [details] [diff] [review]
Patch

Review of attachment 612943 [details] [diff] [review]:
-----------------------------------------------------------------

Have you verified that this avoids the problem?

That is:

* Take a build before this fix
* Force-quit Fennec
* Verify that you can't reopen
* Uninstall
* Repeat with fix
* Verify that you can reopen without error

?
renoming. I checked with sriram and this issue isn't reproducible enough to test his patches. can we minus until this bug is actionable?
blocking-fennec1.0: beta+ → ?
Notes:
* Desktop also checks for locked DBs on startup and, if it happens, tells users how to make a new profile. (not that helpful)
* For tabs.db, I'd consider just recreating the DB and let Sync refill the data. Synced tabs do not need to be durable locally.
* For browser.db, we should see if we can pull any data from the DB. If not, we may have no choice but to recreate.

We need to get a busted DB so we can test solutions.
About thread-safety: the important thing for keeping SQLite databases sane in Android is to avoid opening multiple DB connections from different threads. Our content provider solves this problem (as a side effect) because it concentrates all DB accesses into only one DB connection (the SQLiteOpenHelper instance).

So, as long as we don't create separate DB connections to tabs.db or browser.db outside the content provider, it should be (in theory) safe to disable locking. But yes, we need a reliable way to guarantee that this actually fixes the bug here.
blocking-fennec1.0: ? → -
Comment on attachment 612943 [details] [diff] [review]
Patch

Clearing review flag until we get clarity on whether this actually fixes this bug.
Attachment #612943 - Flags: review?(rnewman)
blocking-fennec1.0: - → ?
Let's try to get this fixed. We need to get some logging/data and try to figure out what's happening.
Assignee: sriram → lucasr.at.mozilla
blocking-fennec1.0: ? → beta+
Duplicate of this bug: 745245
FYI: I've just submitted patches to bug 746444 that forces unlocking when the database file is permanently locked after a crash. Given that I couldn't reproduce the bug, I'll land the patch in m-c and see if that reduces the number of "database is locked" issues in the next few days. If the patch does work around the issue, I'll submit another patch to do the same in the tabs database.
Attachment #612943 - Attachment is obsolete: true
Attachment #618242 - Flags: review?(blassey.bugs)
Attachment #618244 - Flags: review?(blassey.bugs)
This patch apply the same unlocking behaviour in tabs.db than in browser.db (see bug 746444).
Comment on attachment 618242 [details] [diff] [review]
Force unlock on tabs.db file when database is opened in read-only mode

Review of attachment 618242 [details] [diff] [review]:
-----------------------------------------------------------------

ensureDatabaseIsNotLocked in TabsProvider.java.in looks identical to BrowserProvider.java.in. Let's share that code somehow
Attachment #618242 - Flags: review?(blassey.bugs) → review-
Attachment #618279 - Flags: review?(blassey.bugs)
Attachment #618242 - Attachment is obsolete: true
Attachment #618244 - Attachment is obsolete: true
Attachment #618244 - Flags: review?(blassey.bugs)
Didn't care much about the code duplication because the tabs provider is pretty much of copy of browser provider right now. New patches avoid the code duplication.
Whiteboard: [has patch][needs review blassey]
Attachment #618279 - Flags: review?(blassey.bugs) → review+
Attachment #618280 - Flags: review?(blassey.bugs) → review+
Comment on attachment 618279 [details] [diff] [review]
(1/2) Move database unlock function to DBUtils

Mobile-only, Fennec beta blocker.
Attachment #618279 - Flags: approval-mozilla-aurora?
Comment on attachment 618280 [details] [diff] [review]
(2/2) Force unlock on tabs.db file when database is opened in read-only mode

Mobile-only, Fennec beta blocker.
Attachment #618280 - Flags: approval-mozilla-aurora?
Attachment #618279 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #618280 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [has patch][needs review blassey] → [has patch]
Target Milestone: --- → Firefox 15
Depends on: pixelated
Whiteboard: [has patch] → [has patch][inbound]
You need to log in before you can comment on or make changes to this bug.