Closed Bug 744816 Opened 10 years ago Closed 10 years ago

Fennec crashes in concurrent access of password db


(Firefox for Android Graveyard :: General, defect, P1)

14 Branch


(blocking-fennec1.0 beta+)

Firefox 14
Tracking Status
blocking-fennec1.0 --- beta+


(Reporter: tracy, Assigned: wesj)



(Keywords: crash, Whiteboard: [native-crash][has reviewed patch])


(3 files, 1 obsolete file)

Attached file log of crash
Seen with latest inbound build with password Sync 


0) clean install the given build
1) Setup sync to existing desktop client via jpake
2) attempt to add a password to fennec at say

tested results:
fennec crashes and sync fails

expected results:
no crash, password is added to db and sync succeeds.
blocking-fennec1.0: --- → ?
Keywords: crash
Whiteboard: [native-crash]
The causes for those (by our current understanding) require it to be simultaneous with Gecko launching. That is not the case here?
Looks like an invalid free in GeckoJNI code. I blame wesj.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #3)
> Looks like an invalid free in GeckoJNI code. I blame wesj.

Quick sleuthing! Poor wesj.
Blocks: 709385
Assignee: nobody → wjohnston
blocking-fennec1.0: ? → beta+
Weird. I rewrote encrypt and decrypt to both do nothing but

char* result = (char*)malloc(2048);
return jenv->NewStringUTF("");

And I still crash.
(In reply to Wesley Johnston (:wesj) from comment #5)

> And I still crash.

As far as I know, this implies that the problem is more general heap corruption that is simply being detected during free.

Time to audit all of the C++ that runs up until that point…
Blocks: 742104
I have temporarily disabled the password sync stage until this bug is resolved.
Priority: -- → P1
Attached patch Bandaid patch (obsolete) — Splinter Review
I don't like this much, but wanted to have this up as a bandaid patch while I try to figure this out. Basically, just holds a buffer forever.

I can trigger this bug without having to have two threads with concurrent access to the db. It seems related to the length of the string I try to encode (although that also seems unlikely and probably is unrelated).
Attachment #615000 - Flags: feedback?(rnewman)
Attached patch FixSplinter Review
I knew this was something stupid staring me in the face.
Attachment #615000 - Attachment is obsolete: true
Attachment #615000 - Flags: feedback?(rnewman)
Attachment #615004 - Flags: review?(rnewman)
Comment on attachment 615004 [details] [diff] [review]

Review of attachment 615004 [details] [diff] [review]:

Heh. Good spot.

Can you write a trivial test that fails when linked with eFence[0] or equivalent? Or file a bug to run cppcheck[1] against this source?

This kind of overrun should be easy to catch automatically, and who knows… there might be more.

[0] <>
[1] <>
Attachment #615004 - Flags: review?(rnewman) → review+
Whiteboard: [native-crash] → [native-crash][has reviewed patch]

I'm looking into using efence (i don't see it used anywhere else in our source, but it seems like it would be a useful thing...). There's also some work using cppcheck (bug 679417), but no bug to automagically run it. I did run it here though. Filing bugs for things it shows...

might be relevant to this, too.
Running into a somewhat if not exact same crash -- see attached log 

Nightly (04/16), Galaxy Nexus (Android 4.0.4)
Setting target milestone for inbound.
Target Milestone: --- → Firefox 14
(In reply to Richard Newman [:rnewman] from comment #7)
> I have temporarily disabled the password sync stage until this bug is
> resolved.

This was re-enabled by:

changeset:   91647:190a237b1456
user:        Nick Alexander <>
date:        Fri Apr 13 19:41:40 2012 -0700
summary:     Bug 739629 - Expose safe account creation API for profile migration. r=rnewman
Duplicate of this bug: 746133
Blocks: 745039
Closed: 10 years ago
Resolution: --- → FIXED
verified with m-c nightly build of 20120418
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.