Bug 1264566 - Part 2: Refactor all usage of FileDescriptor. r=valentin
authorWei-Cheng Pan <wpan@mozilla.com>
Fri, 27 May 2016 16:12:51 +0800
changeset 306398 08d0e33b4cb427bd5fa9b713b7d0289ad3026ad0
parent 306397 95d12e109117ac979c2ef2fea6f518f912c8b899
child 306399 0476118cfea7aef2cb676cc9a78b6ce109cab270
push id30484
push usercbook@mozilla.com
push dateMon, 25 Jul 2016 13:51:04 +0000
treeherdermozilla-central@e23f2ec25e96 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1264566
milestone50.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 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
@@ -1417,17 +1417,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
@@ -4917,19 +4917,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
@@ -1909,19 +1909,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;