Closed Bug 745174 Opened 12 years ago Closed 11 years ago

java.lang.IllegalStateException: Unable to retrieve AudioTrack pointer for getPosition() at android.media.AudioTrack.native_get_position(Native Method)

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
critical

Tracking

(firefox16 affected, firefox17 affected, firefox18 affected, firefox19 affected, firefox20 affected, firefox21 affected, firefox22 unaffected)

RESOLVED WORKSFORME
Tracking Status
firefox16 --- affected
firefox17 --- affected
firefox18 --- affected
firefox19 --- affected
firefox20 --- affected
firefox21 --- affected
firefox22 --- unaffected

People

(Reporter: scoobidiver, Assigned: padenot)

References

(Blocks 1 open bug, )

Details

(Keywords: crash, reproducible, Whiteboard: [native-crash] [games:p1])

Crash Data

Attachments

(1 file, 1 obsolete file)

It was hit by two users from 14.0a1/20120405105200. See for instance bp-08bbe06d-1005-4fa0-af81-58a5a2120411.

java.lang.IllegalStateException: Unable to retrieve AudioTrack pointer for getPosition()
	at android.media.AudioTrack.native_get_position(Native Method)
	at android.media.AudioTrack.getPlaybackHeadPosition(AudioTrack.java:584)
	at dalvik.system.NativeStart.run(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalStateException%3A+Unable+to+retrieve+AudioTrack+pointer+for+getPosition%28%29+at+android.media.AudioTrack.native_get_position%28Native+Method%29
Seems like Fennec crashed when trying to find the seek position of the audio track?
Ran into this issue by going to Martijn's test case:
http://people.mozilla.com/~mwargers/tests/video/atomicplayboy_winopenclose.htm

I opened it, let it play, hit the back button then tried to quit.  I think it's a timing issue of quitting while it's trying to get the position.
Version: Firefox 14 → Trunk
I hit this with 17 while looking at the testcase in bug 820217; upon going "back" from the page.  I don't -think- I was trying to Quit, but I may have been.  There's a good chance we'll start hitting this more and more as people try to do more game stuff (therefore more audio stuff) with Firefox on Android.
Whiteboard: [native-crash] → [native-crash] [games:p2]
I am able to reproduce this 100% of the time in Fennec via Quake Demo (http://www.quaddicted.com/stuff/WebQuake/Client/WebQuake.htm), and then closing the tab while running.

E/GeckoAppShell( 6979): java.lang.IllegalStateException: Unable to retrieve AudioTrack pointer for getPosition()
E/GeckoAppShell( 6979): 	at android.media.AudioTrack.native_get_position(Native Method)
E/GeckoAppShell( 6979): 	at android.media.AudioTrack.getPlaybackHeadPosition(AudioTrack.java:633)
E/GeckoAppShell( 6979): 	at dalvik.system.NativeStart.run(Native Method)
E/Gecko   ( 6979): mozalloc_abort: Redirecting call to abort() to mozalloc_abort
We should really get this crash fixed in the Android audio implementation.  Brad, who owns that?
Whiteboard: [native-crash] [games:p2] → [native-crash] [games:p1]
Needinfo to answer Vlad's question.
Flags: needinfo?(blassey.bugs)
It has many owners at this point. Assigning to gcp for now, but also CC'ing Paul in case he wants to take a crack.
Assignee: nobody → gpascutto
Flags: needinfo?(blassey.bugs)
This code is removed in beta, but I can write you a patch if absolutely needed.
Assignee: gpascutto → paul
Attached patch patch (obsolete) — Splinter Review
Can someone try something like that? I cannot repro using comment 5 STR (in bug
820217) with the patch (I can repro without), and I get a shader compilation
error on comment 6's URL.
Brad, not sure who would be able to repro, can you find someone? I'm stuck in a work week without a lot of devices.
Flags: needinfo?(blassey.bugs)
(In reply to Aaron Train [:aaronmt] from comment #6)
> I am able to reproduce this 100% of the time in Fennec via Quake Demo
> (http://www.quaddicted.com/stuff/WebQuake/Client/WebQuake.htm), and then
> closing the tab while running.

I couldn't run that demo. Upon loading, I get an error dialog saying "Error compiling shader: Compile failed".

Aaron since you were able to repo, can you test paul's patch?
Flags: needinfo?(blassey.bugs) → needinfo?(aaron.train)
I'm unable to reproduce the original crash anymore.
Flags: needinfo?(aaron.train)
Comment on attachment 761030 [details] [diff] [review]
patch

Review time it is, then!

Basically, the idea here is to make sure to destroy properly the stream.

We do that by making sure to not call the |Stop| method when we are in an initialized state, because it throws (we can be in an uninitialized state because the constructor fails because of OOM, says logcat).

Now, we catch the exception, which is good. What is not so good is that we returned early in case we had a exception, leaking a ref to the |output_unit|, not deleting it. Calling methods on this half-destroyed instance of the stream would do bizarre things.
Attachment #761030 - Flags: review?(kinetik)
Comment on attachment 761030 [details] [diff] [review]
patch

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

(In reply to Paul Adenot (:padenot) from comment #15)
> We do that by making sure to not call the |Stop| method when we are in an
> initialized state, because it throws (we can be in an uninitialized state

uninitialized? :-)

> because the constructor fails because of OOM, says logcat).

Looking at sa_stream_open, I'm not sure how output_unit would be set if the AudioTrack ctor failed, since it should've returned before initializing output_unit with a global ref.

::: media/libsydneyaudio/src/sydney_audio_android.c
@@ +281,5 @@
> +      }
> +
> +      (*jenv)->CallVoidMethod(jenv, s->output_unit, at.flush);
> +      (*jenv)->CallVoidMethod(jenv, s->output_unit, at.release);
> +      (*jenv)->DeleteGlobalRef(jenv, s->output_unit);

I think we need to delete the global ref any time it's set, so move this outside of the state test block.
Attachment #761030 - Flags: review?(kinetik) → review+
Attachment #761030 - Attachment is obsolete: true
Comment on attachment 762785 [details] [diff] [review]
Don't leak the track when calling stop in a bad state in Android sydneyaudio backend.

Well, not sure if this is worth it, the code is only present on release.

[Approval Request Comment]
Regression caused by (bug #): initial sydney audio for android landing 
User impact if declined: Crash when using HTML audio
Testing completed (on m-c, etc.):  code is not present anymore on m-c, m-a, m-b, only in release. Manual testing completed.
Risk to taking this patch (and alternatives if risky): crash
String or IDL/UUID changes made by this patch: none
Attachment #762785 - Flags: approval-mozilla-release?
Comment on attachment 762785 [details] [diff] [review]
Don't leak the track when calling stop in a bad state in Android sydneyaudio backend.

Release is in a week, so let's punt till FF22.
Attachment #762785 - Flags: approval-mozilla-release? → approval-mozilla-release-
This code is gone in 22 (we switched from the old sydneyaudio to the new cubeb audio library), so WONTFIX, I guess.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
Workforme is better unless the signature is morphed with the same STR.
Resolution: WONTFIX → WORKSFORME
Blocks: gecko-games
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: