Closed Bug 814708 Opened 12 years ago Closed 11 years ago

Intermittent test_playback_rate.html | Exited with code -6 during test run

Categories

(Core :: Audio/Video, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: emorley, Assigned: padenot)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

+++ 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
}
No longer blocks: 438871
Linux:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17304580&tree=Mozilla-Inbound
OS: Mac OS X → All
Hardware: x86_64 → All
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.
Attachment #685685 - Flags: review?(paul)
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.
Attachment #685685 - Flags: review?(paul) → review+
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.
Attachment #686121 - Flags: review?(kinetik)
Assignee: nobody → paul
Blocks: 814532
No longer depends on: 814532
(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
Whiteboard: leave open
Whiteboard: leave open → [leave open]
Blocks: 815106
Blocks: 816143
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.
Attachment #686121 - Flags: review?(kinetik) → review+
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.
Attachment #686121 - Flags: review+
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Status: RESOLVED → REOPENED
Resolution: 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.
Status: REOPENED → RESOLVED
Closed: 12 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.