bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

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

RESOLVED FIXED

Status

()

Core
Audio/Video
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: emorley, Assigned: padenot)

Tracking

({intermittent-failure})

Trunk
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
+++ 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
}
(Reporter)

Updated

6 years ago
No longer blocks: 438871
(Reporter)

Comment 1

6 years ago
Linux:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17304580&tree=Mozilla-Inbound
OS: Mac OS X → All
Hardware: x86_64 → All
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 12

6 years ago
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?

Comment 13

6 years ago
(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.

Comment 14

6 years ago
Created attachment 685685 [details] [diff] [review]
Use nsAutoPtr instead of nsAutoRef
Attachment #685685 - Flags: review?(paul)

Comment 15

6 years ago
BTW, I couldn't reproduce the bug locally.
(Assignee)

Comment 16

6 years ago
I'm gonna leave a machine running the mochitest in a loop with this patch applied and some instrumentation overnight. We will see.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 24

6 years ago
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+
(Assignee)

Comment 25

6 years ago
Created attachment 686121 [details] [diff] [review]
Properly initialize mTimeStretcher to nullptr. r=

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)

Updated

6 years ago
Assignee: nobody → paul
(Reporter)

Updated

6 years ago
Blocks: 814532
No longer depends on: 814532

Comment 26

6 years ago
(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
(Reporter)

Updated

6 years ago
Whiteboard: leave open → [leave open]
Comment hidden (Treeherder Robot)

Updated

6 years ago
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
(Assignee)

Comment 29

6 years ago
And yet it does not seem to crash anymore. Go figure.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
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+
Comment hidden (Treeherder Robot)

Comment 35

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Whiteboard: [leave open]
Comment hidden (Treeherder Robot)
(Assignee)

Updated

6 years ago
Duplicate of this bug: 814532
(Assignee)

Updated

6 years ago
Duplicate of this bug: 815106
(Reporter)

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 44

6 years ago
I just pushed a potential patch for this in bug 821737. We will see.
(Assignee)

Comment 45

6 years ago
Considering it has not crashed in a while, I believe the patch in bug 821737 fixed it.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.