+++ This bug was initially created as a clone of Bug #814532 +++ Rev5 MacOSX Mountain Lion 10.8 mozilla-central opt test mochitest-1 on 2012-11-23 06:07:12 PST for push cfafa3e83938 slave: talos-mtnlion-r5-046 https://tbpl.mozilla.org/php/getParsedLog.php?id=17301851&tree=Firefox { 130464 INFO TEST-START | /tests/content/media/test/test_playback_rate.html 130465 INFO TEST-INFO | /tests/content/media/test/test_playback_rate.html | Started Fri Nov 23 2012 06:21:02 GMT-0800 (PST) (1353680462.637s) 130466 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | [started big.wav-0] Length of array should match number of running tests 130467 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | Pitch preservation should be enabled by default. 130468 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | [started sound.ogg-1] Length of array should match number of running tests 130469 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | Pitch preservation should be enabled by default. 130470 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | playbackRate should be initially 1.0 130471 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | defaultPlaybackRate should be initially 1.0 130472 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | PlaybackRate should be clamped to 0.25. 130473 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | playbackRate should be reset to 1.0 on play() call 130474 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | playbackRate should be initially 1.0 130475 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | defaultPlaybackRate should be initially 1.0 130476 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | PlaybackRate should be clamped to 0.25. 130477 INFO TEST-PASS | /tests/content/media/test/test_playback_rate.html | playbackRate should be reset to 1.0 on play() call libc++abi.dylib: pure virtual method called TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_playback_rate.html | Exited with code -6 during test run }

Linux: https://tbpl.mozilla.org/php/getParsedLog.php?id=17304580&tree=Mozilla-Inbound

I've managed to reproduce this on Linux. For some reason, the rate transposer is not created properly, hence the crash when we call a virtual method. In gdb, I got this, from inside the soundtouch::SoundTouch instance: (gdb) p pRateTransposer $2 = (soundtouch::RateTransposer *) 0x5a5a5a5a5a5a5a5a (jemalloc freed memory) Interestingly, during the same gdb session, |p mTimeStretcher->pRateTransposer| from the AudioStream scope (specifically mozilla::AudioStream::EnsureTimeStretcherInitialized) shows a valid address. Something very bizarre is happening. Jacek, any idea here?

(In reply to Paul ADENOT (:padenot) from comment #12) > I've managed to reproduce this on Linux. For some reason, the rate > transposer is not created properly, hence the crash when we call a virtual > method. > > In gdb, I got this, from inside the soundtouch::SoundTouch instance: > > (gdb) p pRateTransposer > $2 = (soundtouch::RateTransposer *) 0x5a5a5a5a5a5a5a5a (jemalloc freed > memory) > > Interestingly, during the same gdb session, |p > mTimeStretcher->pRateTransposer| from the AudioStream scope (specifically > mozilla::AudioStream::EnsureTimeStretcherInitialized) shows a valid address. > Something very bizarre is happening. > > Jacek, any idea here? No, not really. One thing I found while looking at this code is nsAutoRef used for storing soundtouch::SoundTouch. Is there any reason you didn't use nsAutoPtr? It seems to me like nsAutoRef isn't the reason of the crash (although that's the suspecious area of code), but changing it to nsAutoPtr should make things cleaner, unless I'm missing something. I will attach a patch.

BTW, I couldn't reproduce the bug locally.

I'm gonna leave a machine running the mochitest in a loop with this patch applied and some instrumentation overnight. We will see.

Comment on attachment 685685 [details] [diff] [review] Use nsAutoPtr instead of nsAutoRef Review of attachment 685685 [details] [diff] [review]: ----------------------------------------------------------------- This is indeed nicer.

This should be applied on top of Jacek's patch. Found the cause of the crashes (this also fixes bug 814532 that has the same root cause), we were not initializing the pointer properly, so the |EnsureTimeStretcherInitialized()| was not working properly every time. With this patch, I cannot reproduce locally anymore.

(In reply to Paul ADENOT (:padenot) from comment #25) > With this patch, I cannot reproduce locally anymore. Great, good to see it fixed. I've pushed the reviewed part: https://hg.mozilla.org/integration/mozilla-inbound/rev/2d3251f224f7

Comment on attachment 686121 [details] [diff] [review] Properly initialize mTimeStretcher to nullptr. r= > AudioStream::AudioStream() [...] >+ mTimeStretcher(nullptr) The nsAutoPtr default-constructor _does_ initialize itself to null[1], so this shouldn't be necessary (or have any functional effect), now that we've changed mTimeStretcher to nsAutoPtr. [1] https://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsAutoPtr.h#78

And yet it does not seem to crash anymore. Go figure.

Comment on attachment 686121 [details] [diff] [review] Properly initialize mTimeStretcher to nullptr. r= Oops. Yeah, we shouldn't need to initialize an nsAuto*. Something else must be going on.

No failures on m-i since my patch landed, no failures on m-c since merge and the last error was on try push from before the merge. I think we may assume that it's fixed.

I just pushed a potential patch for this in bug 821737. We will see.

Considering it has not crashed in a while, I believe the patch in bug 821737 fixed it.