1406831 - tighten AssertOnGraphThreadOrNotRunning() check

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, enhancement, P2)

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: bug 1406831 simplify AssertOnGraphThreadOrNotRunning()

mLifecycleState is always > LIFECYCLE_RUNNING when mDetectedNotRunning

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

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: bug 1406831 tighten AssertOnGraphThreadOrNotRunning() to not accept graph thread after mDetectedNotRunning

This also permits setting mDriver to null after mDetectedNotRunning, which is
useful for fixing bug 1406830.

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

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: bug 1406831 don't tolerate just owning the monitor if AssertOnGraphThreadOrNotRunning() is not called on the correct thread

Owning the monitor is not sufficient for consistent state if state can be
accessed without the monitor.

The requirements for SetCurrentDriver() are tighted to require both the
monitor and correct thread, as CurrentDriver() can be called either with the
monitor or on the graph thread.

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

Comment 4

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d388956e7dcf1f14e34b5ddaf0aa4182223a988e

Comment 5

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8916486 [details]
bug 1406831 simplify AssertOnGraphThreadOrNotRunning()

https://reviewboard.mozilla.org/r/187614/#review192646

Comment 6

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8916487 [details]
bug 1406831 tighten AssertOnGraphThreadOrNotRunning() to not accept graph thread after mDetectedNotRunning

https://reviewboard.mozilla.org/r/187616/#review192648

Comment 7

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8916488 [details]
bug 1406831 don't tolerate just owning the monitor if AssertOnGraphThreadOrNotRunning() is not called on the correct thread

https://reviewboard.mozilla.org/r/187618/#review192652

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6348624e870
simplify AssertOnGraphThreadOrNotRunning() r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/f534d96564e0
tighten AssertOnGraphThreadOrNotRunning() to not accept graph thread after mDetectedNotRunning r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/e3f39de40209
don't tolerate just owning the monitor if AssertOnGraphThreadOrNotRunning() is not called on the correct thread r=pehrsons

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/f6348624e870
https://hg.mozilla.org/mozilla-central/rev/f534d96564e0
https://hg.mozilla.org/mozilla-central/rev/e3f39de40209