Closed Bug 1494556 Opened Last year Closed Last year

Crash in TimerThread::RemoveTimer

Categories

(Core :: WebVR, defect, critical)

64 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 --- fixed

People

(Reporter: calixte, Assigned: kip)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-f04a90c1-d998-4c00-a44a-d88350180927.
=============================================================

Top 10 frames of crashing thread:

0 XUL TimerThread::RemoveTimer mfbt/RefPtr.h:332
1 XUL nsTimerImpl::CancelImpl xpcom/threads/nsTimerImpl.cpp:522
2 XUL nsTimer::Cancel xpcom/threads/nsTimerImpl.cpp:509
3 XUL mozilla::gfx::VRListenerThreadHolder::Shutdown gfx/vr/VRManager.cpp:302
4 XUL gfxPlatform::ShutdownLayersIPC gfx/thebes/gfxPlatform.cpp:1181
5 XUL mozilla::ShutdownXPCOM xpcom/build/XPCOMInit.cpp:898
6 XUL ScopedXPCOMStartup::~ScopedXPCOMStartup xpcom/build/XPCOMInit.cpp:817
7 XUL XREMain::XRE_main toolkit/xre/nsAppRunner.cpp:1410
8 XUL mozilla::BootstrapImpl::XRE_main toolkit/xre/nsAppRunner.cpp:5018
9 firefox main browser/app/nsBrowserApp.cpp:233

=============================================================

There are 2 crashes (from 2 installations) in nightly 64 starting with buildid 20180926132058. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1473397.

[1] https://hg.mozilla.org/mozilla-central/rev?node=49f48ff493c6
Flags: needinfo?(kgilbert)
I'll take this and investigate. This may be related to Bug 1494141 also.
Assignee: nobody → kgilbert
Flags: needinfo?(kgilbert)
See Also: → 1494141
We are refactoring much of the code in gfx/vr, moving most of the code that runs in the VRListenerThread into it's own process.  The remaining code will be non-blocking once this refactoring is complete (estimated 1 week).

I suspect the simplest way to solve this issue (and Bug 1494141), may be to simply remove the VRListenerThread and the related code starting and stopping this thread.  If this is done prior to completion of the refactoring for Bug 1473399 (Enable VRService thread by default), there would be a regression in responsiveness during detection of VR hardware due to blocking API calls moving off the thread.

This regression would affect only VR users, causing a momentary stutter when starting a WebVR site for the first time in a session, and be resolved once Bug 1473399 lands shortly.

I feel that this may be a reasonable compromise rather than fixing code that is about to be deleted anyways, allowing the proper fix in Bug 1473399 to land sooner
I'm preparing a patch that removes the VRListenerThread, and will land it here.

I've also added a follow-up bug to fix the regression caused by removing the VRListener thread:

Added Bug 1495104 (VRSystemManagerExternal::Enumerate should be non-blocking)
We are refactoring much of the code in gfx/vr, moving
most of the code that runs in the VRListenerThread into
it's own process.  The remaining code will be non-blocking
once this refactoring is complete.

In order to resolve some shutdown crashes, it is simpler
to remove the VRListenerThread and the related code
starting and stopping this thread.  If this is done
prior to completion of the refactoring for Bug 1473399
(Enable VRService thread by default), there would be a
regression in responsiveness during detection of VR
hardware due to blocking API calls moving off the thread.
Added a patch to remove the VRListener thread.  I am testing now to see if it works on real hardware.

Pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=524241af7d1a7d180c934772f8b12c4742081ec6
Try push now looks clean.

Have rebased patch.  Will do a sanity check to ensure VR support doesn't regress while using real hardware before landing.
I have confirmed that this patch works on hardware (Tested with a Windows MR headset on OpenVR)
Comment on attachment 9013021 [details]
Bug 1494556 - Remove VRListenerThread

Daosheng Mu[:daoshengmu] has approved the revision.
Attachment #9013021 - Flags: review+
Blocks: 1494490
https://hg.mozilla.org/mozilla-central/rev/e03ce4ca0e44
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.