Closed Bug 1032266 Opened 5 years ago Closed 5 years ago

ALSA crash in AudioStream::Shutdown when seeking during playback

Categories

(Core :: Audio/Video, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
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

People

(Reporter: jbeich, Assigned: padenot)

References

Details

(Whiteboard: ft:loop)

Attachments

(1 file)

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
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
Let's try changing OS so crash-stats see the bug.
OS: FreeBSD → Linux
Attached patch patchSplinter Review
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
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
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-
(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.
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
Closed: 5 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?
Keywords: leave-open
Target Milestone: --- → mozilla33
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+
You need to log in before you can comment on or make changes to this bug.