ALSA crash in AudioStream::Shutdown when seeking during playback

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jbeich, Assigned: padenot)

Tracking

Trunk
mozilla33
All
Linux
Points:
---

Firefox Tracking Flags

(firefox30 unaffected, firefox31 unaffected, firefox32 fixed, firefox33 fixed, b2g-v1.3 unaffected, b2g-v1.3T unaffected, b2g-v1.4 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: ft:loop)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
1. Open http://hpr.dogphilosophy.net/test/opus.opus
2. Seek to any position a few times while the file is playing
3. It should crash with an assert() on alsa_stream_destroy

This happens even in linuxulator with Nightly binary form ftp.mozilla.org.

Assertion failed: (stm && (stm->state == INACTIVE || stm->state == ERROR)), function alsa_stream_destroy, file media/libcubeb/src/cubeb_alsa.c, line 870.

Program received signal SIGABRT, Aborted.
[Switching to Thread 81e60ec00 (LWP 100717 Media Audio)]
thr_kill () at thr_kill.S:3
3       thr_kill.S: No such file or directory.
(gdb) bt
#0  thr_kill () at thr_kill.S:3
#1  0x00000008012d5579 in abort () at lib/libc/stdlib/abort.c:65
#2  0x00000008012b60b1 in __assert (func=0x8074a9809 <.L__func__.alsa_stream_destroy> "alsa_stream_destroy", file=0xfffff80061a58000 <Address 0xfffff80061a58000 out of bounds>, line=1638236160, failedexpr=<optimized out>) at lib/libc/gen/assert.c:54
#3  0x0000000806316f80 in alsa_stream_destroy (stm=0x821b67580) at media/libcubeb/src/cubeb_alsa.c:870
#4  0x0000000806315903 in cubeb_stream_destroy (stream=0x821b67580) at media/libcubeb/src/cubeb.c:219
#5  0x000000080519c155 in nsAutoRefTraits<cubeb_stream>::Release (aStream=0x821b67580) at content/media/AudioStream.h:25
#6  0x000000080519c090 in nsAutoRefBase<cubeb_stream>::SafeRelease (this=0x821c08e60) at xpcom/base/nsAutoRef.h:667
#7  0x000000080519433f in nsAutoRef<cubeb_stream>::reset (this=0x821c08e60) at xpcom/base/nsAutoRef.h:197
#8  0x000000080517d0b0 in mozilla::AudioStream::Shutdown (this=0x821c08d00) at content/media/AudioStream.cpp:844
#9  0x000000080517c901 in mozilla::AudioSink::Cleanup (this=0x823332580) at content/media/AudioSink.cpp:225
#10 0x000000080517bb48 in mozilla::AudioSink::AudioLoop (this=0x823332580) at content/media/AudioSink.cpp:183
#11 0x000000080519cd0a in nsRunnableMethodImpl<void (mozilla::AudioSink::*)(), void, true>::Run (this=0x82332f820) at xpcom/glue/nsThreadUtils.h:391
#12 0x000000080262c092 in nsThread::ProcessNextEvent (this=0x8221ad800, aMayWait=false, aResult=0x7ffffe7cfd2e) at xpcom/threads/nsThread.cpp:762
#13 0x00000008025214b7 in NS_ProcessNextEvent (aThread=0x8221ad800, aMayWait=false) at xpcom/glue/nsThreadUtils.cpp:284
#14 0x0000000802c50f23 in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0x823bd46c0, aDelegate=0x8233c9740) at ipc/glue/MessagePump.cpp:337
#15 0x0000000802ba1576 in MessageLoop::RunInternal (this=0x8233c9740) at ipc/chromium/src/base/message_loop.cc:229
#16 0x0000000802ba14a5 in MessageLoop::RunHandler (this=0x8233c9740) at ipc/chromium/src/base/message_loop.cc:222
#17 0x0000000802ba147d in MessageLoop::Run (this=0x8233c9740) at ipc/chromium/src/base/message_loop.cc:196
#18 0x000000080262a766 in nsThread::ThreadFunc (aArg=0x8221ad800) at xpcom/threads/nsThread.cpp:342
#19 0x000000080ef1fed2 in _pt_root () from /usr/local/lib/libplds4.so.1
#20 0x0000000800f6d555 in thread_start (curthread=0x81e60ec00) at lib/libthr/thread/thr_create.c:284
#21 0x00007ffffe5d0000 in ?? ()
Cannot access memory at address 0x7ffffe7d0000
(Reporter)

