Bug 827749 - Aggressively duplicate file handles. r=cjones.

ipc/glue/FileDescriptor.cpp

@@ -8,58 +8,111 @@
 #include "nsDebug.h"
 
 #ifdef XP_WIN
+
 #include <windows.h>
 #define INVALID_HANDLE INVALID_HANDLE_VALUE
-#else
+
+#else // XP_WIN
+
 #include <unistd.h>
-#define INVALID_HANDLE -1
+
+#ifndef OS_POSIX
+#define OS_POSIX
 #endif
 
+#include "base/eintr_wrapper.h"
+#define INVALID_HANDLE -1
+
+#endif // XP_WIN
+
 using mozilla::ipc::FileDescriptor;
 
 FileDescriptor::FileDescriptor()
-: mHandle(INVALID_HANDLE)
+: mHandle(INVALID_HANDLE), mHandleCreatedByOtherProcess(false),
+  mHandleCreatedByOtherProcessWasUsed(false)
 { }
 
-FileDescriptor::PickleType
-FileDescriptor::ShareTo(const FileDescriptor::IPDLPrivate&,
-                        FileDescriptor::ProcessHandle aOtherProcess) const
+FileDescriptor::FileDescriptor(PlatformHandleType aHandle)
+: mHandle(INVALID_HANDLE), mHandleCreatedByOtherProcess(false),
+  mHandleCreatedByOtherProcessWasUsed(false)
+{
+  DuplicateInCurrentProcess(aHandle);
+}
+
+void
+FileDescriptor::DuplicateInCurrentProcess(PlatformHandleType aHandle)
 {
+  MOZ_ASSERT(!mHandleCreatedByOtherProcess);
+
+  if (IsValid(aHandle)) {
+    PlatformHandleType newHandle;
 #ifdef XP_WIN
-  if (mHandle == INVALID_HANDLE) {
-    return INVALID_HANDLE;
+    if (DuplicateHandle(GetCurrentProcess(), aHandle, GetCurrentProcess(),
+                        &newHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+#else // XP_WIN
+    if ((newHandle = dup(aHandle)) != INVALID_HANDLE) {
+#endif
+      mHandle = newHandle;
+      return;
+    }
+    NS_WARNING("Failed to duplicate file descriptor!");
   }
 
-  PlatformHandleType newHandle;
-  if (!DuplicateHandle(GetCurrentProcess(), mHandle, aOtherProcess, &newHandle,
-                       0, FALSE, DUPLICATE_SAME_ACCESS)) {
-    NS_WARNING("Failed to duplicate file handle!");
-    return INVALID_HANDLE;
+  mHandle = INVALID_HANDLE;
+}
+
+void
+FileDescriptor::CloseCurrentProcessHandle()
+{
+  MOZ_ASSERT_IF(mHandleCreatedByOtherProcess,
+                mHandleCreatedByOtherProcessWasUsed);
+
+  // Don't actually close handles created by another process.
+  if (mHandleCreatedByOtherProcess) {
+    return;
   }
 
-  return newHandle;
+  if (IsValid()) {
+#ifdef XP_WIN
+    if (!CloseHandle(mHandle)) {
+      NS_WARNING("Failed to close file handle!");
+    }
 #else // XP_WIN
-  if (mHandle == INVALID_HANDLE) {
-    return base::FileDescriptor();
+    HANDLE_EINTR(close(mHandle));
+#endif
+    mHandle = INVALID_HANDLE;
   }
+}
 
-  PlatformHandleType newHandle = dup(mHandle);
-  if (newHandle < 0) {
-    NS_WARNING("Failed to duplicate file descriptor!");
-    return base::FileDescriptor();
+FileDescriptor::PickleType
+FileDescriptor::ShareTo(const FileDescriptor::IPDLPrivate&,
+                        FileDescriptor::ProcessHandle aOtherProcess) const
+{
+  PlatformHandleType newHandle;
+#ifdef XP_WIN
+  if (IsValid()) {
+    if (DuplicateHandle(GetCurrentProcess(), mHandle, aOtherProcess,
+                        &newHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+      return newHandle;
+    }
+    NS_WARNING("Failed to duplicate file handle!");
   }
-
-  // This file descriptor must be closed once the caller is done using it, so
-  // pass true here for the 'auto_close' argument.
-  return base::FileDescriptor(newHandle, true);
+  return INVALID_HANDLE;
+#else // XP_WIN
+  if (IsValid()) {
+    newHandle = dup(mHandle);
+    return base::FileDescriptor(newHandle, /* auto_close */ true);
+  }
+  return base::FileDescriptor();
 #endif
 
   MOZ_NOT_REACHED("Must not get here!");
   return PickleType();
 }
 
+// static
 bool
-FileDescriptor::IsValid() const
+FileDescriptor::IsValid(PlatformHandleType aHandle)
 {
-  return mHandle != INVALID_HANDLE;
+  return aHandle != INVALID_HANDLE;
 }

ipc/glue/FileDescriptor.h

@@ -7,6 +7,7 @@
 
 #include "base/basictypes.h"
 #include "base/process.h"
+#include "mozilla/DebugOnly.h"
 #include "nscore.h"
 
 #ifdef XP_WIN
@@ -50,18 +51,47 @@ class FileDescriptor
 
   FileDescriptor();
 
-  FileDescriptor(PlatformHandleType aHandle)
-  : mHandle(aHandle)
-  { }
+  FileDescriptor(const FileDescriptor& aOther)
+  {
+    *this = aOther;
+  }
+
+  FileDescriptor(PlatformHandleType aHandle);
 
   FileDescriptor(const IPDLPrivate&, const PickleType& aPickle)
 #ifdef XP_WIN
   : mHandle(aPickle)
 #else
   : mHandle(aPickle.fd)
 #endif
+  , mHandleCreatedByOtherProcess(true)
+  , mHandleCreatedByOtherProcessWasUsed(false)
   { }
 
+  ~FileDescriptor()
+  {
+    CloseCurrentProcessHandle();
+  }
+
+  FileDescriptor&
+  operator=(const FileDescriptor& aOther)
+  {
+    CloseCurrentProcessHandle();
+
+    if (aOther.mHandleCreatedByOtherProcess) {
+      mHandleCreatedByOtherProcess = true;
+      mHandleCreatedByOtherProcessWasUsed =
+        aOther.mHandleCreatedByOtherProcessWasUsed;
+      mHandle = aOther.PlatformHandle();
+    } else {
+      DuplicateInCurrentProcess(aOther.PlatformHandle());
+      mHandleCreatedByOtherProcess = false;
+      mHandleCreatedByOtherProcessWasUsed = false;
+    }
+
+    return *this;
+  }
+
   // Performs platform-specific actions to duplicate mHandle in the other
   // process (e.g. dup() on POSIX, DuplicateHandle() on Windows). Returns a
   // pickled value that can be passed to the other process via IPC.
@@ -71,11 +101,17 @@ class FileDescriptor
   // Tests mHandle against a well-known invalid platform-specific file handle
   // (e.g. -1 on POSIX, INVALID_HANDLE_VALUE on Windows).
   bool
-  IsValid() const;
+  IsValid() const
+  {
+    return IsValid(mHandle);
+  }
 
   PlatformHandleType
   PlatformHandle() const
   {
+    if (mHandleCreatedByOtherProcess) {
+      mHandleCreatedByOtherProcessWasUsed = true;
+    }
     return mHandle;
   }
 
@@ -86,7 +122,26 @@ class FileDescriptor
   }
 
 private:
+  static bool
+  IsValid(PlatformHandleType aHandle);
+
+  void
+  DuplicateInCurrentProcess(PlatformHandleType aHandle);
+
+  void
+  CloseCurrentProcessHandle();
+
   PlatformHandleType mHandle;
+
+  // If this is true then this instance is created by IPDL to ferry a handle to
+  // its eventual consumer and we never close the handle. If this is false then
+  // we are a RAII wrapper around the handle and we close the handle on
+  // destruction.
+  bool mHandleCreatedByOtherProcess;
+
+  // This is to ensure that we don't leak the handle (which is only possible
+  // when we're in the receiving process).
+  mutable DebugOnly<bool> mHandleCreatedByOtherProcessWasUsed;
 };
 
 } // namespace ipc