Bug 552294: *Channel::OnError must run atomically. r=bent
authorChris Jones <jones.chris.g@gmail.com>
Thu, 18 Mar 2010 17:52:28 -0500
changeset 39611 b93d6faaa64cdc1fe4394e2bd18e29b5cd3cfe90
parent 39610 628ed02b87e63f20b51d498b42028d506ab45938
child 39612 6f14a461968de93c69ca2fc109e6e1107225aeb4
push id12296
push usercjones@mozilla.com
push dateThu, 18 Mar 2010 22:52:59 +0000
treeherdermozilla-central@bda8efa53512 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs552294
milestone1.9.3a4pre
Bug 552294: *Channel::OnError must run atomically. r=bent
ipc/glue/AsyncChannel.cpp
ipc/glue/AsyncChannel.h
ipc/glue/RPCChannel.cpp
ipc/glue/SyncChannel.cpp
--- a/ipc/glue/AsyncChannel.cpp
+++ b/ipc/glue/AsyncChannel.cpp
@@ -439,21 +439,28 @@ AsyncChannel::OnChannelConnected(int32 p
 
 void
 AsyncChannel::OnChannelError()
 {
     AssertIOThread();
 
     MutexAutoLock lock(mMutex);
 
-    // NB: this can race with the `Goodbye' event being processed by
-    // the worker thread
     if (ChannelClosing != mChannelState)
         mChannelState = ChannelError;
 
+    PostErrorNotifyTask();
+}
+
+void
+AsyncChannel::PostErrorNotifyTask()
+{
+    AssertIOThread();
+    mMutex.AssertCurrentThreadOwns();
+
     NS_ASSERTION(!mChannelErrorTask, "OnChannelError called twice?");
 
     // This must be the last code that runs on this thread!
     mChannelErrorTask =
         NewRunnableMethod(this, &AsyncChannel::OnNotifyMaybeChannelError);
     mWorkerLoop->PostTask(FROM_HERE, mChannelErrorTask);
 }
 
--- a/ipc/glue/AsyncChannel.h
+++ b/ipc/glue/AsyncChannel.h
@@ -170,16 +170,17 @@ protected:
 
     virtual void Clear();
 
     // Run on the IO thread
 
     void OnChannelOpened();
     void OnSend(Message* aMsg);
     void OnCloseChannel();
+    void PostErrorNotifyTask();
 
     // Return true if |msg| is a special message targeted at the IO
     // thread, in which case it shouldn't be delivered to the worker.
     bool MaybeInterceptSpecialIOMessage(const Message& msg);
     void ProcessGoodbyeMessage();
 
     Transport* mTransport;
     AsyncListener* mListener;
--- a/ipc/glue/RPCChannel.cpp
+++ b/ipc/glue/RPCChannel.cpp
@@ -649,27 +649,23 @@ RPCChannel::OnMessageReceived(const Mess
 }
 
 
 void
 RPCChannel::OnChannelError()
 {
     AssertIOThread();
 
-    {
-        MutexAutoLock lock(mMutex);
+    MutexAutoLock lock(mMutex);
+
+    if (ChannelClosing != mChannelState)
+        mChannelState = ChannelError;
 
-        // NB: this can race with the `Goodbye' event being processed by
-        // the worker thread
-        if (ChannelClosing != mChannelState)
-            mChannelState = ChannelError;
+    // skip SyncChannel::OnError(); we subsume its duties
+    if (AwaitingSyncReply() || 0 < StackDepth())
+        NotifyWorkerThread();
 
-        // skip SyncChannel::OnError(); we subsume its duties
-        if (AwaitingSyncReply() || 0 < StackDepth())
-            NotifyWorkerThread();
-    }
-
-    AsyncChannel::OnChannelError();
+    PostErrorNotifyTask();
 }
 
 } // namespace ipc
 } // namespace mozilla
 
--- a/ipc/glue/SyncChannel.cpp
+++ b/ipc/glue/SyncChannel.cpp
@@ -208,29 +208,25 @@ SyncChannel::OnMessageReceived(const Mes
     }
 }
 
 void
 SyncChannel::OnChannelError()
 {
     AssertIOThread();
 
-    {
-        MutexAutoLock lock(mMutex);
+    MutexAutoLock lock(mMutex);
+
+    if (ChannelClosing != mChannelState)
+        mChannelState = ChannelError;
 
-        // NB: this can race with the `Goodbye' event being processed by
-        // the worker thread
-        if (ChannelClosing != mChannelState)
-            mChannelState = ChannelError;
+    if (AwaitingSyncReply())
+        NotifyWorkerThread();
 
-        if (AwaitingSyncReply())
-            NotifyWorkerThread();
-    }
-
-    AsyncChannel::OnChannelError();
+    PostErrorNotifyTask();
 }
 
 //
 // Synchronization between worker and IO threads
 //
 
 namespace {