Last Comment Bug 717096 - Crash may occur when switching between webm tab or App and Flash tab (java.lang.IllegalStateException: play() called on uninitialized AudioTrack)
: Crash may occur when switching between webm tab or App and Flash tab (java.la...
Status: VERIFIED FIXED
[native-crash][inbound]
: crash, reproducible, topcrash
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: ARM Android
: P1 critical (vote)
: mozilla15
Assigned To: Gian-Carlo Pascutto [:gcp]
:
:
Mentors:
: 696804 (view as bug list)
Depends on:
Blocks: 746696
  Show dependency treegraph
 
Reported: 2012-01-10 16:17 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2012-05-30 10:58 PDT (History)
12 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
verified
verified
+
11+


Attachments
logcat (196.50 KB, text/plain)
2012-01-10 16:17 PST, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
Patch. Add exception handling, make sure FindClass gets global ref. Release native buffers. (6.10 KB, patch)
2012-05-07 05:31 PDT, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review-
Details | Diff | Splinter Review
Add exception handling, make sure FindClass gets global ref. Release native buffers. (6.04 KB, patch)
2012-05-07 08:46 PDT, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review+
mark.finkle: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-10 16:17:54 PST
Created attachment 587529 [details]
logcat

1. go to http://http://people.mozilla.com/~nhirata/html_tp/big_buck_bunny_480p.webm
2. play the video
3. go to http://www.cnn.com/video
4. click on the video and play
5. switch between the videos until the videos stop playing

Expected: switching will occur just fine
Actual: eventually you will crash

Note:
Fennec 20120110, Nexus S, Flash 11

01-10 15:45:18.109: W/dalvikvm(1284): threadid=24: thread exiting with uncaught exception (group=0x40015560)
01-10 15:45:18.109: W/PowerManagerService(114): Caller does not have DEVICE_POWER permission.  pid=1284 uid=10053
01-10 15:45:18.113: I/GeckoPluginsAudio(1284): 0x49aa8760 - Write failed -3
01-10 15:45:18.148: W/AudioTrack(1284): obtainBuffer() track 0x3c1da8 disabled, restarting
01-10 15:45:18.210: E/AndroidRuntime(1284): FATAL EXCEPTION: Thread-97
01-10 15:45:18.210: E/AndroidRuntime(1284): java.lang.IllegalStateException: play() called on uninitialized AudioTrack.
01-10 15:45:18.210: E/AndroidRuntime(1284): 	at android.media.AudioTrack.play(AudioTrack.java:824)
01-10 15:45:18.210: E/AndroidRuntime(1284): 	at dalvik.system.NativeStart.run(Native Method)
01-10 15:45:18.253: I/GeckoApp(1284): pause
01-10 15:45:18.253: W/ActivityManager(114):   Force finishing activity org.mozilla.fennec/.App
01-10 15:45:18.332: W/AudioTrack(1284): obtainBuffer() track 0x48aae0 disabled, restarting
01-10 15:45:18.386: D/dalvikvm(1284): GC_EXTERNAL_ALLOC freed 836K, 61% free 3226K/8071K, external 7673K/11670K, paused 122ms
01-10 15:45:18.449: V/RenderScript_jni(198): surfaceCreated
01-10 15:45:18.449: V/RenderScript_jni(198): surfaceChanged
01-10 15:45:18.453: W/AudioTrack(1284): obtainBuffer() track 0x394278 disabled, restarting
01-10 15:45:18.742: I/GeckoApp(1284): stop
01-10 15:45:18.750: I/GeckoApp(1284): destroy
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2012-01-12 10:58:46 PST
Dougt, do you want to take this since you wrote the audio stuff?
Comment 2 Doug Turner (:dougt) 2012-01-12 14:01:51 PST
i am not working on these right now.  resetting assignee.
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-02-03 18:03:16 PST
Fixing title. The ViewRoot exception does occur in the log, but is not the cause of the crash (it was also fixed in bug 715836).
Comment 4 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-02-27 12:16:41 PST
Java crash; nom-ing for blocking fennec
Comment 5 Kevin Brosnan [:kbrosnan] 2012-03-04 12:48:20 PST
Not a top crash, not blocking. Still good to fix though.
Comment 6 Scoobidiver (away) 2012-03-10 22:38:23 PST
There's one crash in 13.0a1/20120310: bp-9f800e70-a9ee-4bb6-9c09-e25042120310.
Comment 7 Scoobidiver (away) 2012-03-19 03:56:24 PDT
It's #4 top crasher in 13.0a2.
Comment 8 Chris Peterson [:cpeterson] 2012-03-19 12:03:36 PDT
snorp, bon appétit!
Comment 9 JP Rosevear [:jpr] 2012-04-04 18:46:06 PDT
Crashes in 14.0a1 still, #18
Comment 10 JP Rosevear [:jpr] 2012-04-04 19:07:25 PDT
Renoming with topcrash designation.
Comment 11 Brad Lassey [:blassey] (use needinfo?) 2012-04-05 12:32:37 PDT
#18 top crash is not high enough to warrant blocking on
Comment 12 Sheila Mooney 2012-04-12 10:05:10 PDT
You guys can decide if this is blocking or not BUT, it's reproducible, it's at #9 in the past day, #11 in the past 3 days. If was low on the list in the 7 day report because of all the other crashes that were still appearing because people hadn't updated. Because it's reproducible don't you think it warrants some investigation?
Comment 13 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-04-12 12:45:09 PDT
renoming based on comment 12
Comment 14 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-04-19 15:45:13 PDT
I ran into this bug by playing a youtube video in fennec, and switching to a different app (google +) by hitting the back button.  I think this might be a more common use case... ie a social app like twitter, facebook, google plus to switch to a website with flash and then switching back.
Comment 15 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-04-26 12:45:50 PDT
I was thinking of bug 703601 had a similarity, but bug 703601 is marked fixed.
Comment 16 Gian-Carlo Pascutto [:gcp] 2012-05-02 09:06:39 PDT
From what I can see, switching to and from the Flash tab (WebM has nothing to do with this), causes some kind of leak, which causes the AudioTrack to fail to allocate:

E/AudioFlinger( 2597): not enough memory for AudioTrack size=88264
D/MemoryDealer( 2597):   AudioTrack (0xaf4c0, size=1048576)
D/MemoryDealer( 2597):     0: 000af4d8 | 0x00000000 | 0x000158E0 | A 
D/MemoryDealer( 2597):     1: 000af508 | 0x000158E0 | 0x000158E0 | A 
D/MemoryDealer( 2597):     2: 00071248 | 0x0002B1C0 | 0x000158E0 | A 
D/MemoryDealer( 2597):     3: 00081968 | 0x00040AA0 | 0x000158E0 | A 
D/MemoryDealer( 2597):     4: 0007b3d0 | 0x00056380 | 0x000158E0 | A 
D/MemoryDealer( 2597):     5: 00068e30 | 0x0006BC60 | 0x000158E0 | A 
D/MemoryDealer( 2597):     6: 00071a30 | 0x00081540 | 0x000158E0 | A 
D/MemoryDealer( 2597):     7: 00060ea0 | 0x00096E20 | 0x000158E0 | A 
D/MemoryDealer( 2597):     8: 00068bd0 | 0x000AC700 | 0x000158E0 | A 
D/MemoryDealer( 2597):     9: 00060e38 | 0x000C1FE0 | 0x000158E0 | A 
D/MemoryDealer( 2597):    10: 00068a90 | 0x000D78C0 | 0x000158E0 | A 
D/MemoryDealer( 2597):    11: 000cb390 | 0x000ED1A0 | 0x00012E60 | F 
D/MemoryDealer( 2597):   size allocated: 971168 (948 KB)
E/AudioTrack(20444): AudioFlinger could not create track, status: -12
E/AudioTrack-JNI(20444): Error initializing AudioTrack
E/AudioTrack-Java(20444): [ android.media.AudioTrack ] Error code -20 when initializing AudioTrack.

I guess we don't properly check for that in some places and eventually crash. Still digging the code now.
Comment 17 Chris Peterson [:cpeterson] 2012-05-02 10:56:58 PDT
Could we be leaking AudioTracks?
Comment 18 Gian-Carlo Pascutto [:gcp] 2012-05-02 12:31:53 PDT
I've seen no evidence (yet!) that it's AudioTracks specifically that we're leaking, but we're almost certainly leaking *something*.
Comment 19 Gian-Carlo Pascutto [:gcp] 2012-05-04 05:39:08 PDT
I've got some fixes that make the audio plugin code more robust when dealing with errors, and makes it free the internal audio buffers explicitly, as well as fix some bugs like not getting a global ref to a class. After these, we no longer get the crash due to "play called on ...", and you can switch back and forth quite a bit longer...

...however, we're still leaking *somewhere*, and eventually one of the many PushLocalFrame calls we have will generate a: 
W/System.err(21665): java.lang.OutOfMemoryError: out of stack in JNI PushLocalFrame

And we'll die. Our current code doesn't really allow sane error handling in those AutoLocalJNIFrame things.

I don't see evidence that AudioTrack is responsible here. Probably just Flash being Flash or us leaking in interacting with it?

My fix will make this signature go away, but we'll get (slightly less) OOM errors instead.
Comment 20 Gian-Carlo Pascutto [:gcp] 2012-05-07 05:31:14 PDT
Created attachment 621568 [details] [diff] [review]
Patch. Add exception handling, make sure FindClass gets global ref. Release native buffers.
Comment 21 Brad Lassey [:blassey] (use needinfo?) 2012-05-07 07:27:23 PDT
Comment on attachment 621568 [details] [diff] [review]
Patch. Add exception handling, make sure FindClass gets global ref. Release native buffers.

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

r- to get the logic in anp_audio_newTrack cleaned up

::: dom/plugins/base/android/ANPAudio.cpp
@@ +103,5 @@
>  
>  static jclass
>  init_jni_bindings(JNIEnv *jenv) {
> +  jclass jc_local = jenv->FindClass("android/media/AudioTrack");
> +  jclass jc = (jclass)jenv->NewGlobalRef(jc_local);

jclass jc = jclass)jenv->NewGlobalRef(jenv->FindClass("android/media/AudioTrack"));

@@ +278,5 @@
>                                  jformat,
>                                  s->bufferSize,
>                                  MODE_STREAM);
>  
>    jthrowable exception = jenv->ExceptionOccurred();

