Bug 836523 - Cleanup UnixSocketImpl from within I/O thread. r=qdot, r=echou
authorThomas Zimmermann <tdz@users.sourceforge.net>
Tue, 12 Feb 2013 09:16:45 -0500
changeset 131510 164c9a8f3711b395f380588cb8ffcd922e7da7a4
parent 131509 4251e6dd02180f830a61dab98eed7451d9cb4749
child 131511 06090cb1dc1f501c160acd8288516b346f24ccdd
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot, echou
bugs836523
milestone21.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 836523 - Cleanup UnixSocketImpl from within I/O thread. r=qdot, r=echou When closing a socket from within the main thread, we need to make sure that the I/O thread does not operate on the related instance of UnixSocketImpl. With this patch, the main thread posts a SocketCloseTask to the I/O thread. The SocketCloseTask removes the socket from the I/O thread's list of watched file descriptors and dispatches an instance of DeleteInstanceRunnable for the socket's UnixSocketImpl, which cleans up the data structures. These steps serialize the close operation within the I/O thread, and ensure that the main thread processed all other dispatched runnables that may use the UnixSocketImpl.
ipc/unixsocket/UnixSocket.cpp
--- a/ipc/unixsocket/UnixSocket.cpp
+++ b/ipc/unixsocket/UnixSocket.cpp
@@ -71,21 +71,16 @@ public:
 
   bool isFdValid()
   {
     return mFd > 0;
   }
 
   void CancelTask()
   {
-    if (!mTask) {
-      return;
-    }
-    mTask->Cancel();
-    mTask = nullptr;
     MutexAutoLock lock(mLock);
     mCurrentTaskIsCanceled = true;
   }
   
   bool IsCanceled()
   {
     MutexAutoLock lock(mLock);
     return mCurrentTaskIsCanceled;
@@ -139,16 +134,21 @@ public:
   void Listen();
 
   /** 
    * Accept an incoming connection
    */
   void Accept();
 
   /** 
+   * Close an open connection
+   */
+  void Close();
+
+  /**
    * Stop whatever connect/accept task is running
    */
   void StopTask()
   {
     if (mTask) {
       mTask->Cancel();
       mTask = nullptr;
     }
@@ -373,30 +373,31 @@ public:
   }
 
 private:
   nsRefPtr<UnixSocketConsumer> mConsumer;
   UnixSocketImpl* mImpl;
   UnixSocketRawData* mData;
 };
 
-class SocketCloseTask : public nsRunnable
+class SocketCloseTask : public Task
 {
 public:
   SocketCloseTask(UnixSocketImpl* aImpl)
     : mImpl(aImpl)
   {
     MOZ_ASSERT(aImpl);
   }
 
-  NS_IMETHOD
-  Run()
+  void Run()
   {
-    mImpl->mConsumer->CloseSocket();
-    return NS_OK;
+    NS_ENSURE_TRUE_VOID(mImpl);
+
+    mImpl->UnsetTask();
+    mImpl->Close();
   }
 
 private:
   UnixSocketImpl* mImpl;
 };
 
 class StartImplReadingTask : public Task
 {
@@ -447,16 +448,29 @@ void SocketConnectTask::Run() {
   mImpl->UnsetTask();
   if (mCanceled) {
     return;
   }
   mImpl->Connect();
 }
 
 void
+UnixSocketImpl::Close()
+{
+  mReadWatcher.StopWatchingFileDescriptor();
+  mWriteWatcher.StopWatchingFileDescriptor();
+
+  nsRefPtr<nsIRunnable> t(new DeleteInstanceRunnable<UnixSocketImpl>(this));
+  NS_ENSURE_TRUE_VOID(t);
+
+  nsresult rv = NS_DispatchToMainThread(t);
+  NS_ENSURE_SUCCESS_VOID(rv);
+}
+
+void
 UnixSocketImpl::Accept()
 {
 
   if (!mConnector) {
     NS_WARNING("No connector object available!");
     return;
   }
 
@@ -606,28 +620,19 @@ UnixSocketConsumer::CloseSocket()
   if (!mImpl) {
     return;
   }
   UnixSocketImpl* impl = mImpl;
   // To make sure the owner doesn't die on the IOThread, remove pointer here
   mImpl = nullptr;
   // Line it up to be destructed on the IO Thread
   impl->mConsumer.forget();
-  impl->StopTask();
 
-  // The receiver task should have been stopped at this point, but
-  // SocketReceiverTask runnables might still be pending the main
-  // thread. We enqueue the DeleteInstanceRunnable _after_ any pending
-  // SocketReceiverTask. Otherwise we might free 'impl' before those
-  // runnables have been executed.
-  nsRefPtr<nsIRunnable> t(new DeleteInstanceRunnable<UnixSocketImpl>(impl));
-  NS_ENSURE_TRUE_VOID(t);
-  nsresult rv = NS_DispatchToMainThread(t);
-  NS_ENSURE_SUCCESS_VOID(rv);
-  t.forget();
+  impl->CancelTask();
+  XRE_GetIOMessageLoop()->PostTask(FROM_HERE, new SocketCloseTask(impl));
 
   NotifyDisconnect();
 }
 
 void
 UnixSocketImpl::OnFileCanReadWithoutBlocking(int aFd)
 {
   enum SocketConnectionStatus status = mConsumer->GetConnectionStatus();
@@ -658,18 +663,17 @@ UnixSocketImpl::OnFileCanReadWithoutBloc
           }
 #ifdef DEBUG
           NS_WARNING("Cannot read from network");
 #endif
           // At this point, assume that we can't actually access
           // the socket anymore
           mReadWatcher.StopWatchingFileDescriptor();
           mWriteWatcher.StopWatchingFileDescriptor();
-          nsRefPtr<SocketCloseTask> t = new SocketCloseTask(this);
-          NS_DispatchToMainThread(t);
+          XRE_GetIOMessageLoop()->PostTask(FROM_HERE, new SocketCloseTask(this));
           return;
         }
         if (ret) {
           mIncoming = new UnixSocketRawData(ret);
           memcpy(mIncoming->mData, data, ret);
           nsRefPtr<SocketReceiveTask> t =
             new SocketReceiveTask(this, mIncoming.forget());
           NS_DispatchToMainThread(t);
@@ -833,13 +837,13 @@ UnixSocketConsumer::ListenSocket(UnixSoc
 void
 UnixSocketConsumer::CancelSocketTask()
 {
   mConnectionStatus = SOCKET_DISCONNECTED;
   if(!mImpl) {
     NS_WARNING("No socket implementation to cancel task on!");
     return;
   }
-  mImpl->CancelTask();
+  mImpl->StopTask();
 }
 
 } // namespace ipc
 } // namespace mozilla