945327 - crash in libc.so@0x22048 / libc.so@0x21f90 - Android 4.4 Nexus devices - Downloading files

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

This bug was filed from the Socorro interface and is 
report bp-f6c44ac6-311d-4f1a-a60e-078ee2131201.
=============================================================

Crash on the older Nexus 7 2012 when running Android 4.4, http://en.wikipedia.org/wiki/Nexus_7_%282012_version%29

Comment 1

[Benjamin Kerensa [:bkerensa]]

Some of the crash comments indicate downloading could be leading to crashes so maybe we can get some testcases on Kit Kat around downloads?

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Attachment: Screenshot of duplicate file names

I downloaded the same file repeatedly and eventually Firefox gave the file the same name. This crashed Firefox.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

Renominating for tracking Fennec and to get an assignee.

Comment 5

[Kevin Brosnan [Ex-Mozilla]]

This is reproducible on the ART run time as well

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: gdb output

Downloaded about 50 of [1] under gdb and produced the attached output. Ran `where` and `list` at the end. Note color codes are included.

In particular:

> #0  0x68817342 in mozalloc_abort (msg=0x6b8fab38 "[9119] ###!!! ABORT: 
>  Failed to push local JNI frame: 'ret == 0', file ../../dist/include
>  /AndroidBridge.h, line 515") at /home/mcomella/dev/fig/memory/mozalloc
>  /mozalloc_abort.cpp:30

[1]: https://upload.wikimedia.org/wikipedia/commons/d/d3/IU_at_the_Life_Style_Awards_2011_%282%29.jpg

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: logcat output

For run associated with comment 6, dumped a few minutes after the crash.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Jim, this looks like a mismatched JNI Push/PopFrame, can you dig in?

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

I was able to reproduce this on my Nexus 4, and I see several cases where local refs can be leaked in AndroidBridge. Right now I'm doing an audit of AndroidBridge methods.

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Improve local ref management in AndroidBridge (v1)

Seems to fix the crash on my Nexus 4 using mcomella's STR.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8355594 [details] [diff] [review]
Improve local ref management in AndroidBridge (v1)

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

::: widget/android/AndroidBridge.cpp
@@ +337,5 @@
>          aHandlersArray->AppendElement(app, false);
>          if (aDefaultApp && isDefault.Length() > 0)
>              *aDefaultApp = app;
> +
> +        aJNIEnv->PopLocalFrame(NULL);

why not use an AutoLocalJNIFrame?

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Improve local ref management in AndroidBridge (v1.1)

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #11)
> Comment on attachment 8355594 [details] [diff] [review]
> Improve local ref management in AndroidBridge (v1)
> 
> Review of attachment 8355594 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/android/AndroidBridge.cpp
> @@ +337,5 @@
> >          aHandlersArray->AppendElement(app, false);
> >          if (aDefaultApp && isDefault.Length() > 0)
> >              *aDefaultApp = app;
> > +
> > +        aJNIEnv->PopLocalFrame(NULL);
> 
> why not use an AutoLocalJNIFrame?

Good idea.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d7dfd3217a54

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d7dfd3217a54

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8356177 [details] [diff] [review]
Improve local ref management in AndroidBridge (v1.1)

[Approval Request Comment]

Bug caused by (feature/regressing bug #): N/A

User impact if declined: Crash doing certain tasks such as downloading files

Testing completed (on m-c, etc.): Locally, m-c

Risk to taking this patch (and alternatives if risky): Small; patch only fixes previous bugs and does not alter functionality

String or IDL/UUID changes made by this patch: None

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bcdc6180d7c4
https://hg.mozilla.org/releases/mozilla-beta/rev/06a89edd358b