1341666 - Assertion failure: false (We should be reviving the graph?), at /home/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:4012

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Description

[James Graham [:jgraham]]

I'm trying to enable leak checking for web-platform-tests. In doing so I ran into some leaks so turned on --run-by-dir for the testrun to try to narrow down which tests were leaking. Unfortunately the main effect was to start getting assertions from MediaStreamGraph:

Assertion failure: false (We should be reviving the graph?), at /home/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:4012

Try run is:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9442fabaefdaca00fce0acb6c63afbf600c3fb1a&selectedJob=79384294

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Padenot, can you help triage this?

Comment 2

[James Graham [:jgraham]]

Sorry, I don't know how this ranking stuff works… how do I tell when this will be looked at? It's a blocker for turning on leak checking in web-platform-tests.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

p1 (for our team) means typically we hope to address it within the next quarter or so, and usually ahead of p2 bugs.  Rank is how important it is; for p1 the values are 1 to 19 (19 being the lowest-importance p1 bugs).  Sec bugs for example might be 5 or 10; average p1 bugs are 15.

This is a MSG-shutdown bug, where AudioContext (WebAudio) is trying to a Suspend, Resume or Close via a runnable, and by the time it runs we're in shutdown for the MediaStreamGraph.  Likely the assertion here (which is debug-only) is overkill, and in fact we should simply ignore it or just log it; Suspend, Resume and Close operations are moot if the graph is being shut down.

Paul - just remove it?

Comment 4

[James Graham [:jgraham]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4dfd66a5aa1afce9693efbf2de6b523347bef4a5 is the previous try run. Note that that includes a patch to delete the assert in this bug, just to see what else is problematic.

Comment 5

[Paul Adenot (:padenot)]

Pushed a tentative patch here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10f186b6ad96fc50428e849db189c14bb68c1087

Comment 6

[Paul Adenot (:padenot)]

James, have you had a chance to do a try push with the patch in comment 5 ?

Comment 7

[James Graham [:jgraham]]

Ah, yes, sorry. It works great. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d177eafc4f5af13decc5d054d1c0efad15b59c4&selectedJob=83146009 only has different problems :)

Comment 8

[Paul Adenot (:padenot)]

Attachment: Bug 1341666 - Allow running a `close` message during an MSG shutdown.

Review commit: https://reviewboard.mozilla.org/r/121380/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121380/

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8848464 [details]
Bug 1341666 - Allow running a `close` message during an MSG shutdown.

https://reviewboard.mozilla.org/r/121380/#review123520

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62f80de05fa2
Allow running a `close` message during an MSG shutdown. r=jesup

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c3b473b6247603ffe01f721f9579a4982674caa

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=62f80de05fa2039bc9be0e9cfed70d420e49ffbb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=84699527&repo=mozilla-inbound

[task 2017-03-17T18:33:58.669731Z] 18:33:58     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/media/Unified_cpp_dom_media2.cpp:92:0:
[task 2017-03-17T18:33:58.670087Z] 18:33:58     INFO -  /home/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp: In member function 'virtual void mozilla::MediaStreamGraph::ApplyAudioContextOperation(mozilla::MediaStream*, const nsTArray<mozilla::MediaStream*>&, mozilla::dom::AudioContextOperation, void*)::AudioContextOperationControlMessage::RunDuringShutdown()':
[task 2017-03-17T18:33:58.670274Z] 18:33:58     INFO -  /home/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:4012:192: error: use of parameter from containing function
[task 2017-03-17T18:33:58.670512Z] 18:33:58     INFO -         MOZ_ASSERT(aOperation == AudioContextOperation::Close, "We should be reviving the graph?");
[task 2017-03-17T18:33:58.670811Z] 18:33:58     INFO -                                                                                                                                                                                                  ^
[task 2017-03-17T18:33:58.671055Z] 18:33:58     INFO -  /home/worker/workspace/build/src/dom/media/MediaStreamGraph.cpp:3989:68: note: 'mozilla::dom::AudioContextOperation aOperation' declared here
[task 2017-03-17T18:33:58.671250Z] 18:33:58     INFO -                                                AudioContextOperation aOperation,
[task 2017-03-17T18:33:58.671369Z] 18:33:58     INFO -                                                                      ^
[task 2017-03-17T18:33:58.671531Z] 18:33:58     INFO -  /home/worker/workspace/build/src/config/rules.mk:1016: recipe for target 'Unified_cpp_dom_media2.o' failed

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed19875877c6
Allow running a `close` message during an MSG shutdown. r=jesup

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ed19875877c6

Comment 14

[Paul Adenot (:padenot)]

clearing NI.