Backed out 2 changesets (bug 1349699) for Win8 opt xpcshell crashes at mozilla::ipc::MessageChannel::WillDestroyCurrentMessageLoop
authorPhil Ringnalda <philringnalda@gmail.com>
Mon, 03 Apr 2017 22:54:59 -0700
changeset 351042 ebc1182ae0a58e1e6d4641edd41913e2c879da64
parent 351041 a56372e9dc96187eedecebf29a7ddf8d1e180e0c
child 351043 4cda9336503172c5bad871155c1abf1c44f45fa9
push id88787
push userphilringnalda@gmail.com
push dateTue, 04 Apr 2017 05:55:09 +0000
treeherdermozilla-inbound@ebc1182ae0a5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1349699
milestone55.0a1
backs out5cc766c0864e83d4caaf4eb712d466a5c2d38809
13f1d3918fe5a9363479c48fe0d6620aa2d77fe0
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
Backed out 2 changesets (bug 1349699) for Win8 opt xpcshell crashes at mozilla::ipc::MessageChannel::WillDestroyCurrentMessageLoop Backed out changeset 5cc766c0864e (bug 1349699) Backed out changeset 13f1d3918fe5 (bug 1349699)
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/ipdl/ipdl/lower.py
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -483,20 +483,18 @@ 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(const char* aName,
-                               IToplevelProtocol *aListener)
-  : mName(aName),
-    mListener(aListener),
+MessageChannel::MessageChannel(IToplevelProtocol *aListener)
+  : mListener(aListener),
     mChannelState(ChannelClosed),
     mSide(UnknownSide),
     mLink(nullptr),
     mWorkerLoop(nullptr),
     mChannelErrorTask(nullptr),
     mWorkerLoopID(-1),
     mTimeoutMs(kNoTimeout),
     mInTimeoutSecondHalf(false),
@@ -625,58 +623,32 @@ 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 !defined(ANDROID)
-    if (!Unsound_IsClosed()) {
-#if defined(MOZ_CRASHREPORTER)
-        CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("ProtocolName"),
-                                           nsDependentCString(mName));
-#endif
-        MOZ_CRASH("MessageChannel destroyed without being closed");
-    }
-#endif
-
     if (gParentProcessBlocker == this) {
         gParentProcessBlocker = nullptr;
     }
 
-    if (mWorkerLoop) {
-        mWorkerLoop->RemoveDestructionObserver(this);
-    }
-
     mWorkerLoop = nullptr;
     delete mLink;
     mLink = nullptr;
 
     mOnChannelConnectedTask->Cancel();
 
     if (mChannelErrorTask) {
         mChannelErrorTask->Cancel();
@@ -699,18 +671,16 @@ 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)
@@ -777,17 +747,16 @@ 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,35 +67,34 @@ enum ChannelState {
     ChannelConnected,
     ChannelTimeout,
     ChannelClosing,
     ChannelError
 };
 
 class AutoEnterTransaction;
 
-class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver
+class MessageChannel : HasResultCodes
 {
     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(const char *aName,
-                            IToplevelProtocol *aListener);
+    explicit MessageChannel(IToplevelProtocol *aListener);
     ~MessageChannel();
 
     IToplevelProtocol *Listener() const {
         return mListener;
     }
 
     // "Open" from the perspective of the transport layer; the underlying
     // socketpair/pipe should already be created.
@@ -487,22 +486,17 @@ class MessageChannel : HasResultCodes, M
 
     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,17 +2663,16 @@ 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]),