Closed Bug 1103107 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
e10s + ---

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file 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.
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
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)
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
tracking-e10s: --- → ?
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)
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+
Flags: needinfo?(nical.bugzilla)
Assignee: nobody → continuation
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)
Attachment #8527030 - Flags: review?(nical.bugzilla) → review+
e10s test
https://hg.mozilla.org/mozilla-central/rev/f03b99824f54
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.