Closed Bug 964859 Opened 8 years ago Closed 8 years ago

crash in sqliteInternalCall

Categories

(Firefox for Android Graveyard :: General, defect)

29 Branch
All
Android
defect
Not set
critical

Tracking

(firefox29 affected)

RESOLVED FIXED
Firefox 29
Tracking Status
firefox29 --- affected

People

(Reporter: aaronmt, Assigned: jchen)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-5380e4ea-87c1-45d6-8f21-c00162140127.
=============================================================
I've run into this on my Nexus 5 and Nexus 7 2013. I don't have any STR other than both times I saw this crash I unlocked my device after letting it idle for a while.

Given the stack this looks to be fallout from bug 958706 (as expected).
I just hit this on my Nexus 4 (Android 4.4.2) right after a force Sync; new profile.
Assignee: nobody → nchen
Status: NEW → ASSIGNED
I think the crash happens because errorMsg is not initialized here [1]. Yay goto! This patch refactors exception handling in that file.

[1] http://mxr.mozilla.org/mozilla-central/source/mozglue/android/SQLiteBridge.cpp?rev=ccca3c4247e1#355
Attachment #8368213 - Flags: review?(gpascutto)
Comment on attachment 8368213 [details] [diff] [review]
Refactor SQLite bridge exception handling (v1)

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

r+ on the condition that you fix the db leaks.

::: mozglue/android/SQLiteBridge.cpp
@@ +161,5 @@
>      jenv->ReleaseStringUTFChars(jDb, dbPath);
>      if (rc != SQLITE_OK) {
> +        throwSqliteException(jenv,
> +            "Can't open database: %s", f_sqlite3_errmsg(db));
> +        return nullptr;

Your rework of the logic here is buggy. You need to call sqlite3_close even if open failed, see the sqlite3 documentation.

@@ +199,5 @@
>      jenv->ReleaseStringUTFChars(jDb, dbPath);
>      if (rc != SQLITE_OK) {
> +        throwSqliteException(jenv,
> +            "Can't open database: %s", f_sqlite3_errmsg(db));
> +        return 0;

Same remark, this is buggered and leaks the database handle.
Attachment #8368213 - Flags: review?(gpascutto) → review+
> r+ on the condition that you fix the db leaks.

Done. Nice catch!
Attachment #8368213 - Attachment is obsolete: true
Attachment #8368689 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/1cdb63b010bb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.