Bug 1048011: fix a race condition that leaks PProcLoaderParent. r=khuey
authorCervantes Yu <cyu@mozilla.com>
Mon, 11 Aug 2014 15:50:20 +0800
changeset 231591 a39933b3beccd3c9680e8e929609164909690248
parent 231590 9f9bbf84c3e8174278336e322643a4fc697e45eb
child 231592 b0c87b1e4e6e2791b283f65533246c418964efd7
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1048011
milestone35.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 1048011: fix a race condition that leaks PProcLoaderParent. r=khuey
ipc/glue/ProcessUtils_linux.cpp
--- a/ipc/glue/ProcessUtils_linux.cpp
+++ b/ipc/glue/ProcessUtils_linux.cpp
@@ -112,70 +112,64 @@ static void ProcLoaderClientDeinit();
  * starting from kBeginReserveFileDescriptor so that operations like
  * __android_log_print() won't take these magic FDs.
  */
 static const int kReservedFileDescriptors = 5;
 static const int kBeginReserveFileDescriptor = STDERR_FILENO + 1;
 
 class ProcLoaderParent : public PProcLoaderParent
 {
-private:
-  nsAutoPtr<FileDescriptor> mChannelFd; // To keep a reference.
-
 public:
-  ProcLoaderParent(FileDescriptor *aFd) : mChannelFd(aFd) {}
+  ProcLoaderParent() {}
+  virtual ~ProcLoaderParent() {}
 
   virtual void ActorDestroy(ActorDestroyReason aWhy) MOZ_OVERRIDE;
 
   virtual bool RecvLoadComplete(const int32_t &aPid,
                                 const int32_t &aCookie) MOZ_OVERRIDE;
-
-  virtual void OnChannelError() MOZ_OVERRIDE;
 };
 
 void
 ProcLoaderParent::ActorDestroy(ActorDestroyReason aWhy)
 {
+  if (aWhy == AbnormalShutdown) {
+    NS_WARNING("ProcLoaderParent is destroyed abnormally.");
+  }
+
+  if (sProcLoaderClientOnDeinit) {
+    // Get error for closing while the channel is already error.
+    return;
+  }
+
+  // Destroy self asynchronously.
+  ProcLoaderClientDeinit();
 }
 
 static void
 _ProcLoaderParentDestroy(PProcLoaderParent *aLoader)
 {
-  aLoader->Close();
   delete aLoader;
   sProcLoaderClientOnDeinit = false;
 }
 
 bool
 ProcLoaderParent::RecvLoadComplete(const int32_t &aPid,
                                    const int32_t &aCookie)
 {
-  ProcLoaderClientDeinit();
   return true;
 }
 
 static void
 CloseFileDescriptors(FdArray& aFds)
 {
   for (size_t i = 0; i < aFds.length(); i++) {
     unused << HANDLE_EINTR(close(aFds[i]));
   }
 }
 
-void
-ProcLoaderParent::OnChannelError()
-{
-  if (sProcLoaderClientOnDeinit) {
-    // Get error for closing while the channel is already error.
-    return;
-  }
-  NS_WARNING("ProcLoaderParent is in channel error");
-  ProcLoaderClientDeinit();
-}
-
 /**
  * Initialize the client of B2G loader for loader itself.
  *
  * The initialization of B2G loader are divided into two stages. First
  * stage is to collect child info passed from the main program of the
  * loader.  Second stage is to initialize Gecko according to info from the
  * first stage and make the client of loader service ready.
  *
@@ -199,21 +193,21 @@ void
 ProcLoaderClientGeckoInit()
 {
   MOZ_ASSERT(sProcLoaderClientInitialized, "call ProcLoaderClientInit() at first");
   MOZ_ASSERT(!sProcLoaderClientGeckoInitialized,
              "call ProcLoaderClientGeckoInit() more than once");
 
   sProcLoaderClientGeckoInitialized = true;
 
-  FileDescriptor *fd = new FileDescriptor(sProcLoaderChannelFd);
-  close(sProcLoaderChannelFd);
+  TransportDescriptor fd;
+  fd.mFd = base::FileDescriptor(sProcLoaderChannelFd, /*auto_close=*/ false);
   sProcLoaderChannelFd = -1;
-  Transport *transport = OpenDescriptor(*fd, Transport::MODE_CLIENT);
-  sProcLoaderParent = new ProcLoaderParent(fd);
+  Transport *transport = OpenDescriptor(fd, Transport::MODE_CLIENT);
+  sProcLoaderParent = new ProcLoaderParent();
   sProcLoaderParent->Open(transport,
                           sProcLoaderPid,
                           XRE_GetIOMessageLoop(),
                           ParentSide);
   sProcLoaderLoop = MessageLoop::current();
 }
 
 /**
@@ -543,18 +537,18 @@ ProcLoaderServiceRun(pid_t aPeerPid, int
   gArgc = aArgc;
 
   {
     nsresult rv = XRE_InitCommandLine(aArgc, _argv);
     if (NS_FAILED(rv)) {
       MOZ_CRASH();
     }
 
-    FileDescriptor fd(aFd);
-    close(aFd);
+    TransportDescriptor fd;
+    fd.mFd = base::FileDescriptor(aFd, /*auto_close =*/false);
 
     MOZ_ASSERT(!sProcLoaderServing);
     MessageLoop loop;
 
     nsAutoPtr<ContentProcess> process;
     process = new ContentProcess(aPeerPid);
     ChildThread *iothread = process->child_thread();