[e10s] shut down "ABORT: actor has been |delete|d" for PRenderFrameChild in mochitest browser chrome

RESOLVED FIXED in mozilla36

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

(Blocks: 1 bug)

Trunk
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Created attachment 8526942 [details]
assertion stack

In bug 1091766, I have a patch that changes our behavior when sending a message failing from doing an exit(0) to returning and continuing on.  In opt builds, in e10s mochitest-browser-chrome 2, this now causes us to hit the following error, across all platforms:
  ###!!! ABORT: actor has been |delete|d: file [...]obj-firefox/ipc/ipdl/PRenderFrameChild.cpp, line 359

It looks like this is in PRenderFrameChild::Write:
        id = (__v)->mId;
        if ((1) == (id)) {
            NS_RUNTIMEABORT("actor has been |delete|d");
        }

This seems to have happened between
  https://hg.mozilla.org/mozilla-central/rev/44cdce6fca3a
and
  https://hg.mozilla.org/mozilla-central/rev/aa72ddfe9f93

I'll see if I can narrow down what test is causing this any more.
(Assignee)

Comment 1

3 years ago
Well, it looks like now it is happening in bc1, bc2 and bc3, so I guess that's better in a way.
  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=82f7ce409c07
(Assignee)

Comment 2

3 years ago
I was able to reproduce this locally with:
  ./mach mochitest-browser --e10s addon-sdk/test/

Do you have any ideas here, nical?  Thanks.
Flags: needinfo?(nical.bugzilla)
(Assignee)

Updated

3 years ago
Summary: [e10s] ABORT: actor has been |delete|d in browser mochitest chrome 2 → [e10s] shut down "ABORT: actor has been |delete|d" for PRenderFrameChild in mochitest browser chrome
(Assignee)

Updated

3 years ago
tracking-e10s: --- → ?
(Assignee)

Comment 3

3 years ago
This is the stack for when PRenderFrameChild::DestroySubtree() is called:
#01: mozilla::dom::PBrowserChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) (/tmp/./PBrowserChild.cpp:3101)
#02: mozilla::dom::PContentChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) (/tmp/./PContentChild.cpp:6381)
#03: mozilla::dom::PContentChild::OnChannelClose() (/tmp/./PContentChild.cpp:6262)
#04: mozilla::ipc::MessageChannel::NotifyChannelClosed() (/home/amccreight/mc/ipc/glue/MessageChannel.cpp:1686)
#05: MessageLoop::RunTask(Task*) (/home/amccreight/mc/ipc/chromium/src/base/message_loop.cc:362)

This is the stack for when RenderFrameChild::Destroy() is called:
#01: mozilla::ipc::LoggingEnabledFor(char const*) (/tmp/../../dist/include/mozilla/ipc/ProtocolUtils.h:274)
#02: mozilla::dom::TabChild::DestroyWindow() (/home/amccreight/mc/dom/ipc/TabChild.cpp:1569)
#03: nsRefPtr<mozilla::dom::TabChildGlobal>::get() const (/home/amccreight/mc/xpcom/base/nsRefPtr.h:208)
#04: mozilla::dom::PBrowserChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) (/tmp/./PBrowserChild.cpp:3115)
#05: mozilla::dom::PContentChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) (/tmp/./PContentChild.cpp:6381)
#06: mozilla::dom::PContentChild::OnChannelClose() (/tmp/./PContentChild.cpp:6262)
#07: mozilla::ipc::MessageChannel::NotifyChannelClosed() (/home/amccreight/mc/ipc/glue/MessageChannel.cpp:1686)
#08: MessageLoop::RunTask(Task*) (/home/amccreight/mc/ipc/chromium/src/base/message_loop.cc:362)
(Assignee)

Comment 4

3 years ago
Created attachment 8527030 [details] [diff] [review]
Don't Destroy() RenderFrameChild if it is already ActorDestroy()ed.

I think this is what billm was suggesting to me.  Basically, we just set a flag when we call ActorDestroy, and just don't do anything in Destroy if it is set.

Pretty hacky, but it seems to fix the test case above.  Is this an okay approach for you, nical, or should something better be done?
Attachment #8527030 - Flags: feedback?(nical.bugzilla)
Comment on attachment 8527030 [details] [diff] [review]
Don't Destroy() RenderFrameChild if it is already ActorDestroy()ed.

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

It's good, we already have a pattern of doing this in some of the gfx protocols, see dxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/LayerTransactionChild.cpp#25
Attachment #8527030 - Flags: feedback?(nical.bugzilla) → feedback+

Updated

3 years ago
Flags: needinfo?(nical.bugzilla)
(Assignee)

Updated

3 years ago
Assignee: nobody → continuation
(Assignee)

Comment 6

3 years ago
Comment on attachment 8527030 [details] [diff] [review]
Don't Destroy() RenderFrameChild if it is already ActorDestroy()ed.

Linux debug try run looked good: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=cca308840f8a
Attachment #8527030 - Flags: review?(nical.bugzilla)

Updated

3 years ago
Attachment #8527030 - Flags: review?(nical.bugzilla) → review+
e10s test
Blocks: 984139
tracking-e10s: ? → +
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f03b99824f54
https://hg.mozilla.org/mozilla-central/rev/f03b99824f54
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.