Last Comment Bug 705112 - crash JNI_CreateJavaVM | sa_stream_destroy
: crash JNI_CreateJavaVM | sa_stream_destroy
Status: RESOLVED FIXED
[native-crash][mobile-crash]
: crash, regression, reproducible, topcrash
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 10 Branch
: Other Android
: -- critical (vote)
: mozilla11
Assigned To: Matthew Gregan [:kinetik]
:
:
Mentors:
https://bugzilla.mozilla.org/attachme...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-24 05:16 PST by Bill Gianopoulos [:WG9s]
Modified: 2012-04-11 20:34 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
12+
fixed


Attachments
v0 (3.54 KB, patch)
2011-11-24 11:40 PST, Matthew Gregan [:kinetik]
doug.turner: review+
Details | Diff | Splinter Review
esr patch v0 (3.19 KB, patch)
2012-04-06 17:03 PDT, Matthew Gregan [:kinetik]
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Bill Gianopoulos [:WG9s] 2011-11-24 05:16:10 PST
This bug was filed from the Socorro interface and is 
report bp-23e33a0e-f29f-418b-a33f-fb7962111124 .
=============================================================
Comment 1 Bill Gianopoulos [:WG9s] 2011-11-24 05:19:51 PST
Trying to play the OGG file in this attachment from bug 704450 crashes on a Samsung Galaxy Tab 8.9.
Comment 2 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-11-24 06:28:29 PST
Frame 	Module 	Signature [Expand] 	Source
0 	libdvm.so 	JNI_CreateJavaVM 	
1 	libxul.so 	sa_stream_destroy 	media/libsydneyaudio/src/sydney_audio_android.c:280
2 	libxul.so 	nsNativeAudioStream::Init 	content/media/nsAudioStream.cpp:424
3 	libxul.so 	mozilla::dom::AudioParent::AudioParent 	dom/ipc/AudioParent.cpp:303
4 	libxul.so 	mozilla::dom::ContentParent::AllocPAudio 	dom/ipc/ContentParent.cpp:876
5 	libxul.so 	mozilla::dom::PContentParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PContentParent.cpp:851
6 	libxul.so 	mozilla::ipc::AsyncChannel::OnDispatchMessage 	ipc/glue/AsyncChannel.cpp:294
7 	libxul.so 	mozilla::ipc::RPCChannel::OnMaybeDequeueOne 	ipc/glue/RPCChannel.cpp:433
8 	libxul.so 	RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run 	ipc/chromium/src/base/tuple.h:383
9 	libxul.so 	mozilla::ipc::RPCChannel::DequeueTask::Run 	RPCChannel.h:464
10 	libxul.so 	MessageLoop::RunTask 	ipc/chromium/src/base/message_loop.cc:318
11 	libxul.so 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/message_loop.cc:326
12 	libxul.so 	MessageLoop::DoWork 	ipc/chromium/src/base/message_loop.cc:426
13 	libxul.so 	mozilla::ipc::DoWorkRunnable::Run 	ipc/glue/MessagePump.cpp:70
14 	libxul.so 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:631
15 	libxul.so 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:245
16 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
17 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:208
18 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:201
19 	libxul.so 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:189
20 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:228
21 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3551
22 	libxul.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	toolkit/xre/nsAndroidStartup.cpp:132
23 	libmozutils.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:232
24 	libdvm.so 	dvmPlatformInvoke 	
25 	libdvm.so 	dvmCallJNIMethod_general 	
26 	libdvm.so 	dvmResolveNativeMethod 	
27 	libdvm.so 	dvmAsmSisterStart 	
28 	libdvm.so 	dvmMterpStd 	
29 	libdvm.so 	dvmInterpret 	
30 	libdvm.so 	dvmCallMethodV 	
31 	libdvm.so 	dvmCallMethod 	
32 	libdvm.so 	dvmAttachCurrentThread 	
33 	libc.so 	__thread_entry 	
34 	libc.so 	pthread_create 	
35 		@0x5e8301ae
Comment 3 Bill Gianopoulos [:WG9s] 2011-11-24 07:12:13 PST
That audio file has a non-standard sample-rate.

Using this file newly attached results in a file that plays without crashing:

  https://bugzilla.mozilla.org/attachment.cgi?id=576756&action=edit

Perhaps for Android we should have a list of valid sample rates and not try to play things not in the list?
Comment 4 Bill Gianopoulos [:WG9s] 2011-11-24 07:41:39 PST
Adding regression Keyword and requesting tracking Firefox 10 because this does not crash in the current Firefox version 9 beta.
Comment 5 Matthew Gregan [:kinetik] 2011-11-24 11:40:02 PST
Created attachment 576810 [details] [diff] [review]
v0
Comment 6 Doug Turner (:dougt) 2011-11-25 00:28:17 PST
Comment on attachment 576810 [details] [diff] [review]
v0

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

