Closed Bug 1313557 Opened 3 years ago Closed 3 years ago

Data race: mIsShutdown in MediaDataDecoderProxy::InternalInit()

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

http://searchfox.org/mozilla-central/rev/4012e61cc38758ffa1f7ce26039173f67b048853/dom/media/platforms/agnostic/gmp/MediaDataDecoderProxy.cpp#27

Accessing mIsShutdown on the proxy thread is a data race because it is modified in Shutdown() on another thread.

In addition, the assertion is not right because there is no guarantee that InternalInit() always happens before Shutdown().
Assignee: nobody → jwwang
Depends on: 1146086
Priority: -- → P3
Attachment #8805420 - Flags: review?(jyavenard)
Comment on attachment 8805420 [details]
Bug 1313557 - remove the assertion from InternalInit() to avoid data race.

https://reviewboard.mozilla.org/r/89138/#review88300

to me it would be nicer to make this thread-safe by making IsShutdown atomic.
Also, if we're shutdown, there's no point attempting to initialise the GMP so may as well reject the promise immediately.

But seeing this is done only for debugging purposes... meh
Attachment #8805420 - Flags: review?(jyavenard) → review+
Comment on attachment 8805420 [details]
Bug 1313557 - remove the assertion from InternalInit() to avoid data race.

https://reviewboard.mozilla.org/r/89138/#review88300

It would be optimal to do that. But it won't buy us much because the chance of InternalInit() and Shutdown() happening at the same time is quite low.
Comment on attachment 8805420 [details]
Bug 1313557 - remove the assertion from InternalInit() to avoid data race.

https://reviewboard.mozilla.org/r/89138/#review88300

I'm amazed you would settle for less than perfect ! :)
Blocks: 1312337
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c50b4bafe2e
remove the assertion from InternalInit() to avoid data race. r=jya
https://hg.mozilla.org/mozilla-central/rev/9c50b4bafe2e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.