Closed Bug 884654 Opened 7 years ago Closed 7 years ago

Deadlock because of OmxDecoder::statusChanged()

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, firefox23 wontfix, firefox24 wontfix, firefox25 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

Attachments

(3 files, 1 obsolete file)

When OmxDecoder::statusChanged() is called. At first the function grab OMXCodecProxy's mutex and then nsBuiltinDecoder's ReentrantMonitor. The order is opposite of other places. This could make dead lock.
Assignee: nobody → sotaro.ikeda.g
blocking-b2g: --- → leo?
Can this bug impact watching of YouTube videos off of YouTube's HTML 5 mobile site significantly?
(In reply to Jason Smith [:jsmith] from comment #2)
> Can this bug impact watching of YouTube videos off of YouTube's HTML 5
> mobile site significantly?

It happens sometimes not significantly. But when the bug happens, only way is kill the app.
Confirmed the patch fix the deadlock on v1.1 leo.
The deadlock could be relatively easily regenerated by STR in Bug 884182 comment #0.
Attachment #764544 - Flags: review?(chris.double)
Comment on attachment 764544 [details] [diff] [review]
patch - fix deadlock come from OmxDecoder::statusChanged()

Review of attachment 764544 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/omx/OmxDecoder.cpp
@@ +767,5 @@
> +    case kNotifyPostReleaseVideoBuffer:
> +    {
> +      Mutex::Autolock autoLock(mSeekLock);
> +      // Free pending video buffers when OmxDecoder is not seeking video.
> +      // If OmxDecoder is in seeking video, the buffers are freed on seek exit. 

Remove trailing whitespace. Remove 'in'.

@@ +768,5 @@
> +    {
> +      Mutex::Autolock autoLock(mSeekLock);
> +      // Free pending video buffers when OmxDecoder is not seeking video.
> +      // If OmxDecoder is in seeking video, the buffers are freed on seek exit. 
> +      if (mIsVideoSeeking != true) {

if (!mIsVideoSeeking)
Attachment #764544 - Flags: review?(chris.double) → review+
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Apply the comment. Carry "chris.double: review+".
Attachment #764544 - Attachment is obsolete: true
A patch for b2g18. Carry "chris.double: review+".
Attachment #764802 - Flags: review+
Comment on attachment 764803 [details] [diff] [review]
patch v2 for b2g18 - fix deadlock come from OmxDecoder::statusChanged()

carry r+.
Attachment #764803 - Flags: review+
blocking-b2g: leo? → leo+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/879afe4b00b7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.