Closed Bug 1212220 Opened 5 years ago Closed 5 years ago

It seems not safe to read sVideoQueueSendToCompositorSize off the main thread

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

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?
Blocks: 1194918
Flags: needinfo?(roc)
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?
Flags: needinfo?(roc)
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.
Assignee: nobody → jwwang
Bug 1212220 - cache pref values so they are safe to access off the main thread. r=roc.
Attachment #8672388 - Flags: review?(roc)
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
Thanks!
https://hg.mozilla.org/mozilla-central/rev/ab08acaa0bcb
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.