Bug 1049801 - Cancel the DispatchOnChannelConnected runnable when destructing the MessageChannel. r=bent, a=sledru
authorBen Kelly <ben@wanderview.com>
Thu, 21 Aug 2014 14:13:23 -0400
changeset 217634 597626afe5e1f8f2a33187b4ca301ce39bd31cdd
parent 217633 e85f5894f07571d911c501ccd1134b0d5b39ea06
child 217635 5ff5ecda2dfbf5eb2f1880c0f8be086558114a16
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent, sledru
bugs1049801
milestone33.0a2
Bug 1049801 - Cancel the DispatchOnChannelConnected runnable when destructing the MessageChannel. r=bent, a=sledru
ipc/glue/MessageChannel.cpp
ipc/glue/MessageChannel.h
ipc/glue/MessageLink.cpp
--- a/ipc/glue/MessageChannel.cpp
+++ b/ipc/glue/MessageChannel.cpp
@@ -203,29 +203,35 @@ MessageChannel::MessageChannel(MessageLi
     mPendingUrgentReplies(0),
     mPendingRPCReplies(0),
     mCurrentRPCTransaction(0),
     mDispatchingSyncMessage(false),
     mDispatchingUrgentMessageCount(0),
     mRemoteStackDepthGuess(false),
     mSawInterruptOutMsg(false),
     mAbortOnError(false),
-    mFlags(REQUIRE_DEFAULT)
+    mFlags(REQUIRE_DEFAULT),
+    mPeerPidSet(false),
+    mPeerPid(-1)
 {
     MOZ_COUNT_CTOR(ipc::MessageChannel);
 
 #ifdef OS_WIN
     mTopFrame = nullptr;
     mIsSyncWaitingOnNonMainThread = false;
 #endif
 
     mDequeueOneTask = new RefCountedTask(NewRunnableMethod(
                                                  this,
                                                  &MessageChannel::OnMaybeDequeueOne));
 
+    mOnChannelConnectedTask = new RefCountedTask(NewRunnableMethod(
+        this,
+        &MessageChannel::DispatchOnChannelConnected));
+
 #ifdef OS_WIN
     mEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr);
     NS_ASSERTION(mEvent, "CreateEvent failed! Nothing is going to work!");
 #endif
 }
 
 MessageChannel::~MessageChannel()
 {
@@ -278,16 +284,18 @@ MessageChannel::Clear()
     // before mListener.  But just to be safe, mListener is a weak pointer.
 
     mDequeueOneTask->Cancel();
 
     mWorkerLoop = nullptr;
     delete mLink;
     mLink = nullptr;
 
+    mOnChannelConnectedTask->Cancel();
+
     if (mChannelErrorTask) {
         mChannelErrorTask->Cancel();
         mChannelErrorTask = nullptr;
     }
 
     // Free up any memory used by pending messages.
     mPending.clear();
     mPendingUrgentRequest = nullptr;
@@ -1422,29 +1430,29 @@ MessageChannel::SetReplyTimeoutMs(int32_
     mTimeoutMs = (aTimeoutMs <= 0)
                  ? kNoTimeout
                  : (int32_t)ceil((double)aTimeoutMs / 2.0);
 }
 
 void
 MessageChannel::OnChannelConnected(int32_t peer_id)
 {
-    mWorkerLoop->PostTask(
-        FROM_HERE,
-        NewRunnableMethod(this,
-                          &MessageChannel::DispatchOnChannelConnected,
-                          peer_id));
+    MOZ_ASSERT(!mPeerPidSet);
+    mPeerPidSet = true;
+    mPeerPid = peer_id;
+    mWorkerLoop->PostTask(FROM_HERE, new DequeueTask(mOnChannelConnectedTask));
 }
 
 void
-MessageChannel::DispatchOnChannelConnected(int32_t peer_pid)
+MessageChannel::DispatchOnChannelConnected()
 {
     AssertWorkerThread();
+    MOZ_ASSERT(mPeerPidSet);
     if (mListener)
-        mListener->OnChannelConnected(peer_pid);
+        mListener->OnChannelConnected(mPeerPid);
 }
 
 void
 MessageChannel::ReportMessageRouteError(const char* channelName) const
 {
     PrintErrorMessage(mSide, channelName, "Need a route");
     mListener->OnProcessingError(MsgRouteError);
 }
--- a/ipc/glue/MessageChannel.h
+++ b/ipc/glue/MessageChannel.h
@@ -6,16 +6,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef ipc_glue_MessageChannel_h
 #define ipc_glue_MessageChannel_h 1
 
 #include "base/basictypes.h"
 #include "base/message_loop.h"
 
+#include "mozilla/DebugOnly.h"
 #include "mozilla/Monitor.h"
 #include "mozilla/Vector.h"
 #include "mozilla/WeakPtr.h"
 #include "mozilla/ipc/Transport.h"
 #include "MessageLink.h"
 #include "nsAutoPtr.h"
 
 #include <deque>
@@ -197,17 +198,17 @@ class MessageChannel : HasResultCodes
     void OnNotifyMaybeChannelError();
     void ReportConnectionError(const char* aChannelName) const;
     void ReportMessageRouteError(const char* channelName) const;
     bool MaybeHandleError(Result code, const char* channelName);
 
     void Clear();
 
     // Send OnChannelConnected notification to listeners.
-    void DispatchOnChannelConnected(int32_t peer_pid);
+    void DispatchOnChannelConnected();
 
     // Any protocol that requires blocking until a reply arrives, will send its
     // outgoing message through this function. Currently, two protocols do this:
     //
     //  sync, which can only initiate messages from child to parent.
     //  urgent, which can only initiate messages from parent to child.
     //
     // SendAndWait() expects that the worker thread owns the monitor, and that
@@ -650,16 +651,23 @@ class MessageChannel : HasResultCodes
 #endif
 
     // Should the channel abort the process from the I/O thread when
     // a channel error occurs?
     bool mAbortOnError;
 
     // See SetChannelFlags
     ChannelFlags mFlags;
+
+    // Task and state used to asynchronously notify channel has been connected
+    // safely.  This is necessary to be able to cancel notification if we are
+    // closed at the same time.
+    nsRefPtr<RefCountedTask> mOnChannelConnectedTask;
+    DebugOnly<bool> mPeerPidSet;
+    int32_t mPeerPid;
 };
 
 bool
 ProcessingUrgentMessages();
 
 } // namespace ipc
 } // namespace mozilla
 
