1640009 - Crash in [@ PLDHashTable::Iterator::Iterator | mozilla::gfx::VRManager::ShutdownVRManagerParents]

Status: RESOLVED FIXED

(Core :: WebVR, defect)

Description

[Andrew McCreight [:mccr8]]

This bug is for crash report bp-d0f57bfe-a996-4a3b-a944-9cb530200521.

Top 10 frames of crashing thread:

0 xul.dll PLDHashTable::Iterator::Iterator xpcom/ds/PLDHashTable.cpp:713
1 xul.dll mozilla::gfx::VRManager::ShutdownVRManagerParents gfx/vr/VRManager.cpp:1134
2 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/gfx/vr/ipc/VRManagerParent.cpp:131:7'>::Run xpcom/threads/nsThreadUtils.h:574
3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1211
4 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:302
5 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
7 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:444
8 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:399
9 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:139

The URLs look pretty generic and not specifically WebVR-related (Facebook, Twitch, YouTube, Reddit, etc.) It looks like this first showed up in the 20200519160549 build.

Comment 1

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: #4 top crash for the May 20th Windows Nightly, and a regression.

Here's the regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=96c90df47bdab0cebeed56e31dbaf2d9ed5b97f6&tochange=8f68705097b4bf88cd61b43b14401cde98ac75b6

Not much there, but bug 1637890 involves changing something with shutdown, and this crash is happening during shutdown, so maybe that's to blame.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1637890

Comment 3

[Julien Cristau [:jcristau]]

Jean-Yves, can you take a look?

Comment 4

[Jean-Yves Avenard [:jya]]

The VRManager code is racy.
First access to the CompositorThread is done via the global CompositorThread() this would be access on the main thread, and cleared on the main thread.

VRManager keeps a reference to the CompositorThreadHolder() but never use that to access it.

Then we have VRManager::MaybeGet() being used from the CompositorThread by VRManagerParent::ShutdownInternal

VRManager::MaybeGet() returns sVRManagerSingleton ; but this is cleared on the mainthread by the shutdown manager.

VRManager::Shutdown() gets called on the CompositorThread when the compositor thread gets shutdown, and it dispatches a task to clear the VRManager

At the same time, we have the VRManager::sVRManagerSingleton being called on the main thread that too will shutdown the VRManager.

The shutdown tasks since bug 1637890 are run in LIFO order which could have changed the timing of events.

What appears here however is that we have two tasks being dispatched on two different thread to shutdown the VRManager, if the main thread ones win, it may delete sVRManagerSingleton

To access sVRManagerSingleton the code does the following on the compositor thread:

void VRManagerParent::ShutdownInternal() {
VRManager* vm = VRManager::MaybeGet();
if (!vm) {
return;
}
vm->ShutdownVRManagerParents();
}

If the main thread task run first; vm can become a dangling pointer

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1640009 - Hold a reference to VRManager singleton for the duration of the parent shutdown. r?sotaro

Comment 6

[Andrew McCreight [:mccr8]]

Thanks for the fix, jya. I'm going to mark this as sec-moderate because it is happening late in shutdown.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/f9f4bdf52d9b437a8a95484dd2563c4b45baabfd
https://hg.mozilla.org/mozilla-central/rev/f9f4bdf52d9b