Closed Bug 1323804 Opened 7 years ago Closed 7 years ago

Crash in @0x0 | nsTimerImpl::Fire

Categories

(Core :: XPCOM, defect, P1)

x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: mccr8, Unassigned)

References

Details

(Keywords: crash, regression)

Crash Data

This bug was filed from the Socorro interface and is 
report bp-d47f3944-ce02-4eec-9284-b61c62161215.
=============================================================

I only see a few hundred crashes like this across all branches, but it looks like we sometimes try to run mCallback.c when it is null. It seems like we should null check that.
I don't see how mCallback.c can reasonably be nullptr while mCallbackType is CallbackType::Function.
It could be null if a timer was cross-thread cancelled while it was in the midst of firing. :(  Would be interesting to know if the crash was more or less common after bug 1157323 landed.
In the past 7 days there have been 228 crashes with this signature in Aurora 52, and 0 in Nightly 53, which suggests that something may have landed that fixed it. froydnj, any idea what it might have been? If it has been fixed, it would be nice to uplift the fix to Aurora.
Flags: needinfo?(nfroyd)
I think Ehsan has been refactoring some nsGlobalWindow code in FF53 that uses nsITimer.  I don't have any reason to believe it would cause this, but perhaps its related.

There may also be some changes related to requestIdleCallback that go through timers.
Maybe we should consider uplifting bug 132864 to beta if it lands in early enough in the release cycle.  It adds locking to the timer cancelation.
Blocks: 1328643
Crash Signature: [@ @0x0 | nsTimerImpl::Fire] → [@ @0x0 | nsTimerImpl::Fire] [@ nsTimerImpl::Fire ]
Keywords: regression
See Also: → 1313822
[Tracking Requested - why for this release]: #17 top crash for 52.0b in last 7 days.
Priority: -- → P1
this crash seems to have turned into [@0x0 | nsTimerEvent::Run] in 52.0b2
Crash Signature: [@ @0x0 | nsTimerImpl::Fire] [@ nsTimerImpl::Fire ] → [@ @0x0 | nsTimerImpl::Fire] [@ nsTimerImpl::Fire ] [@ @0x0 | nsTimerEvent::Run]
Tracking as regression and top crash in 52.
I would like to see what the patch from bug 1328643 does to this crash rate on Nightly before doing anything here.
Flags: needinfo?(nfroyd)
Looks like bug 1328643's patch helped the crash rate a lot! Should we uplift that?

Given it's no longer a super-top-crasher, should we un-track for 52?
Flags: needinfo?(nfroyd)
Flags: needinfo?(jcristau)
Got this backward...
No longer blocks: 1328643
Depends on: 1328643
See Also: → 1306733
(In reply to Andrew Overholt [:overholt] from comment #11)
> Looks like bug 1328643's patch helped the crash rate a lot! Should we uplift
> that?

I would be OK with uplifting that.
Flags: needinfo?(nfroyd)
Bug 1328643 seems to have fixed this.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Agreed, the rate seems to have fallen off a cliff starting with 52b5 as well. \m/
You need to log in before you can comment on or make changes to this bug.