Last Comment Bug 723793 - Lazily initialize libcubeb
: Lazily initialize libcubeb
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Matthew Gregan [:kinetik]
:
Mentors:
Depends on: 944707
Blocks: cubeb
  Show dependency treegraph
 
Reported: 2012-02-02 18:09 PST by Matthew Gregan [:kinetik]
Modified: 2013-12-08 14:39 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Lazily initialize libcubeb on first use. (2.08 KB, patch)
2012-02-02 18:13 PST, Matthew Gregan [:kinetik]
roc: review+
Details | Diff | Review
Lazily initialize libcubeb on first use. (2.12 KB, patch)
2012-02-02 18:25 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Review

Description Matthew Gregan [:kinetik] 2012-02-02 18:09:12 PST
Initializing libcubeb from nsAudioStream::InitLibrary means it is initialized even in situations where it'll never be used (e.g. inside xpcshell).  It also breaks the delayload optimization for the gkmedias DSO added in bug 712175.
Comment 1 Matthew Gregan [:kinetik] 2012-02-02 18:13:28 PST
Created attachment 594045 [details] [diff] [review]
Lazily initialize libcubeb on first use.
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-02-02 18:20:28 PST
Comment on attachment 594045 [details] [diff] [review]
Lazily initialize libcubeb on first use.

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

::: content/media/nsAudioStream.cpp
@@ +963,4 @@
>  
>    {
>      cubeb_stream* stream;
> +    if (cubeb_stream_init(GetCubebContext(), &stream, "nsBufferedAudioStream", params,

Hoist this out so we only have to take the lock once per Init.
Comment 3 Matthew Gregan [:kinetik] 2012-02-02 18:25:34 PST
Created attachment 594047 [details] [diff] [review]
Lazily initialize libcubeb on first use.
Comment 4 Matthew Gregan [:kinetik] 2012-02-02 19:02:21 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/60fc46d5b1ca
Comment 5 Phil Ringnalda (:philor) 2012-02-02 23:38:47 PST
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/62b24f89b354 for a couple of unhappy Windows crashtest runs:

https://tbpl.mozilla.org/php/getParsedLog.php?id=9054674&tree=Mozilla-Inbound
REFTEST TEST-START | file:///C:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/369126-1.html | 402 / 2012 (19%)
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
###!!! ABORT: Main-thread-only object used off the main thread: file e:/builds/moz2_slave/m-in-w32/build/xpcom/base/nsCycleCollector.cpp, line 1307
###!!! ABORT: Main-thread-only object used off the main thread: file e:/builds/moz2_slave/m-in-w32/build/xpcom/base/nsCycleCollector.cpp, line 1307
TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/369126-1.html | Exited with code 3 during test run

https://tbpl.mozilla.org/php/getParsedLog.php?id=9054770&tree=Mozilla-Inbound
REFTEST TEST-START | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/436900-2.html | 409 / 2012 (20%)
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/436900-2.html | Exited with code 3 during test run
Comment 6 Phil Ringnalda (:philor) 2012-02-02 23:55:03 PST
Hmm, now I'm deeply confused.

Those two failures were a pair of oranges, on the push two after this and the push three after this.

Once I backed this out for clearly being at fault there, then I looked at a pair of purple Windows crashtests, one push after this and one push before this, and they were

https://tbpl.mozilla.org/php/getParsedLog.php?id=9052396&tree=Mozilla-Inbound
REFTEST TEST-PASS | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/403574-1.xhtml | (LOAD ONLY)
REFTEST INFO | Loading a blank page
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/403574-1.xhtml | application timed out after 330 seconds with no output

https://tbpl.mozilla.org/php/getParsedLog.php?id=9054106&tree=Mozilla-Inbound
REFTEST TEST-START | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/430628-1.html | 405 / 2012 (20%)
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
Assertion failed: r == MMSYSERR_NOERROR, file e:/builds/moz2_slave/m-in-w32/build/media/libcubeb/src/cubeb_winmm.c, line 398
TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/test/build/reftest/tests/docshell/base/crashtests/430628-1.html | application timed out after 330 seconds with no output

So, um, the assertions were expected and nonfatal and already present, and this *did* cause crashes? Or, you just picked an astonishingly bad time to push it, when seemingly-related failures were planning on happening anyway? Or, your patch actually travelled backward in time, to affect the push before you pushed it?
Comment 7 Matthew Gregan [:kinetik] 2012-02-03 00:28:54 PST
This must be a problem with the original libcubeb push (bug 623444).  I'll disable that code with a pref change for now.
Comment 8 Matthew Gregan [:kinetik] 2012-04-15 20:02:43 PDT
Relanded:

http://hg.mozilla.org/integration/mozilla-inbound/rev/b99da5d23b1b
Comment 9 :Ehsan Akhgari (out sick) 2012-04-16 08:33:21 PDT
https://hg.mozilla.org/mozilla-central/rev/b99da5d23b1b

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