Closed
Bug 934232
Opened 11 years ago
Closed 11 years ago
Assertion failure in pa_stream_set_state_callback()
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla28
Tracking | Status | |
---|---|---|
firefox28 | --- | fixed |
People
(Reporter: gomesbascoy, Assigned: kinetik)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file)
(deleted),
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release) Build ID: 20130917081302 Steps to reproduce: Tried to load many different songs quickly using mod.js (http://duckinator.net/mod.js/) that is probably using the already deprecated Audio Data API. Actual results: Firefox crashed after an assertion failure from PulseAudio: Assertion 's' failed at pulse/stream.c:2089, function pa_stream_set_state_callback(). Aborting. Aborted (core dumped) Expected results: I believe (didnt debugged anything) the origin of the error is in /source/media/libcubeb/src/cubeb_pulse.c line 504: stm->stream = WRAP(pa_stream_new)(stm->context->context, stream_name, &ss, &map); WRAP(pa_stream_set_state_callback)(stm->stream, stream_state_callback, stm); Since pa_stream_set_state_callback() checks if the first parameter ('s') is not 0, so you should check first what pa_stream_new is returning.
Updated•11 years ago
|
Severity: normal → critical
Component: Untriaged → Video/Audio
Product: Firefox → Core
Assignee | ||
Comment 1•11 years ago
|
||
I don't think this needs to be sec-sensitive, it's a simple assertion failure.
Group: core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•11 years ago
|
Version: 24 Branch → Trunk
Assignee | ||
Comment 2•11 years ago
|
||
Thanks for the bug report and analysis, you're completely correct that pa_stream_new failures should be handled. I've pushed a fix here: https://tbpl.mozilla.org/?tree=Try&rev=4e9f1d7629bf Builds will turn up here once they're done: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.com-4e9f1d7629bf/ Can you please test the resulting build and let me know how it goes? I'm curious about why pa_stream_new is failing on your system, so I've added some PA error logging to stderr too.
Flags: needinfo?(gomesbascoy)
Reporter | ||
Comment 3•11 years ago
|
||
Thanks for your reply Matthew, your fix looks alright and should work, I'll try it tonight. The reason I sent this to core_security is because the assert only checks if 's' is not 0: my concern was that maybe that's not enough and some more advanced validation is needed... but never mind, it was just an unfounded feeling.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to pera from comment #3) > Thanks for your reply Matthew, your fix looks alright and should work, I'll > try it tonight. Thanks, I fear the result is that it'll fix the crash but you'll get no audio now, so there may be a secondary issue we need to get to the bottom of. > The reason I sent this to core_security is because the assert only checks if > 's' is not 0: my concern was that maybe that's not enough and some more > advanced validation is needed... but never mind, it was just an unfounded > feeling. It's not a problem, it doesn't hurt to be careful!
Assignee | ||
Comment 6•11 years ago
|
||
The crash fix is simple, so let's take that and deal with any other problems in a new bug.
Assignee | ||
Comment 7•11 years ago
|
||
FWIW, I removed the pa_channel_map_init_auto because passing pa_stream_new NULL does the same thing.
Updated•11 years ago
|
Attachment #8334849 -
Flags: review?(paul) → review+
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fd5efa062a5
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3fd5efa062a5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(gomesbascoy)
Comment 11•9 years ago
|
||
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #10) > pera, can you please verify this is fixed in the latest Aurora builds? Canceling this long overdue request.
Flags: needinfo?(gomesbascoy)
You need to log in
before you can comment on or make changes to this bug.
Description
•