Closed Bug 1649058 Opened 1 month ago Closed 1 month ago

MediaEngineDefaultVideoSource() will deadlock if used

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: jya, Assigned: achronop)

References

(Regression)

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/21f2b48e01f2e14a94e8d39a665b56fcc08ecbdb/dom/media/webrtc/MediaEngineDefault.cpp#46-56

will deadlock if called.

This was introduced in bug 1518946.

media::Await blocks the current thread, while waiting for the promise to be resolved.
The promise can't be resolved on the thread we're waiting on; nor can we wait on the same thread.

This is clearly documented there:
https://searchfox.org/mozilla-central/rev/21f2b48e01f2e14a94e8d39a665b56fcc08ecbdb/dom/media/systemservices/MediaUtils.h#187-188

Here Await is given the main thread for both the thread to resolve the promise and the thread to be waiting on.
So the code will block the mainthread and the promise to be resolved needs to dispatch a task to the main thread; which is blocked.

Flags: needinfo?(achronop)

This is true. However, it would be a problem if the method was called from the main thread. It is not and that's is why it has been implemented like that. I will add an assert to make sure that the method is not called from the main thread. Thank you for the report.

Severity: -- → S4
Flags: needinfo?(achronop)
Priority: -- → P3
Assignee: nobody → achronop

It would have been much more efficition to use SyncRunnable instead of MozPromise here which will unnecessarily dispatches at least 3 tasks (and creating the runnables)

Severity: S4 → N/A
Type: defect → enhancement
Pushed by achronopoulos@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d709e60c76e
Replace MozPromise with SyncRunnable and assert not main thread. r=jya
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
You need to log in before you can comment on or make changes to this bug.