Bug 1264566 - Part 2: Refactor all usage of FileDescriptor. r?valentin draft
authorWei-Cheng Pan <wpan@mozilla.com>
Fri, 27 May 2016 16:12:51 +0800
changeset 387516 26ca0d28089b54975aa13d71cd21a15fef45830e
parent 387515 8aca966c40695a08d14af38cf2d08b36e59e4813
child 387517 1d6336993f5c81f6fd75fdc06cb82dcdafbb7801
push id22974
push userbmo:wpan@mozilla.com
push dateThu, 14 Jul 2016 06:19:47 +0000
reviewersvalentin
bugs1264566
milestone50.0a1
Bug 1264566 - Part 2: Refactor all usage of FileDescriptor. r?valentin Callers should use a UniquePtr to hold the platform handle. MozReview-Commit-ID: 6BWnyAf4b3a
devtools/shared/heapsnapshot/FileDescriptorOutputStream.cpp
dom/asmjscache/AsmJSCache.cpp
dom/camera/GonkCameraControl.cpp
dom/ipc/ContentChild.cpp
dom/ipc/ContentParent.cpp
dom/plugins/ipc/PluginModuleParent.cpp
ipc/glue/FileDescriptorUtils.cpp
ipc/glue/FileDescriptorUtils.h
ipc/glue/Transport_posix.cpp
netwerk/base/nsFileStreams.cpp
netwerk/ipc/RemoteOpenFileChild.cpp
security/sandbox/linux/gtest/TestBroker.cpp
xpcom/io/nsAnonymousTemporaryFile.cpp
--- a/devtools/shared/heapsnapshot/FileDescriptorOutputStream.cpp
+++ b/devtools/shared/heapsnapshot/FileDescriptorOutputStream.cpp
@@ -10,17 +10,18 @@ namespace mozilla {
 namespace devtools {
 
 /* static */ already_AddRefed<FileDescriptorOutputStream>
 FileDescriptorOutputStream::Create(const ipc::FileDescriptor& fileDescriptor)
 {
   if (NS_WARN_IF(!fileDescriptor.IsValid()))
     return nullptr;
 
-  PRFileDesc* prfd = PR_ImportFile(PROsfd(fileDescriptor.PlatformHandle()));
+  auto rawFD = fileDescriptor.ClonePlatformHandle();
+  PRFileDesc* prfd = PR_ImportFile(PROsfd(rawFD.release()));
   if (NS_WARN_IF(!prfd))
     return nullptr;
 
   RefPtr<FileDescriptorOutputStream> stream = new FileDescriptorOutputStream(prfd);
   return stream.forget();
 }
 
 NS_IMPL_ISUPPORTS(FileDescriptorOutputStream, nsIOutputStream);
--- a/dom/asmjscache/AsmJSCache.cpp
+++ b/dom/asmjscache/AsmJSCache.cpp
@@ -1305,17 +1305,18 @@ private:
   RecvOnOpenCacheFile(const int64_t& aFileSize,
                       const FileDescriptor& aFileDesc) override
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mState == eOpening);
 
     mFileSize = aFileSize;
 
-    mFileDesc = PR_ImportFile(PROsfd(aFileDesc.PlatformHandle()));
+    auto rawFD = aFileDesc.ClonePlatformHandle();
+    mFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
     if (!mFileDesc) {
       return false;
     }
 
     mState = eOpened;
     Notify(JS::AsmJSCache_Success);
     return true;
   }
