Closed Bug 1343666 Opened 3 years ago Closed 3 years ago

Assertion failure: sVRManagerChildSingleton

Categories

(Core :: Graphics, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed

People

(Reporter: truber, Assigned: kip)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [gfx-noted])

Attachments

(3 files, 1 obsolete file)

Attached file testcase.html
The attached testcase causes an assertion failure at shutdown in mozilla-central rev 34c6c2f302e7.

Assertion failure: sVRManagerChildSingleton, at /home/worker/workspace/build/src/gfx/vr/ipc/VRManagerChild.cpp:75
#01: mozilla::dom::VREventObserver::DisconnectFromOwner at dom/vr/VREventObserver.cpp:49
#02: mozilla::dom::VREventObserver::Release at memory/mozalloc/mozalloc.h:218
#03: nsGlobalWindow::~nsGlobalWindow at xpcom/ds/nsTArray.h:2270
#04: nsGlobalWindow::~nsGlobalWindow at dom/base/nsGlobalWindow.cpp:1707
#05: SnowWhiteKiller::~SnowWhiteKiller at mfbt/SegmentedVector.h:309
#06: nsCycleCollector::FreeSnowWhite at xpcom/base/nsCycleCollector.cpp:2831
#07: nsCycleCollector::BeginCollection at xpcom/base/nsCycleCollector.cpp:3828
#08: nsCycleCollector::Collect at xpcom/base/nsCycleCollector.cpp:3650
#09: nsCycleCollector::ShutdownCollect at xpcom/base/nsCycleCollector.cpp:3591
Flags: needinfo?(kgilbert)
Priority: -- → P3
Whiteboard: [gfx-noted]
I'll take this and investigate, thanks!
Assignee: nobody → kgilbert
Flags: needinfo?(kgilbert)
I see the logic error in VREventObserver::DisconnectFromOwner()...

I need to check VRManagerChild::IsCreated() rather than checking if VRManagerChild::Get() returns a nullptr.

This bug should affect only debug builds.

Patch incoming..
Attachment #8843056 - Flags: review?(dmu)
Comment on attachment 8843056 [details]
Bug 1343666 - Prevent crash on shutdown due to assertion in VRManagerChild::Get()

https://reviewboard.mozilla.org/r/116806/#review118582

LGTM
Attachment #8843056 - Flags: review?(dmu) → review+
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06074f7e8901
Prevent crash on shutdown due to assertion in VRManagerChild::Get() r=daoshengmu
https://hg.mozilla.org/mozilla-central/rev/06074f7e8901
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Can we land this attached testcase as a crashtest? Also, does this need to be backported or can it ride the 54 train?
Flags: needinfo?(kgilbert)
Flags: in-testsuite?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)
> Can we land this attached testcase as a crashtest? Also, does this need to
> be backported or can it ride the 54 train?
WebVR will not be enabled by default in release 54 (it has slipped to 55); however, it is already enabled in Aurora.  I expect the incidence to be low of users encountering this.  It would be low-risk to back-port this to 54 if we see any reports there.

The testcase would be useful as a crashtest.  I'd be glad to get this in.
Flags: needinfo?(kgilbert)
This landed in time for 54 prior to Monday's uplift to Aurora, so it sounds like we're good in that respect. Thanks for taking care of getting the testcase landed as a crashtest too!
Attachment #8846195 - Attachment is obsolete: true
Attachment #8846853 - Flags: review?(daoshengmu)
Attachment #8846853 - Flags: review?(daoshengmu) → review?(dmu)
Comment on attachment 8846853 [details] [diff] [review]
Bug 1343666 - Part 2: Add Crashtest

Review of attachment 8846853 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, although I am interested in why you add "document.documentElement.removeAttribute("class");" at the end of this test file?
Attachment #8846853 - Flags: review?(dmu) → review+
Hi Kip and Daosheng
We're fixing data: URI to have its own unique origin in bug 1324406, I noticed you added a test that using data: URI in this bug, please don't do this anymore unless you're pretty sure what you're doing.

Thanks
You need to log in before you can comment on or make changes to this bug.