Closed Bug 1340041 Opened 4 years ago Closed 4 years ago

Intermittent dom/media/mediasource/test/test_FrameSelection.html | application crashed [@ PR_Unlock]


(Firefox for Android Graveyard :: Audio/Video, defect)

Not set


(firefox52 unaffected, firefox53 affected, firefox54 affected)

Tracking Status
firefox52 --- unaffected
firefox53 --- affected
firefox54 --- affected


(Reporter: intermittent-bug-filer, Assigned: jhlin)


(Keywords: intermittent-failure)


(1 file)

0!PR_Unlock [ptsynch.c:e783bdf2cb50 : 203 + 0x0]
    r0 = 0x2a04ea28    r1 = 0x00000000    r2 = 0x00000001    r3 = 0x638a49bc
    r4 = 0x51dad9e4    r5 = 0x00410050    r6 = 0x00000000    r7 = 0x51dad9e8
    r8 = 0x000080e8    r9 = 0x00000000   r10 = 0x00000000   r12 = 0x52e06e4c
    fp = 0x51dada04    sp = 0x51dad988    lr = 0x52d77061    pc = 0x52d77060
   Found by: given as instruction pointer in context
1!mozilla::BaseAutoLock<mozilla::Mutex>::~BaseAutoLock [Mutex.h:e783bdf2cb50 : 75 + 0x7]
    r4 = 0x51dad9e4    r5 = 0x51dad9ec    r6 = 0x00000000    r7 = 0x51dad9e8
    r8 = 0x000080e8    r9 = 0x00000000   r10 = 0x00000000    fp = 0x51dada04
    sp = 0x51dad998    pc = 0x55b84d61
   Found by: call frame info
2!mozilla::RemoteVideoDecoder::CallbacksSupport::HandleOutput [RemoteDataDecoder.cpp:e783bdf2cb50 : 200 + 0x5]
Flags: needinfo?(jolin)
Product: Core → Firefox for Android
Tried to reproduce locally but only got time-out failures. :(
The crash happened when destructing autolock of a mutex, I could only imagine that the mutex is destroyed before unlocking, which means its owner (RemoteVideoDecoder) is destroyed when handling output buffer.

Jean-Yves, is it possible that Shutdown() is called before all outputs are collected by Drain()? If so, will protecting callbacks with mutex or dispatching them to mTaskQueue fix this? Thanks.
Flags: needinfo?(jolin) → needinfo?(jyavenard)
It is possible for Shutdown to be called before Drain has completed.

However, Flush will be called first and only once Flush has completed (the FlushPromise has been resolved that is) will Shutdown be called.

Once Shutdown has been called, the RemoteDataDecoder can no longer use its callback.
The crash indicates that the decoder has been destroyed, so you're in a UAF sitatuation here. Dispatching to the taskqueue won't help, the taskqueue has been destroyed too.

If mDecoder has been destroyed, then the JavaCallbackSupport objet has been destroyed too.

Looks like the Java decoder continues to use the callback, even after mJavaDecoder->Flush(); has been called and even after:
  if (mJavaDecoder) {
    mJavaDecoder = nullptr;

has been done...
Flags: needinfo?(jyavenard)
Comment on attachment 8839806 [details]
Bug 1340041 - make callbacks no-op for dying CodecProxy.
Attachment #8839806 - Flags: review?(nchen) → review+
Pushed by
make callbacks no-op for dying CodecProxy. r=jchen
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Does this affect older versions?
Assignee: nobody → jolin
Flags: needinfo?(jolin)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> Does this affect older versions?

Will request uplifting to aurora. Thanks!
Flags: needinfo?(jolin)
Comment on attachment 8839806 [details]
Bug 1340041 - make callbacks no-op for dying CodecProxy.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1257777 
[User impact if declined]: browser crash
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: it avoids race condition
[String changes made/needed]: none
Attachment #8839806 - Flags: approval-mozilla-aurora?
Backed out in for causing near-permaorange in media/mediasource/test/test_FrameSelection.html and media/webspeech/synth/test/test_speech_simple.html on Android, e.g.
Resolution: FIXED → ---
Target Milestone: Firefox 54 → ---
Comment on attachment 8839806 [details]
Bug 1340041 - make callbacks no-op for dying CodecProxy.

Probably better to wait on that approval request.
Attachment #8839806 - Flags: approval-mozilla-aurora?
There is a better fix in bug 1353313. Mark as dup.
Closed: 4 years ago4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1353313
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.