Closed Bug 1256064 Opened 5 years ago Closed 5 years ago

crash in mozilla::MozPromise<T>::Private::Resolve<T> (called from MediaTimer::UpdateLocked)

Categories

(Core :: Audio/Video: Playback, defect)

46 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox45 --- unaffected
firefox46 + fixed
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: mats, Assigned: cpearce)

References

Details

(Keywords: crash, regression, topcrash-win)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-62d4edd9-0ebd-4f79-8a00-2cea52160311.
=============================================================

722 reported crashes in the past 28 days, all on 32-bit Windows.
It looks like it's spiking in v46.

Stack:

mozilla::MozPromise<bool, bool, 1>::Private::Resolve<bool const&>(bool const&, char const*)
mozilla::MozPromise<bool, bool, 1>::ForwardTo(mozilla::MozPromise<bool, bool, 1>::Private*)
mozilla::MozPromise<bool, bool, 1>::DispatchAll()
mozilla::MozPromise<bool, bool, 1>::Private::Resolve<bool>(bool&&, char const*)
mozilla::MediaTimer::UpdateLocked()
mozilla::MediaTimer::TimerFired()
nsTimerImpl::Fire()
PR_WaitCondVar
nsThreadPool::Run()
PR_EnterMonitor
NS_ProcessNextEvent(nsIThread*, bool)
mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)
MessageLoop::RunHandler()
[...]
[Tracking Requested - why for this release]:
possible crash regression in v46
Fwiw, it's quite high in the Top Crashers for Firefox 46.0b ranking, at #3 currently.
Flags: needinfo?(mats)
(In reply to Mats Palmgren (:mats) from comment #2)
> Fwiw, it's quite high in the Top Crashers for Firefox 46.0b ranking, at #3
> currently.

I think this must be a regression in enabling decoding unencrypted MP4/AAC/H.264 files with the Adobe EME plugin on WinXP and Windows without system decoders.
Flags: needinfo?(mats)
This is 3.4% of all Firefox 46.0b1 crashes, and therefore by far a topcrash.
Hi Chris -- This is a regression and blocking fx46 release; so this needs an owner.  You're the most logical choice.  Thanks!
Assignee: nobody → cpearce
Tracking, recent regression and topcrash.
Let's address the MediaTimer::UpdateLocked instances in this bug, and the others in other bugs.

MediaTimer::UpdateLocked seems to be low volume compared to the others.
Flags: needinfo?(cpearce)
Still the #2 top crash in beta 2, with 3.55% of total crashes.
At that volume this should probably block release.
Blocks: 1258604
There are at least two crashes being caught by the signature mozilla::MozPromise<T>::Private::Resolve<T> that can be distinguished between by looking further up the stack:

1. bug 1256065 - GMPVideoDecoder::GMPInitDone
2. bug 1256064 - MediaTimer::UpdateLocked (*this bug*)

I don't know how to make Socorro distinguish the two signatures. That would be most useful.

However based on manual inspection the GMPVideoDecoder::GMPInitDone crash appears to be the most frequent, and the cause of the topcrash. The fix has been uplifted and the crashes appear to have abated since the fix landed.

This bug's signature, MediaTimer::UpdateLocked, seems much less frequent. We should still fix and uplift the fix, but I don't think it should be considered a top crash.
No longer blocks: 1258604
jwwang: do you have any ideas about the crashing path being called from MediaTimer::UpdateLocked?
Flags: needinfo?(jwwang)
No idea yet...

The crash address is 0xc at http://hg.mozilla.org/releases/mozilla-release/annotate/60e96806ff1c/xpcom/threads/MozPromise.h#l676 which looks like |this| is a null pointer.

However, it is called from http://hg.mozilla.org/releases/mozilla-release/annotate/60e96806ff1c/xpcom/threads/MozPromise.h#l644 where aOther is passed from mChainedPromises[i] at http://hg.mozilla.org/releases/mozilla-release/annotate/60e96806ff1c/xpcom/threads/MozPromise.h#l635 which shouldn't be null.

I will wait for more crash reports to see if I can find any clues.
Flags: needinfo?(jwwang)
[Tracking Requested - why for this release]:

Reseting blocking-46, ni? to ensure that's noticed.

Liz: I wrote a python script to pull the mozilla::MozPromise<T>::Private::Resolve<T> crashes in the period 2016-03-01 -> 2016-03-24 in 46 beta out of socorro. I used these to count the distinct call paths that crashed in mozilla::MozPromise<T>::Private::Resolve<T> signature, and the call site that's tracked in this bug (called from MediaTimer::UpdateLocked) appeared 0 times out of 4941 crashes the in that period in that signature.

So I don't think this case of crash in mozilla::MozPromise<T>::Private::Resolve<T> (called from MediaTimer::UpdateLocked) is a top crash, so I am removing release blocking status.

Also, the other paths that were frequently crashing in mozilla::MozPromise<T>::Private::Resolve<T> are fixed too. Will comment elsewhere.

The MediaTimer::UpdateLocked case may not be fixed yet, so I'm leaving this bug open, but it's not a frequent crash.
Flags: needinfo?(lhenry)
Chris, thanks very much for digging into that.  Which other bugs have the fixes for the crash we're worrying about?
Flags: needinfo?(lhenry)
Yep. The frequent cases are:

Bug 1259397 - crash in mozilla::MozPromise<T>::Private::Resolve<T> (called from GMPAudioDecoder::GMPInitDone or GMPVideoDecoder::GMPInitDone)
(fixed by bug 1256065)

Bug 1242774 - crash in mozilla::MozPromise<T>::Private::Resolve<T> (called from mozilla::media::DecodedAudioDataSink::Drained())
Chris, I think we can mark this fixed based on the fix in https://bugzilla.mozilla.org/show_bug.cgi?id=1242774#c47.
Flags: needinfo?(cpearce)
I've attached my counts of the mozilla::MozPromise<T>::Private::Resolve<T> crashes that occurred in the period 2016-01-1 to 2016-03-29.

The crashes that happened more than about 200 times or so are all fixed. These are the top crashes.

The MediaTimer crashes (which this bug is filed for) hasn't had any crashes in the past 15 days.

I don't know what fixed this, but I think we can close the bug and move on.
Flags: needinfo?(cpearce)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Resetting flags to fit with platform triage process. 
Chris would you also consider this fixed for 47?
Flags: needinfo?(cpearce)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #25)
> Resetting flags to fit with platform triage process. 
> Chris would you also consider this fixed for 47?

No crash reports in 47. I'd call this fixed.
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.