::: media/libsydneyaudio/src/sydney_audio_android.c
@@ +119,4 @@
>  
>  static jclass
>  init_jni_bindings(JNIEnv *jenv) {
> +  jclass class = (*jenv)->FindClass(jenv, "android/media/AudioTrack");

I am kinda of surprised that this returns null.
Comment 7 Doug Turner (:dougt) 2011-11-25 00:28:47 PST
land on m-c, we can merge back to birch whenever.
Comment 8 Bill Gianopoulos [:WG9s] 2011-11-29 09:08:31 PST
Using my own build including this patch, I am unable to reproduce the crash.
Comment 9 Matthew Gregan [:kinetik] 2011-11-29 20:51:21 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/19c6570995fa
Comment 10 Marco Bonardo [::mak] 2011-12-01 03:56:09 PST
https://hg.mozilla.org/mozilla-central/rev/19c6570995fa
Comment 11 Alex Keybl [:akeybl] 2012-01-09 11:23:25 PST
Setting tracking flag for 10 to - since crash-stats does not suggest that this is occurring in FF10beta2.
Comment 12 Scoobidiver (away) 2012-03-28 04:47:04 PDT
It's #9 top browser crasher in Fennec 10.0.3esr.
Comment 13 Alex Keybl [:akeybl] 2012-03-29 17:33:15 PDT
Matthew - how do you feel about the risk to the ESR10 branch (desktop and mobile) if we try to fix on that branch? Thanks in advance.
Comment 14 Matthew Gregan [:kinetik] 2012-03-29 17:59:18 PDT
The code is only used on Android, so the risk is limited to that platform.  It's a straightforward fix, so I'd rate the risk on Android as very low.
Comment 15 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-02 16:44:43 PDT
[Triage Comment]
Please go ahead and prepare/nominate a patch for ESR branch, see https://wiki.mozilla.org/Release_Management/ESR_Landing_Process
Comment 16 Matthew Gregan [:kinetik] 2012-04-06 17:03:01 PDT
Created attachment 613028 [details] [diff] [review]
esr patch v0

Slightly rebased due to missing patches on ESR.
Comment 17 Matthew Gregan [:kinetik] 2012-04-06 17:04:39 PDT
ESR try push: https://tbpl.mozilla.org/?tree=Try&rev=d06166466e74
Comment 18 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-09 14:47:14 PDT
(In reply to Matthew Gregan [:kinetik] from comment #17)
> ESR try push: https://tbpl.mozilla.org/?tree=Try&rev=d06166466e74

So that push is entirely red - have you another? :)
Comment 19 Matthew Gregan [:kinetik] 2012-04-09 16:08:04 PDT
Building Android ESR on try seems to require additional magic.  Let's see if this one works:

https://tbpl.mozilla.org/?tree=Try&rev=ab375a931ee7
Comment 20 Matthew Gregan [:kinetik] 2012-04-09 19:50:02 PDT
(In reply to Matthew Gregan [:kinetik] from comment #19)
> https://tbpl.mozilla.org/?tree=Try&rev=ab375a931ee7

Builds are green.  I think that's good enough--the only difference in this patch compared to the original patch is removal of a few lines freeing resources that aren't allocated in the earlier versions of the code--the changes have effectively been tested by the original patch landing.

I don't know what's up with the tests in this try push, they're all failing to even start due to a config issue/difference on try vs the ESR build config.
Comment 21 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-11 16:16:09 PDT
Comment on attachment 613028 [details] [diff] [review]
esr patch v0

Thanks for preparing this.
Comment 22 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-11 16:17:42 PDT
(In reply to Matthew Gregan [:kinetik] from comment #20)

> I don't know what's up with the tests in this try push, they're all failing
> to even start due to a config issue/difference on try vs the ESR build
> config.

When you land on ESR can you please confirm back in there that the test runs on ESR branch don't mimic what we're seeing on the try push?
Comment 23 Matthew Gregan [:kinetik] 2012-04-11 18:46:55 PDT
(In reply to Lukas Blakk [:lsblakk] from comment #22)
> When you land on ESR can you please confirm back in there that the test runs
> on ESR branch don't mimic what we're seeing on the try push?

Will do.

https://hg.mozilla.org/releases/mozilla-esr10/rev/b6c87541e34c
Comment 24 Matthew Gregan [:kinetik] 2012-04-11 20:34:29 PDT
(In reply to Matthew Gregan [:kinetik] from comment #23)
> https://hg.mozilla.org/releases/mozilla-esr10/rev/b6c87541e34c

Android tests ran fine (other than a few existing intermittent failures), so this looks good.

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