Bug 1105468 - OpenFileAndSendFDRunnable::OpenFile needs to ensure we don't release off main thread. r=khuey
☠☠ backed out by 28545193d61d ☠ ☠
authorAndrew McCreight <continuation@gmail.com>
Wed, 26 Nov 2014 13:05:00 +0100
changeset 218976 ac697e1121348618643867a0a6196b7a6c6f2912
parent 218975 cadfee0aa2af0fa0c5d3e192423024d417b3183a
child 218977 6018754db69a0dee5c572cf47c019bd41fc59cbb
push id52702
push usercbook@mozilla.com
push dateWed, 10 Dec 2014 08:11:38 +0000
treeherdermozilla-inbound@ac697e112134 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1105468
milestone37.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 1105468 - OpenFileAndSendFDRunnable::OpenFile needs to ensure we don't release off main thread. r=khuey If any of the file operations fail, we need to ensure we still dispatch to the main thread. If the dispatch fails, leak the runnable (and the things it owns) rather than crash when we try to release them off main thread.
dom/ipc/TabParent.cpp
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -113,16 +113,17 @@ public:
 
         nsresult rv = mEventTarget->Dispatch(this, NS_DISPATCH_NORMAL);
         NS_ENSURE_SUCCESS_VOID(rv);
     }
 
 private:
     ~OpenFileAndSendFDRunnable()
     {
+        MOZ_ASSERT(NS_IsMainThread());
         MOZ_ASSERT(!mFD);
     }
 
     // This shouldn't be called directly except by the event loop. Use Dispatch
     // to start the sequence.
     NS_IMETHOD Run()
     {
         if (NS_IsMainThread()) {
@@ -165,34 +166,46 @@ private:
             NS_WARNING("Failed to dispatch to stream transport service!");
 
             // It's probably safer to take the main thread IO hit here rather
             // than leak a file descriptor.
             CloseFile();
         }
     }
 
-    void OpenFile()
+    // Helper method to avoid gnarly control flow for failures.
+    void OpenFileImpl()
     {
         MOZ_ASSERT(!NS_IsMainThread());
         MOZ_ASSERT(!mFD);
 
         nsCOMPtr<nsIFile> file;
         nsresult rv = NS_NewLocalFile(mPath, false, getter_AddRefs(file));
         NS_ENSURE_SUCCESS_VOID(rv);
 
         PRFileDesc* fd;
         rv = file->OpenNSPRFileDesc(PR_RDONLY, 0, &fd);
         NS_ENSURE_SUCCESS_VOID(rv);
 
         mFD = fd;
+    }
+
+    void OpenFile()
+    {
+        MOZ_ASSERT(!NS_IsMainThread());
+
+        OpenFileImpl();
 
         if (NS_FAILED(NS_DispatchToMainThread(this))) {
             NS_WARNING("Failed to dispatch to main thread!");
 
+            // Intentionally leak the runnable (but not the fd) rather
+            // than crash when trying to release a main thread object
+            // off the main thread.
+            NS_ADDREF(this);
             CloseFile();
         }
     }
 
     void CloseFile()
     {
         // It's possible for this to happen on the main thread if the dispatch
         // to the stream service fails after we've already opened the file so