1350568 - Crash in Abort | corrupted actor state | mozalloc_abort | NS_DebugBreak | mozilla::ipc::LogicError | mozilla::net::PStunAddrsRequestParent::SendOnStunAddrsAvailable

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-03133273-5f54-45d7-8d45-3ea152170324.
=============================================================

There is 1 crash in nightly 55 with buildid 20170324030205. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1345511.

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

Comment 1

[Michael Froman [:mjf]]

It would appear so, yes.  I'll have to find an IPC expert to help me understand what is happening.

Comment 2

[Nils Ohlmeier [:drno]]

It looks to me like there are two lines which can cause this:
  http://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PStunAddrsRequest.cpp#34
Or
  http://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PStunAddrsRequest.cpp#37

Comment 3

[Nils Ohlmeier [:drno]]

Which then points back to here: http://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PStunAddrsRequestParent.cpp#57

Comment 4

[Michael Froman [:mjf]]

It is definitely the second, based on this info from the crash report:
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0102, AdapterSubsysID: 20178086, AdapterDriverVersion: 9.17.10.4459
FP(D00-L1000-W00001000-T000) DWrite? DWrite+ xpcom_runtime_abort(###!!! ABORT: corrupted actor state: file c:/builds/moz2_slave/m-cen-w32-ntly-000000000000000/build/src/ipc/glue/ProtocolUtils.cpp, line 311)

I just don't know what the possible causes are for corrupted actor state.  The PStunAddrsRequestParent.cpp file is auto generated from the ipdl file.

Comment 5

[Nils Ohlmeier [:drno]]

From looking at this: http://searchfox.org/mozilla-central/search?q=symbol:F_%3CT_mozilla%3A%3Anet%3A%3APStunAddrsRequestParent%3E_0&redirect=false
I'm guessing that the auto generated code assumes each actor can only be used for a single request/response pair once and then has to die.
So I would guess it's either:
- something tries to re-use an actor, which appears to be not legit
- or callbacks are received before the request came in from the client

Comment 6

[Michael Froman [:mjf]]

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

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I see two problems:

1. The ActorDestroy method doesn't do anything:
http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/media/mtransport/ipc/StunAddrsRequestParent.cpp#43
For refcounted actors, it's necessary to set a flag or something that remembers that we're not allowed to send IPC messages anymore. Then you need to check that flag here:
http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/media/mtransport/ipc/StunAddrsRequestParent.cpp#68
and avoid sending the message.

2. I don't see anything that keeps |this| alive between the time these runnable are dispatched and when it runs:
http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/media/mtransport/ipc/StunAddrsRequestParent.cpp#56
http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/media/mtransport/ipc/StunAddrsRequestParent.cpp#56
I'm not familiar with WrapRunnable though. Maybe it does the right thing?

I suspect #2 is causing this crash, but #1 should also be fixed.

Comment 8

[Michael Froman [:mjf]]

Bill, thank you so much for the feedback!  I'd guessed at #1 based on a example from DNSRequestParent.cpp, but I had not considered #2.  Thank you.

Comment 9

[Michael Froman [:mjf]]

Attachment: Bug 1350568 - don't dispatch IPC call to StunAddrsRequestChild after content process IPC channel goes away.

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

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8853993 [details]
Bug 1350568 - don't dispatch IPC call to StunAddrsRequestChild after content process IPC channel goes away.

https://reviewboard.mozilla.org/r/125996/#review128548

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/777453c1b91a
don't dispatch IPC call to StunAddrsRequestChild after content process IPC channel goes away. r=jesup

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/777453c1b91a