Bug 739452, part 2: Ensure we don't process stale 'reconnect' tasks after shutting down. r=kmachulis
authorChris Jones <jones.chris.g@gmail.com>
Sun, 01 Apr 2012 01:57:21 -0700
changeset 94187 02b9017cc6c0e64fa43a37e00044ec2f12cf0f0b
parent 94186 a9cc7ebe75985219945aeb73515e4adaea0893dc
child 94188 6314b9c23fd755d691cab740ebe3b53db24a7705
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmachulis
bugs739452
milestone14.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 739452, part 2: Ensure we don't process stale 'reconnect' tasks after shutting down. r=kmachulis
ipc/ril/Ril.cpp
--- a/ipc/ril/Ril.cpp
+++ b/ipc/ril/Ril.cpp
@@ -53,16 +53,17 @@
 #include "mozilla/FileUtils.h"
 #include "mozilla/Monitor.h"
 #include "mozilla/Util.h"
 #include "nsAutoPtr.h"
 #include "nsIThread.h"
 #include "nsXULAppAPI.h"
 #include "Ril.h"
 
+#undef LOG
 #if defined(MOZ_WIDGET_GONK)
 #include <android/log.h>
 #define LOG(args...)  __android_log_print(ANDROID_LOG_INFO, "Gonk", args)
 #else
 #define LOG(args...)  printf(args);
 #endif
 
 #define RIL_SOCKET_NAME "/dev/socket/rilproxy"
@@ -81,18 +82,18 @@ struct RilClient : public RefCounted<Ril
                    public MessageLoopForIO::Watcher
 
 {
     typedef queue<RilRawData*> RilRawDataQueue;
 
     RilClient() : mSocket(-1)
                 , mMutex("RilClient.mMutex")
                 , mBlockedOnWrite(false)
+                , mIOLoop(MessageLoopForIO::current())
                 , mCurrentRilRawData(NULL)
-                , mIOLoop(MessageLoopForIO::current())
     { }
     virtual ~RilClient() { }
 
     bool OpenSocket();
 
     virtual void OnFileCanReadWithoutBlocking(int fd);
     virtual void OnFileCanWriteWithoutBlocking(int fd);
 
@@ -110,42 +111,84 @@ struct RilClient : public RefCounted<Ril
 
 static RefPtr<RilClient> sClient;
 static RefPtr<RilConsumer> sConsumer;
 
 //-----------------------------------------------------------------------------
 // This code runs on the IO thread.
 //
 
-class RilReconnectTask : public Task {
+class RilReconnectTask : public CancelableTask {
+    RilReconnectTask() : mCanceled(false) { }
+
     virtual void Run();
+    virtual void Cancel() { mCanceled = true; }
+
+    bool mCanceled;
+
+public:
+    static void Enqueue(int aDelayMs = 0) {
+        MessageLoopForIO* ioLoop = MessageLoopForIO::current();
+        MOZ_ASSERT(ioLoop && sClient->mIOLoop == ioLoop);
+        if (sTask) {
+            return;
+        }
+        sTask = new RilReconnectTask();
+        if (aDelayMs) {
+            ioLoop->PostDelayedTask(FROM_HERE, sTask, aDelayMs);
+        } else {
+            ioLoop->PostTask(FROM_HERE, sTask);
+        }
+    }
+
+    static void CancelIt() {
+        if (!sTask) {
+            return;
+        }
+        sTask->Cancel();
+        sTask = nsnull;
+    }
+
+private:
+    // Can *ONLY* be touched by the IO thread.  The event queue owns
+    // this memory when pointer is nonnull; do *NOT* free it manually.
+    static CancelableTask* sTask;
 };
+CancelableTask* RilReconnectTask::sTask;
 
 void RilReconnectTask::Run() {
+    // NB: the order of these two statements is important!  sTask must
+    // always run, whether we've been canceled or not, to avoid
+    // leading a dangling pointer in sTask.
+    sTask = nsnull;
+    if (mCanceled) {
+        return;
+    }
+
     if (sClient->OpenSocket()) {
         return;
     }
-    sClient->mIOLoop->PostDelayedTask(FROM_HERE, new RilReconnectTask(), 1000);
+    Enqueue(1000);
 }
 
 class RilWriteTask : public Task {
     virtual void Run();
 };
 
 void RilWriteTask::Run() {
     sClient->OnFileCanWriteWithoutBlocking(sClient->mSocket.mFd);
 }
 
 static void
 ConnectToRil(Monitor* aMonitor, bool* aSuccess)
 {
     MOZ_ASSERT(!sClient);
 
     sClient = new RilClient();
-    sClient->mIOLoop->PostTask(FROM_HERE, new RilReconnectTask());
+    RilReconnectTask::Enqueue();
     *aSuccess = true;
     {
         MonitorAutoLock lock(*aMonitor);
         lock.Notify();
     }
     // aMonitor may have gone out of scope by now, don't touch it
 }
 
@@ -228,31 +271,31 @@ RilClient::OnFileCanReadWithoutBlocking(
     //   - mIncoming isn't completely read, but there's no more
     //     data available on the socket
     //     If so, break;
 
     MOZ_ASSERT(fd == mSocket.mFd);
     while (true) {
         if (!mIncoming) {
             mIncoming = new RilRawData();
-            int ret = read(fd, mIncoming->mData, RilRawData::MAX_DATA_SIZE);
+            ssize_t ret = read(fd, mIncoming->mData, RilRawData::MAX_DATA_SIZE);
             if (ret <= 0) {
                 LOG("Cannot read from network, error %d\n", ret);
                 // At this point, assume that we can't actually access
                 // the socket anymore, and start a reconnect loop.
                 mIncoming.forget();
                 mReadWatcher.StopWatchingFileDescriptor();
                 mWriteWatcher.StopWatchingFileDescriptor();
                 close(mSocket.mFd);
-                mIOLoop->PostTask(FROM_HERE, new RilReconnectTask());
+                RilReconnectTask::Enqueue();
                 return;
             }
             mIncoming->mSize = ret;
             sConsumer->MessageReceived(mIncoming.forget());
-            if (ret < RilRawData::MAX_DATA_SIZE) {
+            if (ret < ssize_t(RilRawData::MAX_DATA_SIZE)) {
                 return;
             }
         }
     }
 }
 
 void
 RilClient::OnFileCanWriteWithoutBlocking(int fd)
@@ -301,16 +344,19 @@ RilClient::OnFileCanWriteWithoutBlocking
         mCurrentRilRawData = NULL;
     }
 }
 
 
 static void
 DisconnectFromRil(Monitor* aMonitor)
 {
+    // Prevent stale reconnect tasks from being run after we've shut
+    // down.
+    RilReconnectTask::CancelIt();
     // XXX This might "strand" messages in the outgoing queue.  We'll
     // assume that's OK for now.
     sClient = nsnull;
     {
         MonitorAutoLock lock(*aMonitor);
         lock.Notify();
     }
 }