1350682 - Crash in mozalloc_abort | NS_DebugBreak | mozilla::ipc::LogicError | mozilla::layers::PVideoBridge::Transition

Status: RESOLVED FIXED

(Core :: WebVR, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-31ad1de6-652d-4cc5-a347-886332170326.
=============================================================

There are 2 crashes in nightly 55 with buildid 20170325030203. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1299937.

[1] https://hg.mozilla.org/mozilla-central/rev?node=5d8e162ef5f73106037a6ad5c34770f26ab86447

Comment 1

[Daosheng Mu[:daoshengmu]]

I think it would be something wrong from the IPDL build tool, we can try the clobber build and see what's happened next. Ideally, PVRManagerChild::SendStopVibrateHaptic should be "PVRManager::Transition(PVRManager::Msg_StopVibrateHaptic__ID, (&(mState)));" I will keep following this bug if it can be reproduced again.

Comment 2

[Calixte Denizet (:calixte)]

There are 2 new crashes with signature "mozalloc_abort | NS_DebugBreak | mozilla::ipc::LogicError | mozilla::gfx::PVsyncBridge::Transition".

Comment 3

[Daosheng Mu[:daoshengmu]]

It seems to be the actor of VRManagerChild has been deleted at somewhere. After some investigation, I don't find anything that is related to PVRManagerParent::Send__delete__(). I would like to add an assertion to help me determine whether it is because mVRChannelChild is deleted already.

Btw, I still think using a raw pointer for VRManagerChild is better than a RefPtr. If the root cause is the actor is deleted, we should just need to check IsDeleted().

Comment 4

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1350682 - Call VRManagerChild getter when need it at GamepadManager instead of keeping it;

Review commit: https://reviewboard.mozilla.org/r/124124/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124124/

Comment 5

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8851891 [details]
Bug 1350682 - Call VRManagerChild getter when need it at GamepadManager instead of keeping it;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124124/diff/1-2/

Comment 6

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8851891 [details]
Bug 1350682 - Call VRManagerChild getter when need it at GamepadManager instead of keeping it;

https://reviewboard.mozilla.org/r/124124/#review126702

LGTM

Comment 7

[Daosheng Mu[:daoshengmu]]

(In reply to Daosheng Mu[:daoshengmu] from comment #5)
> Comment on attachment 8851891 [details]
> Bug 1350682 - Call VRManagerChild getter when need it at GamepadManager
> instead of keeping it;
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/124124/diff/1-2/

I think it is pretty much like the original mVRManagerChild has been deleted when the sVRManagerChildSingleton is assigned by a new one. Therefore, using VRManagerChild::Get() is more correct.

Comment 8

[:kip (Kearwood Gilbert)]

Comment on attachment 8851891 [details]
Bug 1350682 - Call VRManagerChild getter when need it at GamepadManager instead of keeping it;

https://reviewboard.mozilla.org/r/124124/#review127192

This LGTM, Thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba032e6ed34b
Call VRManagerChild getter when need it at GamepadManager instead of keeping it; r=kip,Lenzak

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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