Bug 1349699 - Assert when destroying a MessageLoop that a live MessageChannel is attached to (r=dvander)
☠☠ backed out by ebc1182ae0a5 ☠ ☠
authorBill McCloskey <billm@mozilla.com>
Mon, 20 Mar 2017 14:15:44 -0700
changeset 351094 13f1d3918fe5a9363479c48fe0d6620aa2d77fe0
parent 351093 a842ddcd299bd32a4a13f55f23b37de648a7cd13
child 351095 5cc766c0864e83d4caaf4eb712d466a5c2d38809
push id31599
push usercbook@mozilla.com
push dateTue, 04 Apr 2017 10:35:26 +0000
treeherdermozilla-central@891981e67948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs1349699
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1349699 - Assert when destroying a MessageLoop that a live MessageChannel is attached to (r=dvander) MozReview-Commit-ID: GGr5UqJl3ui
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/ipdl/ipdl/lower.py
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -483,18 +483,20 @@ private:
 
     // Next item in mChan->mTransactionStack.
     AutoEnterTransaction *mNext;
 
     // Pointer the a reply received for this message, if one was received.
     nsAutoPtr<IPC::Message> mReply;
 };
 
-MessageChannel::MessageChannel(IToplevelProtocol *aListener)
-  : mListener(aListener),
+MessageChannel::MessageChannel(const char* aName,
+                               IToplevelProtocol *aListener)
+  : mName(aName),
+    mListener(aListener),
     mChannelState(ChannelClosed),
     mSide(UnknownSide),
     mLink(nullptr),
     mWorkerLoop(nullptr),
     mChannelErrorTask(nullptr),
     mWorkerLoopID(-1),
     mTimeoutMs(kNoTimeout),
     mInTimeoutSecondHalf(false),
@@ -623,32 +625,48 @@ MessageChannel::CanSend() const
     if (!mMonitor) {
         return false;
     }
     MonitorAutoLock lock(*mMonitor);
     return Connected();
 }
 
 void
+MessageChannel::WillDestroyCurrentMessageLoop()
+{
+#if !defined(ANDROID)
+#if defined(MOZ_CRASHREPORTER)
+    CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("ProtocolName"),
+                                       nsDependentCString(mName));
+#endif
+    MOZ_CRASH("MessageLoop destroyed before MessageChannel that's bound to it");
+#endif
+}
+
+void
 MessageChannel::Clear()
 {
     // Don't clear mWorkerLoopID; we use it in AssertLinkThread() and
     // AssertWorkerThread().
     //
     // Also don't clear mListener.  If we clear it, then sending a message
     // through this channel after it's Clear()'ed can cause this process to
     // crash.
     //
     // In practice, mListener owns the channel, so the channel gets deleted
     // before mListener.  But just to be safe, mListener is a weak pointer.
 
     if (gParentProcessBlocker == this) {
         gParentProcessBlocker = nullptr;
     }
 
+    if (mWorkerLoop) {
+        mWorkerLoop->RemoveDestructionObserver(this);
+    }
+
     mWorkerLoop = nullptr;
     delete mLink;
     mLink = nullptr;
 
     mOnChannelConnectedTask->Cancel();
 
     if (mChannelErrorTask) {
         mChannelErrorTask->Cancel();
@@ -671,16 +689,18 @@ bool
 MessageChannel::Open(Transport* aTransport, MessageLoop* aIOLoop, Side aSide)
 {
     NS_PRECONDITION(!mLink, "Open() called > once");
 
     mMonitor = new RefCountedMonitor();
     mWorkerLoop = MessageLoop::current();
     mWorkerLoopID = mWorkerLoop->id();
 
+    mWorkerLoop->AddDestructionObserver(this);
+
     ProcessLink *link = new ProcessLink(this);
     link->Open(aTransport, aIOLoop, aSide); // :TODO: n.b.: sets mChild
     mLink = link;
     return true;
 }
 
 bool
 MessageChannel::Open(MessageChannel *aTargetChan, MessageLoop *aTargetLoop, Side aSide)
@@ -747,16 +767,17 @@ MessageChannel::OnOpenAsSlave(MessageCha
     aTargetChan->mMonitor->Notify();
 }
 
 void
 MessageChannel::CommonThreadOpenInit(MessageChannel *aTargetChan, Side aSide)
 {
     mWorkerLoop = MessageLoop::current();
     mWorkerLoopID = mWorkerLoop->id();
+    mWorkerLoop->AddDestructionObserver(this);
     mLink = new ThreadLink(this, aTargetChan);
     mSide = aSide;
 }
 
 bool
 MessageChannel::Echo(Message* aMsg)
 {
     nsAutoPtr<Message> msg(aMsg);
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -67,34 +67,35 @@ enum ChannelState {
     ChannelConnected,
     ChannelTimeout,
     ChannelClosing,
     ChannelError
 };
 
 class AutoEnterTransaction;
 
-class MessageChannel : HasResultCodes
+class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
 {
     friend class ProcessLink;
     friend class ThreadLink;
 
     class CxxStackFrame;
     class InterruptFrame;
 
     typedef mozilla::Monitor Monitor;
 
   public:
     static const int32_t kNoTimeout;
 
     typedef IPC::Message Message;
     typedef IPC::MessageInfo MessageInfo;
     typedef mozilla::ipc::Transport Transport;
 
-    explicit MessageChannel(IToplevelProtocol *aListener);
+    explicit MessageChannel(const char *aName,
+                            IToplevelProtocol *aListener);
     ~MessageChannel();
 
     IToplevelProtocol *Listener() const {
         return mListener;
     }
 
     // "Open" from the perspective of the transport layer; the underlying
     // socketpair/pipe should already be created.
@@ -486,17 +487,22 @@ class MessageChannel : HasResultCodes
 
     bool ShouldRunMessage(const Message& aMsg);
     void RunMessage(MessageTask& aTask);
 
     typedef LinkedList<RefPtr<MessageTask>> MessageQueue;
     typedef std::map<size_t, Message> MessageMap;
     typedef IPC::Message::msgid_t msgid_t;
 
+    void WillDestroyCurrentMessageLoop() override;
+
   private:
+    // This will be a string literal, so lifetime is not an issue.
+    const char* mName;
+
     // Based on presumption the listener owns and overlives the channel,
     // this is never nullified.
     IToplevelProtocol* mListener;
     ChannelState mChannelState;
     RefPtr<RefCountedMonitor> mMonitor;
     Side mSide;
     MessageLink* mLink;
     MessageLoop* mWorkerLoop;           // thread where work is done
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -2663,16 +2663,17 @@ class _GenerateProtocolActorCode(ipdl.as
         # Actor()
         ctor = ConstructorDefn(ConstructorDecl(self.clsname))
         side = ExprVar('mozilla::ipc::' + self.side.title() + 'Side')
         if ptype.isToplevel():
             ctor.memberinits = [
                 ExprMemberInit(ExprVar('mozilla::ipc::IToplevelProtocol'),
                                [_protocolId(ptype), side]),
                 ExprMemberInit(p.channelVar(), [
+                    ExprLiteral.String(_actorName(p.name, self.side)),
                     ExprCall(ExprVar('ALLOW_THIS_IN_INITIALIZER_LIST'),
                              [ ExprVar.THIS ]) ]),
                 ExprMemberInit(p.stateVar(),
                                [ p.startState() ])
             ]
         else:
             ctor.memberinits = [
                 ExprMemberInit(ExprVar('mozilla::ipc::IProtocol'), [side]),