--- a/dom/camera/GonkCameraControl.cpp
+++ b/dom/camera/GonkCameraControl.cpp
@@ -1247,25 +1247,25 @@ nsGonkCameraControl::StartRecordingImpl(
   // close the file descriptor when we leave this function. Also note, that
   // since we're already off the main thread, we don't need to dispatch this.
   // We just let the CloseFileRunnable destructor do the work.
   RefPtr<CloseFileRunnable> closer;
   if (aFileDescriptor->mFileDescriptor.IsValid()) {
     closer = new CloseFileRunnable(aFileDescriptor->mFileDescriptor);
   }
   nsresult rv;
-  int fd = aFileDescriptor->mFileDescriptor.PlatformHandle();
+  auto rawFD = aFileDescriptor->mFileDescriptor.ClonePlatformHandle();
   if (aOptions) {
-    rv = SetupRecording(fd, aOptions->rotation, aOptions->maxFileSizeBytes,
+    rv = SetupRecording(rawFD.get(), aOptions->rotation, aOptions->maxFileSizeBytes,
                         aOptions->maxVideoLengthMs);
     if (NS_SUCCEEDED(rv)) {
       rv = SetupRecordingFlash(aOptions->autoEnableLowLightTorch);
     }
   } else {
-    rv = SetupRecording(fd, 0, 0, 0);
+    rv = SetupRecording(rawFD.get(), 0, 0, 0);
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
 #ifdef MOZ_WIDGET_GONK
   if (mRecorder->start() != OK) {
     DOM_CAMERA_LOGE("mRecorder->start() failed\n");
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1435,17 +1435,18 @@ ContentChild::RecvSetProcessSandbox(cons
   // before seccomp is enabled (Bug 1259508). It also increases the startup
   // time of the content process, because cubeb is usually initialized
   // when it is actually needed. This call here is no longer required
   // once Bug 1104619 (remoting audio) is resolved.
   Unused << CubebUtils::GetCubebContext();
 #endif
   int brokerFd = -1;
   if (aBroker.type() == MaybeFileDesc::TFileDescriptor) {
-      brokerFd = aBroker.get_FileDescriptor().PlatformHandle();
+      auto fd = aBroker.get_FileDescriptor().ClonePlatformHandle();
+      brokerFd = fd.release();
       // brokerFd < 0 means to allow direct filesystem access, so
       // make absolutely sure that doesn't happen if the parent
       // didn't intend it.
       MOZ_RELEASE_ASSERT(brokerFd >= 0);
   }
   SetContentProcessSandbox(brokerFd);
 #elif defined(XP_WIN)
   mozilla::SandboxTarget::Instance()->StartSandbox();
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -4915,19 +4915,19 @@ ContentParent::RecvRemoveIdleObserver(co
 bool
 ContentParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 {
 #ifndef MOZ_X11
   NS_RUNTIMEABORT("This message only makes sense on X11 platforms");
 #else
   MOZ_ASSERT(0 > mChildXSocketFdDup.get(),
              "Already backed up X resources??");
-  mChildXSocketFdDup.forget();
   if (aXSocketFd.IsValid()) {
-    mChildXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+    auto rawFD = aXSocketFd.ClonePlatformHandle();
+    mChildXSocketFdDup.reset(rawFD.release());
   }
 #endif
   return true;
 }
 
 bool
 ContentParent::RecvOpenAnonymousTemporaryFile(FileDescOrError *aFD)
 {
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -1913,19 +1913,19 @@ PluginModuleParent::NPP_SetValue(NPP ins
 bool
 PluginModuleParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 {
 #ifndef MOZ_X11
     NS_RUNTIMEABORT("This message only makes sense on X11 platforms");
 #else
     MOZ_ASSERT(0 > mPluginXSocketFdDup.get(),
                "Already backed up X resources??");
-    mPluginXSocketFdDup.forget();
     if (aXSocketFd.IsValid()) {
-      mPluginXSocketFdDup.reset(aXSocketFd.PlatformHandle());
+      auto rawFD = aXSocketFd.ClonePlatformHandle();
+      mPluginXSocketFdDup.reset(rawFD.release());
     }
 #endif
     return true;
 }
 
 void
 PluginModuleParent::NPP_URLRedirectNotify(NPP instance, const char* url,
                                           int32_t status, void* notifyData)
--- a/ipc/glue/FileDescriptorUtils.cpp
+++ b/ipc/glue/FileDescriptorUtils.cpp
@@ -57,29 +57,17 @@ CloseFileRunnable::Dispatch()
   NS_ENSURE_SUCCESS_VOID(rv);
 }
 
 void
 CloseFileRunnable::CloseFile()
 {
   // It's possible for this to happen on the main thread if the dispatch to the
   // stream service fails so we can't assert the thread on which we're running.
-
-  MOZ_ASSERT(mFileDescriptor.IsValid());
-
-  PRFileDesc* fd =
-    PR_ImportFile(PROsfd(mFileDescriptor.PlatformHandle()));
-  NS_WARN_IF_FALSE(fd, "Failed to import file handle!");
-
   mFileDescriptor = FileDescriptor();
-
-  if (fd) {
-    PR_Close(fd);
-    fd = nullptr;
-  }
 }
 
 NS_IMETHODIMP
 CloseFileRunnable::Run()
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   CloseFile();
@@ -88,31 +76,29 @@ CloseFileRunnable::Run()
 
 namespace mozilla {
 namespace ipc {
 
 FILE*
 FileDescriptorToFILE(const FileDescriptor& aDesc,
                      const char* aOpenMode)
 {
-  // Debug builds check whether the handle was "used", even if it's
-  // invalid, so that needs to happen first.
-  FileDescriptor::PlatformHandleType handle = aDesc.PlatformHandle();
   if (!aDesc.IsValid()) {
     errno = EBADF;
     return nullptr;
   }
+  auto handle = aDesc.ClonePlatformHandle();
 #ifdef XP_WIN
-  int fd = _open_osfhandle(reinterpret_cast<intptr_t>(handle), 0);
+  int fd = _open_osfhandle(static_cast<intptr_t>(handle.get()), 0);
   if (fd == -1) {
-    CloseHandle(handle);
     return nullptr;
   }
+  Unused << handle.release();
 #else
-  int fd = handle;
+  int fd = handle.release();
 #endif
   FILE* file = fdopen(fd, aOpenMode);
   if (!file) {
     int saved_errno = errno;
     close(fd);
     errno = saved_errno;
   }
   return file;
--- a/ipc/glue/FileDescriptorUtils.h
+++ b/ipc/glue/FileDescriptorUtils.h
@@ -39,18 +39,18 @@ public:
   void Dispatch();
 
 private:
   ~CloseFileRunnable();
 
   void CloseFile();
 };
 
-// On failure, FileDescriptorToFILE closes the given descriptor; on
-// success, fclose()ing the returned FILE* will close the handle.
+// On failure, FileDescriptorToFILE returns nullptr; on success,
+// returns duplicated FILE*.
 // This is meant for use with FileDescriptors received over IPC.
 FILE* FileDescriptorToFILE(const FileDescriptor& aDesc,
                            const char* aOpenMode);
 
 // FILEToFileDescriptor does not consume the given FILE*; it must be
 // fclose()d as normal, and this does not invalidate the returned
 // FileDescriptor.
 FileDescriptor FILEToFileDescriptor(FILE* aStream);
--- a/ipc/glue/Transport_posix.cpp
+++ b/ipc/glue/Transport_posix.cpp
@@ -55,17 +55,18 @@ UniquePtr<Transport>
 OpenDescriptor(const TransportDescriptor& aTd, Transport::Mode aMode)
 {
   return MakeUnique<Transport>(aTd.mFd.fd, aMode, nullptr);
 }
 
 UniquePtr<Transport>
 OpenDescriptor(const FileDescriptor& aFd, Transport::Mode aMode)
 {
-  return MakeUnique<Transport>(aFd.PlatformHandle(), aMode, nullptr);
+  auto rawFD = aFd.ClonePlatformHandle();
+  return MakeUnique<Transport>(rawFD.release(), aMode, nullptr);
 }
 
 TransportDescriptor
 DuplicateDescriptor(const TransportDescriptor& aTd)
 {
   TransportDescriptor result = aTd;
   result.mFd.fd = dup(aTd.mFd.fd);
   if (result.mFd.fd == -1) {
--- a/netwerk/base/nsFileStreams.cpp
+++ b/netwerk/base/nsFileStreams.cpp
@@ -630,17 +630,18 @@ nsFileInputStream::Deserialize(const Inp
     if (fileDescriptorIndex < aFileDescriptors.Length()) {
         fd = aFileDescriptors[fileDescriptorIndex];
         NS_WARN_IF_FALSE(fd.IsValid(), "Received an invalid file descriptor!");
     } else {
         NS_WARNING("Received a bad file descriptor index!");
     }
 
     if (fd.IsValid()) {
-        PRFileDesc* fileDesc = PR_ImportFile(PROsfd(fd.PlatformHandle()));
+        auto rawFD = fd.ClonePlatformHandle();
+        PRFileDesc* fileDesc = PR_ImportFile(PROsfd(rawFD.release()));
         if (!fileDesc) {
             NS_WARNING("Failed to import file handle!");
             return false;
         }
         mFD = fileDesc;
     }
 
     mBehaviorFlags = params.behaviorFlags();
--- a/netwerk/ipc/RemoteOpenFileChild.cpp
+++ b/netwerk/ipc/RemoteOpenFileChild.cpp
@@ -289,17 +289,18 @@ RemoteOpenFileChild::HandleFileDescripto
     if (NS_FAILED(mFile->GetPath(path))) {
       MOZ_CRASH("Couldn't get path from file!");
     }
 
     tabChild->CancelCachedFileDescriptorCallback(path, this);
   }
 
   if (aFD.IsValid()) {
-    mNSPRFileDesc = PR_ImportFile(aFD.PlatformHandle());
+    auto rawFD = aFD.ClonePlatformHandle();
+    mNSPRFileDesc = PR_ImportFile(rawFD.release());
     if (!mNSPRFileDesc) {
       NS_WARNING("Failed to import file handle!");
     }
   }
 
   NotifyListener(mNSPRFileDesc ? NS_OK : NS_ERROR_FILE_NOT_FOUND);
 #endif
 }
--- a/security/sandbox/linux/gtest/TestBroker.cpp
+++ b/security/sandbox/linux/gtest/TestBroker.cpp
@@ -62,17 +62,18 @@ protected:
   }
 
   virtual void SetUp() {
     ipc::FileDescriptor fd;
 
     mServer = SandboxBroker::Create(GetPolicy(), getpid(), fd);
     ASSERT_NE(mServer, nullptr);
     ASSERT_TRUE(fd.IsValid());
-    mClient.reset(new SandboxBrokerClient(dup(fd.PlatformHandle())));
+    auto rawFD = fd.ClonePlatformHandle();
+    mClient.reset(new SandboxBrokerClient(rawFD.release()));
   }
 
   template<class C, void (C::* Main)()>
   void StartThread(pthread_t *aThread) {
     ASSERT_EQ(0, pthread_create(aThread, nullptr, ThreadMain<C, Main>,
                                 static_cast<C*>(this)));
   }
 
--- a/xpcom/io/nsAnonymousTemporaryFile.cpp
+++ b/xpcom/io/nsAnonymousTemporaryFile.cpp
@@ -121,18 +121,18 @@ NS_OpenAnonymousTemporaryFile(PRFileDesc
       SyncRunnable::DispatchToThread(mainThread,
         new nsRemoteAnonymousTemporaryFileRunnable(&fd));
     }
     if (fd.type() == dom::FileDescOrError::Tnsresult) {
       nsresult rv = fd.get_nsresult();
       MOZ_ASSERT(NS_FAILED(rv));
       return rv;
     }
-    *aOutFileDesc =
-      PR_ImportFile(PROsfd(fd.get_FileDescriptor().PlatformHandle()));
+    auto rawFD = fd.get_FileDescriptor().ClonePlatformHandle();
+    *aOutFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
     return NS_OK;
   }
 
   nsresult rv;
   nsCOMPtr<nsIFile> tmpFile;
   rv = GetTempDir(getter_AddRefs(tmpFile));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;