Closed Bug 1295951 Opened 4 years ago Closed 4 years ago

Crash in SnowWhiteKiller::Trace

Categories

(Core :: Graphics, defect, critical)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 + fixed

People

(Reporter: marcia, Assigned: kip)

References

Details

(4 keywords)

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-5f56ebaa-0b3c-46e3-a849-92eab2160817.
=============================================================

Seen while looking at nightly crash stats: http://bit.ly/2aZoGZ6

Some part of the code crashes in dom/vr/VRDisplay.cpp:257. Adding ni on kgilbert.
Flags: needinfo?(kgilbert)
Group: gfx-core-security
This class is not properly rooting the JS objects. You need to ensure that |mozilla::HoldJSObjects(this);| has been called any time you hold JS objects (and call DropJSObjects in the dtor). The easiest way to do this is to put it in the ctor.

The class is also wrapper cached, so it will work out fine if the wrapper is preserved, but not otherwise.
I'm not entirely sure this is the cause of the crash, but it might be.

As a side note, you don't need MOZ_COUNT_CTOR/_DTOR for a refcounted class.
Blocks: 1250244
Component: XPCOM → Graphics
I am investigating now, thanks!
Flags: needinfo?(kgilbert)
Ask smaug or I for help if if you need it (though I'm going to be on PTO for the rest of the morning).
Is this likely to be nightly-only if it's VR related?
Flags: needinfo?(kgilbert)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5)
> Is this likely to be nightly-only if it's VR related?

Yes, this is likely to be localized to nightly-only and for people with Oculus (VR Headset) hardware only.  The "VRDisplay.cpp" file is only present in nightly as well.
Flags: needinfo?(kgilbert)
Assignee: nobody → kgilbert
I suspect that HoldJSObjects / DropJSObjects calls would be needed for the VRPose, VRDisplayCapabilities, VREyeParameters, and VRDisplay classes, all in this file.  I'll add a patch shortly to this effect.
Flags: needinfo?(continuation)
Please advise if this is the right thing to do, or if I missed anything?  Thanks for all your help!
Yes, that looks fine to me. I'm sorry this isn't more automatic.
Flags: needinfo?(continuation)
Attachment #8782132 - Flags: review?(continuation)
I'm doing a sanity test with VR hardware and a try push, then can commit if you are happy with the patch
(In reply to :kip (Kearwood Gilbert) from comment #11)
> I'm doing a sanity test with VR hardware and a try push, then can commit if
> you are happy with the patch
Changes didn't appear to affect WebVR running on real hardware.  If the patch is r+, I can commit once the try run completes.
Comment on attachment 8782132 [details] [diff] [review]
Bug 1295951 - Fix JS object rooting for WebVR DOM API classes

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

Looks good to me. Note that this will cause an extra hashtable lookup in the ctor and dtor, but hopefully that's not an issue.

::: dom/vr/VRDisplay.cpp
@@ +120,5 @@
>  {
>    return VRFieldOfViewBinding::Wrap(aCx, this, aGivenProto);
>  }
>  
> +NS_IMPL_CYCLE_COLLECTION_CLASS(VREyeParameters)

Why are all these lines showing up that don't look changed? Is it line endings or something?
Attachment #8782132 - Flags: review?(continuation) → review+
Blocks: 1186578
See Also: → 1296446
https://hg.mozilla.org/mozilla-central/rev/69d47888c5c8
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Crash Signature: [@ SnowWhiteKiller::Trace]
Group: gfx-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.