Comment 1

4 years ago
Happens on Ubuntu as well if you move libpulse.so.0 out of the way.
Crash Signature: [@ cubeb_stream_destroy]
Summary: crash in AudioStream::Shutdown when seeking during playback → ALSA crash in AudioStream::Shutdown when seeking during playback
(Reporter)

Comment 2

4 years ago
Let's try changing OS so crash-stats see the bug.
OS: FreeBSD → Linux
(Assignee)

Comment 3

4 years ago
Created attachment 8453869 [details] [diff] [review]
patch

This was originally in bu 848954, but I've been asked to land it separately as it solves issues in other bugs. It has been r+ed by kinetik there.
Assignee: nobody → paul
Status: NEW → ASSIGNED
(Assignee)

Comment 4

4 years ago
Tree is closed, so setting checkin-needed because I'm going to bed and this is blocking jaoo's work.

This has been reviewed in 848954 (part 10), the only change is the commit message.
Keywords: checkin-needed
(Reporter)

Comment 5

4 years ago
Comment on attachment 8453869 [details] [diff] [review]
patch

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

::: media/libcubeb/src/cubeb_alsa.c
@@ +868,5 @@
>    cubeb * ctx;
>  
> +  assert(stm && (stm->state == INACTIVE ||
> +                 stm->state == ERROR ||
> +                 stm->state == DRAINING));

No difference at least on FreeBSD. The assert is still triggered when |stm->state == RUNNING|.
Attachment #8453869 - Flags: feedback-
(Assignee)

Comment 7

4 years ago
(In reply to Jan Beich from comment #5)
> Comment on attachment 8453869 [details] [diff] [review]
> patch
> 
> Review of attachment 8453869 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/libcubeb/src/cubeb_alsa.c
> @@ +868,5 @@
> >    cubeb * ctx;
> >  
> > +  assert(stm && (stm->state == INACTIVE ||
> > +                 stm->state == ERROR ||
> > +                 stm->state == DRAINING));
> 
> No difference at least on FreeBSD. The assert is still triggered when
> |stm->state == RUNNING|.

Interesting, this was not part of the summary, I'd have opened another bug otherwise. This needed to go in anyways. Maybe I can try to repro if I have a minute, this is likely to be a regression from the AudioSink split-off. Taking for investigation, because I think kinetik is super busy.
Keywords: leave-open
NOTE: Jan's failure has a different root cause than the one padenot is fixing for bug 1016277.   Padenot created a new bug for the report in Comment 5:  it's bug 1037423.
(Reporter)

Updated

4 years ago
No longer blocks: 948269
Crash Signature: [@ cubeb_stream_destroy]
Since we opened a new bug for the other issue (Bug 1037423), I'm marking this resolved fixed.

Paul -- can you write an aurora uplift approval for your patch to get it into v2.0?
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(paul)
Resolution: --- → FIXED
Whiteboard: ft:loop
Just curious. How did shutdown happen while draining? Shouldn't we shut down after draining?
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
status-firefox31: --- → unaffected
status-firefox32: --- → affected
status-firefox33: --- → fixed
Keywords: leave-open
Target Milestone: --- → mozilla33
(Assignee)

Comment 12

4 years ago
Comment on attachment 8453869 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: N/A, this has been an issues since cubeb landed ages ago, but way we called the code made it impossible to happen
[User impact if declined]: crash on assert when using Loop (the cubeb library has assert enabled even on release).
[Describe test coverage new/current, TBPL]: this is covered by existing tests and showed up when running tests for bug 1016277 on try (the test on the platform that fails, the b2g emulator, has been disabled for now).
[Risks and why]: this is needed for loop, and has gone though a month worth of try in bug 848954 (where this patch was originally from, part 10).
[String/UUID change made/needed]: none
Attachment #8453869 - Flags: feedback- → approval-mozilla-aurora?
Flags: needinfo?(paul)
Comment on attachment 8453869 [details] [diff] [review]
patch

Aurora+
Attachment #8453869 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-b2g-v1.3: --- → unaffected
status-b2g-v1.3T: --- → unaffected
status-b2g-v1.4: --- → unaffected
status-firefox30: --- → unaffected
https://hg.mozilla.org/releases/mozilla-aurora/rev/e7360cef9de1
status-b2g-v2.0: affected → fixed
status-firefox32: affected → fixed
You need to log in before you can comment on or make changes to this bug.