--- a/ipc/glue/MessageLink.cpp
+++ b/ipc/glue/MessageLink.cpp
@@ -335,26 +335,35 @@ ProcessLink::OnTakeConnectedChannel()
     }
 }
 
 void
 ProcessLink::OnChannelConnected(int32_t peer_pid)
 {
     AssertIOThread();
 
+    bool notifyChannel = false;
+
     {
         MonitorAutoLock lock(*mChan->mMonitor);
-        mChan->mChannelState = ChannelConnected;
-        mChan->mMonitor->Notify();
+        // Only update channel state if its still thinks its opening.  Do not
+        // force it into connected if it has errored out, started closing, etc.
+        if (mChan->mChannelState == ChannelOpening) {
+          mChan->mChannelState = ChannelConnected;
+          mChan->mMonitor->Notify();
+          notifyChannel = true;
+        }
     }
 
     if (mExistingListener)
         mExistingListener->OnChannelConnected(peer_pid);
 
-    mChan->OnChannelConnected(peer_pid);
+    if (notifyChannel) {
+      mChan->OnChannelConnected(peer_pid);
+    }
 }
 
 void
 ProcessLink::OnChannelError()
 {
     AssertIOThread();
     MonitorAutoLock lock(*mChan->mMonitor);
     mChan->OnChannelErrorFromLink();