1494556 - Crash in TimerThread::RemoveTimer

Status: RESOLVED FIXED

(Core :: WebVR, defect)

Description

[Calixte Denizet (:calixte)]

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

Comment 1

[:kip (Kearwood Gilbert)]

I'll take this and investigate. This may be related to Bug 1494141 also.

Comment 2

[:kip (Kearwood Gilbert)]

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

Comment 3

[:kip (Kearwood Gilbert)]

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)

Comment 4

[:kip (Kearwood Gilbert)]

Attachment: Bug 1494556 - Remove VRListenerThread

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.

Comment 5

[:kip (Kearwood Gilbert)]

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

Comment 6

[:kip (Kearwood Gilbert)]

Updated patch to correct try failures.

New try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=beaca354028b32cc9c988abb7a97a2321bb55b20

Comment 7

[Intermittent Failures Robot]

2 failures in 2773 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-central: 2

Platform breakdown:
* linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1494556&startday=2018-09-24&endday=2018-09-30&tree=all

Comment 8

[:kip (Kearwood Gilbert)]

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.

Comment 9

[:kip (Kearwood Gilbert)]

I have confirmed that this patch works on hardware (Tested with a Windows MR headset on OpenVR)

Comment 10

[Daosheng Mu[:daoshengmu]]

Comment on attachment 9013021 [details]
Bug 1494556 - Remove VRListenerThread

Daosheng Mu[:daoshengmu] has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e03ce4ca0e44
Remove VRListenerThread r=daoshengmu

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e03ce4ca0e44