Closed Bug 1418165 Opened 3 years ago Closed 3 years ago

Protect sVideoCaptureThread with sThreadMonitor

Categories

(Core :: WebRTC: Audio/Video, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: mchiang, Assigned: mchiang)

References

Details

Attachments

(1 file)

No description provided.
Assignee: nobody → mchiang
Comment on attachment 8929275 [details]
Bug 1418165 - Protect sVideoCaptureThread with sThreadMonitor.

https://reviewboard.mozilla.org/r/200576/#review205768

Might work, but I'm a bit concerned.

::: dom/media/systemservices/CamerasParent.cpp:341
(Diff revision 1)
> +  MonitorAutoLock lock(*sThreadMonitor);
>    MOZ_ASSERT(sVideoCaptureThread->thread_id() == PlatformThread::CurrentId());

It seems a bit excessive to add a lock here solely to protect a MOZ_ASSERT that tries to assert that we're actually *on* the thread in question.

Since we should always be on the thread, the lock seems unnecessary.

Could we either:

A) put #ifdef DEBUG around it and use {} to limit the scope of the lock to just around the assert? or

B) Remove the assert entirely?
Attachment #8929275 - Flags: review?(jib) → review+
Pushed by mchiang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dec59488ecf1
Protect sVideoCaptureThread with sThreadMonitor. r=jib
Rank: 25
Priority: -- → P3
https://hg.mozilla.org/mozilla-central/rev/dec59488ecf1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.