1212220 - It seems not safe to read sVideoQueueSendToCompositorSize off the main thread

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

https://hg.mozilla.org/mozilla-central/diff/5ec0906f885c/dom/media/MediaDecoderStateMachine.cpp#l1.34

I don't have a clear understanding about the pref system. However, it appears to be a data race to me to read sVideoQueueSendToCompositorSize on the state machine thread while the pref system updates it on the main thread without any means of synchronization.

Do we really want to use |Preferences::AddUintVarCache| so the value changes as soon as the pref value changes without reboot or is |Preferences::GetUint| just fine enough?

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You're right. Technically this is a bug. One way to fix it would be to make the cached value an Atomic and extend the Preferences interface to allow caching in an Atomic. Another would be to store it in the MDSM when the MDSM is constructed. Take your pick?

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

I guess |Preferences::GetUint| is fine enough for we don't really need to get a live update for sVideoQueueSendToCompositorSize when pref "media.video-queue.send-to-compositor-size" changes. I will store the value in MDSM when it is constructed.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51a7a2d09c36

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1212220 - cache pref values so they are safe to access off the main thread. r=roc.

Bug 1212220 - cache pref values so they are safe to access off the main thread. r=roc.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8672388 [details]
MozReview Request: Bug 1212220 - cache pref values so they are safe to access off the main thread. r=roc.

https://reviewboard.mozilla.org/r/21691/#review19497

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://reviewboard.mozilla.org/r/21691/#review19499

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab08acaa0bcb

Comment 9

[Carsten Book [:Tomcat]]

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