crash in sqliteInternalCall

RESOLVED FIXED in Firefox 29

Status

()

--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: jchen)

Tracking

({crash})

29 Branch
Firefox 29
All
Android
crash
Points:
---

Firefox Tracking Flags

(firefox29 affected)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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).
(Reporter)

Comment 2

5 years ago
I just hit this on my Nexus 4 (Android 4.4.2) right after a force Sync; new profile.
status-firefox29: --- → affected
(Assignee)

Updated

5 years ago
Assignee: nobody → nchen
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 8368213 [details] [diff] [review]
Refactor SQLite bridge exception handling (v1)

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+
(Assignee)

Comment 5

5 years ago
Created attachment 8368689 [details] [diff] [review]
Refactor SQLite bridge exception handling (v1.1)

> 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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.