Closed Bug 494336 Opened 11 years ago Closed 11 years ago
_bug482461 .html randomly fails
I caught this random failure in a debugger with extensive logging enabled. Decoding appears to be stalled with mBufferExhausted set to 1 but we're still in DECODER_STATE_DECODING. Unfortunately gdb is super-lame in my Linux VM and can't give me stacks for non-main threads, but it seems to me that the loop "while (mDecodedFrames.IsEmpty() && !mIsDecodingCompleted)" needs to also check mBufferExhausted, otherwise we can get into a situation where mBufferExhausted is 1, the step decode thread is Wait()ing indefinitely under "if (mDecodeStateMachine->mBufferExhausted)", and the decoder thread is simply looping in its while loop. Of course I can't verify that my patch fixes the bug, but it does seem necessary. I moved the NotifyAll to after we set mBufferExhausted to true. We don't want the decoder thread to wake up, see that mBufferExhausted is false, and go to sleep again. I removed the check for mState == DECODER_STATE_DECODING because nothing could have released the monitor since the last check of mState, and this confusingly implies that something could have.
Attachment #379040 - Flags: review?(chris.double)
We probably don't need to block the release on this, but if I'm right and this fixes significant random failures, it'd be great to have.
11 years ago
Whiteboard: [orange] → [orange][needs landing][needs 191 approval]
Comment on attachment 379040 [details] [diff] [review] fix We think this fixes some random video test failures. The patch is very low risk, we just exit a loop in an extra case where it's clearly appropriate to exit.
Attachment #379040 - Flags: approval1.9.1?
Attachment #379040 - Flags: approval1.9.1? → approval1.9.1+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [orange][needs landing][needs 191 approval] → [orange][needs 191 landing]
Whiteboard: [orange][needs 191 landing] → [orange]
You need to log in before you can comment on or make changes to this bug.