use a AutoLocalJNIFrame here, it'll avoid a lot of this code goop and the need for a goto
Comment 22 Gian-Carlo Pascutto [:gcp] 2012-05-07 08:46:38 PDT
Created attachment 621620 [details] [diff] [review]
Add exception handling, make sure FindClass gets global ref. Release native buffers.

A goto or code duplication? The duplication is small so let's take the simpler control flow here.
Comment 23 Gian-Carlo Pascutto [:gcp] 2012-05-08 03:14:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f18d4a2fce0d
Comment 24 Ed Morley [:emorley] 2012-05-08 11:19:24 PDT
https://hg.mozilla.org/mozilla-central/rev/f18d4a2fce0d
Comment 25 Gian-Carlo Pascutto [:gcp] 2012-05-08 12:05:51 PDT
Comment on attachment 621620 [details] [diff] [review]
Add exception handling, make sure FindClass gets global ref. Release native buffers.

[Approval Request Comment]
Mobile only. Blocking Fennec. Also likely fixes 746696 which is another blocker.
Comment 26 Alex Keybl [:akeybl] 2012-05-09 15:51:04 PDT
We are leaving all non-beta+ bugs nominated for Aurora approval in the queue until FN14 Beta 1 is signed off on by QA.
Comment 27 Gian-Carlo Pascutto [:gcp] 2012-05-11 16:39:45 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/a8a1d8dfba1d
Comment 28 Brad Lassey [:blassey] (use needinfo?) 2012-05-12 07:07:26 PDT
*** Bug 696804 has been marked as a duplicate of this bug. ***
Comment 29 Cristian Nicolae (:xti) 2012-05-30 10:58:35 PDT
This crash doesn't occur anymore on the latest Nightly and Aurora builds. Instead another issue can be noticed and is described in Bug 759830. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-30)
Firefox 14.0a2 (2012-05-30)

Device: Galaxy Nexus
OS: Android 4.0.2

Note You need to log in before you can comment on or make changes to this bug.