Add POSIX shared memory support for Mac

This re-adds POSIX shared memory support on Mac. This
is for use in the sharing of field trials using shared memory. You
can see the design doc for this project at:
https://docs.google.com/document/d/1mTjD99PchmCGYtLvpxaXFObVnid1zmXIsWBI7GjuJgo/
and the motivation for this change under "Port to other platforms".

NOTE: turning on NOPRESUBMIT because this re-adds some banned calls to AllowIO.

BUG=671228
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2555483002
Cr-Commit-Position: refs/heads/master@{#437709}

base/BUILD.gn

@@ -512,6 +512,8 @@ component("base") {
     "memory/shared_memory_handle.h",
     "memory/shared_memory_handle_mac.cc",
     "memory/shared_memory_handle_win.cc",
+    "memory/shared_memory_helper.cc",
+    "memory/shared_memory_helper.h",
     "memory/shared_memory_mac.cc",
     "memory/shared_memory_nacl.cc",
     "memory/shared_memory_posix.cc",
@@ -1166,6 +1168,8 @@ component("base") {
       "memory/discardable_memory_allocator.h",
       "memory/discardable_shared_memory.cc",
       "memory/discardable_shared_memory.h",
+      "memory/shared_memory_helper.cc",
+      "memory/shared_memory_helper.h",
       "memory/shared_memory_posix.cc",
       "native_library.cc",
       "native_library_posix.cc",
@@ -1244,6 +1248,8 @@ component("base") {
     ]
 
     sources -= [
+      "memory/shared_memory_helper.cc",
+      "memory/shared_memory_helper.h",
       "message_loop/message_pump_libevent.cc",
       "strings/string16.cc",
     ]

base/memory/shared_memory.h

@@ -34,7 +34,10 @@ class FilePath;
 
 // Options for creating a shared memory object.
 struct BASE_EXPORT SharedMemoryCreateOptions {
-#if !(defined(OS_MACOSX) && !defined(OS_IOS))
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The type of OS primitive that should back the SharedMemory object.
+  SharedMemoryHandle::Type type = SharedMemoryHandle::MACH;
+#else
   // DEPRECATED (crbug.com/345734):
   // If NULL, the object is anonymous.  This pointer is owned by the caller
   // and must live through the call to Create().
@@ -46,7 +49,7 @@ struct BASE_EXPORT SharedMemoryCreateOptions {
   // shared memory must not exist.  This flag is meaningless unless
   // name_deprecated is non-NULL.
   bool open_existing_deprecated = false;
-#endif  // !(defined(OS_MACOSX) && !defined(OS_IOS))
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
   // Size of the shared memory object to be created.
   // When opening an existing object, this has no effect.
@@ -101,7 +104,7 @@ class BASE_EXPORT SharedMemory {
   // The caller is responsible for destroying the duplicated OS primitive.
   static SharedMemoryHandle DuplicateHandle(const SharedMemoryHandle& handle);
 
-#if defined(OS_POSIX) && !(defined(OS_MACOSX) && !defined(OS_IOS))
+#if defined(OS_POSIX)
   // This method requires that the SharedMemoryHandle is backed by a POSIX fd.
   static int GetFdFromSharedMemoryHandle(const SharedMemoryHandle& handle);
 #endif
@@ -255,13 +258,16 @@ class BASE_EXPORT SharedMemory {
  private:
 #if defined(OS_POSIX) && !defined(OS_NACL) && !defined(OS_ANDROID) && \
     !(defined(OS_MACOSX) && !defined(OS_IOS))
-  bool PrepareMapFile(ScopedFILE fp, ScopedFD readonly);
   bool FilePathForMemoryName(const std::string& mem_name, FilePath* path);
 #endif
+
   enum ShareMode {
     SHARE_READONLY,
     SHARE_CURRENT_MODE,
   };
+
+  bool Share(SharedMemoryHandle* new_handle, ShareMode share_mode);
+
   bool ShareToProcessCommon(ProcessHandle process,
                             SharedMemoryHandle* new_handle,
                             bool close_self,
@@ -276,6 +282,12 @@ class BASE_EXPORT SharedMemory {
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
   // The OS primitive that backs the shared memory region.
   SharedMemoryHandle shm_;
+
+  // The mechanism by which the memory is mapped. Only valid if |memory_| is not
+  // |nullptr|.
+  SharedMemoryHandle::Type mapped_memory_mechanism_;
+
+  int readonly_mapped_file_;
 #elif defined(OS_POSIX)
   int                mapped_file_;
   int                readonly_mapped_file_;

base/memory/shared_memory_handle.h

@@ -15,6 +15,7 @@
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
 #include <mach/mach.h>
 #include "base/base_export.h"
+#include "base/file_descriptor_posix.h"
 #include "base/macros.h"
 #include "base/process/process_handle.h"
 #elif defined(OS_POSIX)
@@ -83,9 +84,25 @@ class BASE_EXPORT SharedMemoryHandle {
 #else
 class BASE_EXPORT SharedMemoryHandle {
  public:
+  enum Type {
+    // The SharedMemoryHandle is backed by a POSIX fd.
+    POSIX,
+    // The SharedMemoryHandle is backed by the Mach primitive "memory object".
+    MACH,
+  };
+
   // The default constructor returns an invalid SharedMemoryHandle.
   SharedMemoryHandle();
 
+  // Constructs a SharedMemoryHandle backed by the components of a
+  // FileDescriptor. The newly created instance has the same ownership semantics
+  // as base::FileDescriptor. This typically means that the SharedMemoryHandle
+  // takes ownership of the |fd| if |auto_close| is true. Unfortunately, it's
+  // common for existing code to make shallow copies of SharedMemoryHandle, and
+  // the one that is finally passed into a base::SharedMemory is the one that
+  // "consumes" the fd.
+  explicit SharedMemoryHandle(const base::FileDescriptor& file_descriptor);
+
   // Makes a Mach-based SharedMemoryHandle of the given size. On error,
   // subsequent calls to IsValid() return false.
   explicit SharedMemoryHandle(mach_vm_size_t size);
@@ -104,7 +121,8 @@ class BASE_EXPORT SharedMemoryHandle {
   // OS primitives.
   SharedMemoryHandle& operator=(const SharedMemoryHandle& handle);
 
-  // Duplicates the underlying OS resources.
+  // Duplicates the underlying OS resources. Assumes the SharedMemoryHandle is
+  // of type MACH.
   SharedMemoryHandle Duplicate() const;
 
   // Comparison operators.
@@ -120,7 +138,7 @@ class BASE_EXPORT SharedMemoryHandle {
   mach_port_t GetMemoryObject() const;
 
   // Returns false on a failure to determine the size. On success, populates the
-  // output variable |size|. Returns 0 if the handle is invalid.
+  // output variable |size|.
   bool GetSize(size_t* size) const;
 
   // The SharedMemoryHandle must be valid.
@@ -136,24 +154,36 @@ class BASE_EXPORT SharedMemoryHandle {
   bool OwnershipPassesToIPC() const;
 
  private:
+  friend class SharedMemory;
+
   // Shared code between copy constructor and operator=.
   void CopyRelevantData(const SharedMemoryHandle& handle);
 
-  mach_port_t memory_object_ = MACH_PORT_NULL;
+  Type type_;
 
-  // The size of the shared memory region when |type_| is MACH. Only
-  // relevant if |memory_object_| is not |MACH_PORT_NULL|.
-  mach_vm_size_t size_ = 0;
+  // Each instance of a SharedMemoryHandle is backed either by a POSIX fd or a
+  // mach port. |type_| determines the backing member.
+  union {
+    FileDescriptor file_descriptor_;
 
-  // The pid of the process in which |memory_object_| is usable. Only
-  // relevant if |memory_object_| is not |MACH_PORT_NULL|.
-  base::ProcessId pid_ = 0;
+    struct {
+      mach_port_t memory_object_;
 
-  // Whether passing this object as a parameter to an IPC message passes
-  // ownership of |memory_object_| to the IPC stack. This is meant to mimic
-  // the behavior of the |auto_close| parameter of FileDescriptor.
-  // Defaults to |false|.
-  bool ownership_passes_to_ipc_ = false;
+      // The size of the shared memory region when |type_| is MACH. Only
+      // relevant if |memory_object_| is not |MACH_PORT_NULL|.
+      mach_vm_size_t size_;
+
+      // The pid of the process in which |memory_object_| is usable. Only
+      // relevant if |memory_object_| is not |MACH_PORT_NULL|.
+      base::ProcessId pid_;
+
+      // Whether passing this object as a parameter to an IPC message passes
+      // ownership of |memory_object_| to the IPC stack. This is meant to mimic
+      // the behavior of the |auto_close| parameter of FileDescriptor.
+      // Defaults to |false|.
+      bool ownership_passes_to_ipc_;
+    };
+  };
 };
 #endif
 

base/memory/shared_memory_handle_mac.cc

@@ -10,13 +10,20 @@
 #include <unistd.h>
 
 #include "base/mac/mac_util.h"
+#include "base/mac/mach_logging.h"
 #include "base/posix/eintr_wrapper.h"
 
 namespace base {
 
-SharedMemoryHandle::SharedMemoryHandle() {}
+SharedMemoryHandle::SharedMemoryHandle()
+    : type_(MACH), memory_object_(MACH_PORT_NULL) {}
+
+SharedMemoryHandle::SharedMemoryHandle(
+    const base::FileDescriptor& file_descriptor)
+    : type_(POSIX), file_descriptor_(file_descriptor) {}
 
 SharedMemoryHandle::SharedMemoryHandle(mach_vm_size_t size) {
+  type_ = MACH;
   mach_port_t named_right;
   kern_return_t kr = mach_make_memory_entry_64(
       mach_task_self(),
@@ -39,7 +46,8 @@ SharedMemoryHandle::SharedMemoryHandle(mach_vm_size_t size) {
 SharedMemoryHandle::SharedMemoryHandle(mach_port_t memory_object,
                                        mach_vm_size_t size,
                                        base::ProcessId pid)
-    : memory_object_(memory_object),
+    : type_(MACH),
+      memory_object_(memory_object),
       size_(size),
       pid_(pid),
       ownership_passes_to_ipc_(false) {}
@@ -53,6 +61,7 @@ SharedMemoryHandle& SharedMemoryHandle::operator=(
   if (this == &handle)
     return *this;
 
+  type_ = handle.type_;
   CopyRelevantData(handle);
   return *this;
 }
@@ -74,8 +83,16 @@ bool SharedMemoryHandle::operator==(const SharedMemoryHandle& handle) const {
   if (!IsValid() && !handle.IsValid())
     return true;
 
-  return memory_object_ == handle.memory_object_ && size_ == handle.size_ &&
-         pid_ == handle.pid_;
+  if (type_ != handle.type_)
+    return false;
+
+  switch (type_) {
+    case POSIX:
+      return file_descriptor_.fd == handle.file_descriptor_.fd;
+    case MACH:
+      return memory_object_ == handle.memory_object_ && size_ == handle.size_ &&
+             pid_ == handle.pid_;
+  }
 }
 
 bool SharedMemoryHandle::operator!=(const SharedMemoryHandle& handle) const {
@@ -83,10 +100,16 @@ bool SharedMemoryHandle::operator!=(const SharedMemoryHandle& handle) const {
 }
 
 bool SharedMemoryHandle::IsValid() const {
-  return memory_object_ != MACH_PORT_NULL;
+  switch (type_) {
+    case POSIX:
+      return file_descriptor_.fd >= 0;
+    case MACH:
+      return memory_object_ != MACH_PORT_NULL;
+  }
 }
 
 mach_port_t SharedMemoryHandle::GetMemoryObject() const {
+  DCHECK_EQ(type_, MACH);
   return memory_object_;
 }
 
@@ -96,8 +119,19 @@ bool SharedMemoryHandle::GetSize(size_t* size) const {
     return true;
   }
 
-  *size = size_;
-  return true;
+  switch (type_) {
+    case SharedMemoryHandle::POSIX:
+      struct stat st;
+      if (fstat(file_descriptor_.fd, &st) != 0)
+        return false;
+      if (st.st_size < 0)
+        return false;
+      *size = st.st_size;
+      return true;
+    case SharedMemoryHandle::MACH:
+      *size = size_;
+      return true;
+  }
 }
 
 bool SharedMemoryHandle::MapAt(off_t offset,
@@ -105,42 +139,69 @@ bool SharedMemoryHandle::MapAt(off_t offset,
                                void** memory,
                                bool read_only) {
   DCHECK(IsValid());
-  DCHECK_EQ(pid_, GetCurrentProcId());
-  kern_return_t kr = mach_vm_map(
-      mach_task_self(),
-      reinterpret_cast<mach_vm_address_t*>(memory),  // Output parameter
-      bytes,
-      0,  // Alignment mask
-      VM_FLAGS_ANYWHERE, memory_object_, offset,
-      FALSE,                                           // Copy
-      VM_PROT_READ | (read_only ? 0 : VM_PROT_WRITE),  // Current protection
-      VM_PROT_WRITE | VM_PROT_READ | VM_PROT_IS_MASK,  // Maximum protection
-      VM_INHERIT_NONE);
-  return kr == KERN_SUCCESS;
+  switch (type_) {
+    case SharedMemoryHandle::POSIX:
+      *memory = mmap(nullptr, bytes, PROT_READ | (read_only ? 0 : PROT_WRITE),
+                     MAP_SHARED, file_descriptor_.fd, offset);
+      return *memory != MAP_FAILED;
+    case SharedMemoryHandle::MACH:
+      DCHECK_EQ(pid_, GetCurrentProcId());
+      kern_return_t kr = mach_vm_map(
+          mach_task_self(),
+          reinterpret_cast<mach_vm_address_t*>(memory),    // Output parameter
+          bytes,
+          0,                                               // Alignment mask
+          VM_FLAGS_ANYWHERE,
+          memory_object_,
+          offset,
+          FALSE,                                           // Copy
+          VM_PROT_READ | (read_only ? 0 : VM_PROT_WRITE),  // Current protection
+          VM_PROT_WRITE | VM_PROT_READ | VM_PROT_IS_MASK,  // Maximum protection
+          VM_INHERIT_NONE);
+      return kr == KERN_SUCCESS;
+  }
 }
 
 void SharedMemoryHandle::Close() const {
   if (!IsValid())
     return;
 
-  kern_return_t kr = mach_port_deallocate(mach_task_self(), memory_object_);
-  if (kr != KERN_SUCCESS)
-    DPLOG(ERROR) << "Error deallocating mach port: " << kr;
+  switch (type_) {
+    case POSIX:
+      if (IGNORE_EINTR(close(file_descriptor_.fd)) < 0)
+        DPLOG(ERROR) << "Error closing fd";
+      break;
+    case MACH:
+      kern_return_t kr = mach_port_deallocate(mach_task_self(), memory_object_);
+      if (kr != KERN_SUCCESS)
+        MACH_DLOG(ERROR, kr) << "Error deallocating mach port";
+      break;
+  }
 }
 
 void SharedMemoryHandle::SetOwnershipPassesToIPC(bool ownership_passes) {
+  DCHECK_EQ(type_, MACH);
   ownership_passes_to_ipc_ = ownership_passes;
 }
 
 bool SharedMemoryHandle::OwnershipPassesToIPC() const {
+  DCHECK_EQ(type_, MACH);
   return ownership_passes_to_ipc_;
 }
 
 void SharedMemoryHandle::CopyRelevantData(const SharedMemoryHandle& handle) {
-  memory_object_ = handle.memory_object_;
-  size_ = handle.size_;
-  pid_ = handle.pid_;
-  ownership_passes_to_ipc_ = handle.ownership_passes_to_ipc_;
+  type_ = handle.type_;
+  switch (type_) {
+    case POSIX:
+      file_descriptor_ = handle.file_descriptor_;
+      break;
+    case MACH:
+      memory_object_ = handle.memory_object_;
+      size_ = handle.size_;
+      pid_ = handle.pid_;
+      ownership_passes_to_ipc_ = handle.ownership_passes_to_ipc_;
+      break;
+  }
 }
 
 }  // namespace base

base/memory/shared_memory_helper.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/shared_memory_helper.h"
+
+#include "base/threading/thread_restrictions.h"
+
+namespace base {
+
+struct ScopedPathUnlinkerTraits {
+  static const FilePath* InvalidValue() { return nullptr; }
+
+  static void Free(const FilePath* path) {
+    if (unlink(path->value().c_str()))
+      PLOG(WARNING) << "unlink";
+  }
+};
+
+// Unlinks the FilePath when the object is destroyed.
+using ScopedPathUnlinker =
+    ScopedGeneric<const FilePath*, ScopedPathUnlinkerTraits>;
+
+#if !defined(OS_ANDROID)
+bool CreateAnonymousSharedMemory(const SharedMemoryCreateOptions& options,
+                                 ScopedFILE* fp,
+                                 ScopedFD* readonly_fd,
+                                 FilePath* path) {
+#if !(defined(OS_MACOSX) && !defined(OS_IOS))
+  // It doesn't make sense to have a open-existing private piece of shmem
+  DCHECK(!options.open_existing_deprecated);
+#endif  // !(defined(OS_MACOSX) && !defined(OS_IOS)
+  // Q: Why not use the shm_open() etc. APIs?
+  // A: Because they're limited to 4mb on OS X.  FFFFFFFUUUUUUUUUUU
+  FilePath directory;
+  ScopedPathUnlinker path_unlinker;
+  if (!GetShmemTempDir(options.executable, &directory))
+    return false;
+
+  fp->reset(base::CreateAndOpenTemporaryFileInDir(directory, path));
+
+  if (!*fp)
+    return false;
+
+  // Deleting the file prevents anyone else from mapping it in (making it
+  // private), and prevents the need for cleanup (once the last fd is
+  // closed, it is truly freed).
+  path_unlinker.reset(path);
+
+  if (options.share_read_only) {
+    // Also open as readonly so that we can ShareReadOnlyToProcess.
+    readonly_fd->reset(HANDLE_EINTR(open(path->value().c_str(), O_RDONLY)));
+    if (!readonly_fd->is_valid()) {
+      DPLOG(ERROR) << "open(\"" << path->value() << "\", O_RDONLY) failed";
+      fp->reset();
+      return false;
+    }
+  }
+  return true;
+}
+
+bool PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd, int* mapped_file,
+                    int* readonly_mapped_file) {
+  DCHECK_EQ(-1, *mapped_file);
+  DCHECK_EQ(-1, *readonly_mapped_file);
+  if (fp == NULL)
+    return false;
+
+  // This function theoretically can block on the disk, but realistically
+  // the temporary files we create will just go into the buffer cache
+  // and be deleted before they ever make it out to disk.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
+  if (readonly_fd.is_valid()) {
+    struct stat st = {};
+    if (fstat(fileno(fp.get()), &st))
+      NOTREACHED();
+
+    struct stat readonly_st = {};
+    if (fstat(readonly_fd.get(), &readonly_st))
+      NOTREACHED();
+    if (st.st_dev != readonly_st.st_dev || st.st_ino != readonly_st.st_ino) {
+      LOG(ERROR) << "writable and read-only inodes don't match; bailing";
+      return false;
+    }
+  }
+
+  *mapped_file = HANDLE_EINTR(dup(fileno(fp.get())));
+  if (*mapped_file == -1) {
+    NOTREACHED() << "Call to dup failed, errno=" << errno;
+  }
+  *readonly_mapped_file = readonly_fd.release();
+
+  return true;
+}
+#endif  // !defined(OS_ANDROID)
+
+}  // namespace base

base/memory/shared_memory_helper.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_MEMORY_SHARED_MEMORY_HELPER_H_
+#define BASE_MEMORY_SHARED_MEMORY_HELPER_H_
+
+#include "base/memory/shared_memory.h"
+
+#include <fcntl.h>
+
+namespace base {
+
+#if !defined(OS_ANDROID)
+// Makes a temporary file, fdopens it, and then unlinks it. |fp| is populated
+// with the fdopened FILE. |readonly_fd| is populated with the opened fd if
+// options.share_read_only is true. |path| is populated with the location of
+// the file before it was unlinked.
+// Returns false if there's an unhandled failure.
+bool CreateAnonymousSharedMemory(const SharedMemoryCreateOptions& options,
+                                 ScopedFILE* fp,
+                                 ScopedFD* readonly_fd,
+                                 FilePath* path);
+
+// Takes the outputs of CreateAnonymousSharedMemory and maps them properly to
+// |mapped_file| or |readonly_mapped_file|, depending on which one is populated.
+bool PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd, int* mapped_file,
+                    int* readonly_mapped_file);
+#endif
+
+}  // namespace base
+
+#endif  // BASE_MEMORY_SHARED_MEMORY_HELPER_H_

base/memory/shared_memory_mac.cc

@@ -4,22 +4,33 @@
 
 #include "base/memory/shared_memory.h"
 
+#include <errno.h>
 #include <mach/mach_vm.h>
+#include <stddef.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
-#include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_mach_vm.h"
+#include "base/memory/shared_memory_helper.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/posix/safe_strerror.h"
 #include "base/process/process_metrics.h"
-#include "base/profiler/scoped_tracker.h"
 #include "base/scoped_generic.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 
+#if defined(OS_MACOSX)
+#include "base/mac/foundation_util.h"
+#endif  // OS_MACOSX
+
 namespace base {
 
 namespace {
@@ -67,13 +78,21 @@ bool MakeMachSharedMemoryHandleReadOnly(SharedMemoryHandle* new_handle,
   return true;
 }
 
+
 }  // namespace
 
 SharedMemory::SharedMemory()
-    : mapped_size_(0), memory_(NULL), read_only_(false), requested_size_(0) {}
+    : mapped_memory_mechanism_(SharedMemoryHandle::MACH),
+      readonly_mapped_file_(-1),
+      mapped_size_(0),
+      memory_(NULL),
+      read_only_(false),
+      requested_size_(0) {}
 
 SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
     : shm_(handle),
+      mapped_memory_mechanism_(SharedMemoryHandle::POSIX),
+      readonly_mapped_file_(-1),
       mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
@@ -101,8 +120,7 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
 
 // static
 size_t SharedMemory::GetHandleLimit() {
-  // This should be effectively unlimited on OS X.
-  return 10000;
+  return GetMaxFds();
 }
 
 // static
@@ -111,6 +129,12 @@ SharedMemoryHandle SharedMemory::DuplicateHandle(
   return handle.Duplicate();
 }
 
+// static
+int SharedMemory::GetFdFromSharedMemoryHandle(
+    const SharedMemoryHandle& handle) {
+  return handle.file_descriptor_.fd;
+}
+
 bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   return CreateAnonymous(size) && Map(size);
 }
@@ -125,20 +149,53 @@ bool SharedMemory::GetSizeFromSharedMemoryHandle(
 // Chromium mostly only uses the unique/private shmem as specified by
 // "name == L"". The exception is in the StatsTable.
 bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466437
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile1(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "466437 SharedMemory::Create::Start"));
   DCHECK(!shm_.IsValid());
   if (options.size == 0) return false;
 
   if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
     return false;
 
-  shm_ = SharedMemoryHandle(options.size);
+  if (options.type == SharedMemoryHandle::MACH) {
+    shm_ = SharedMemoryHandle(options.size);
+    requested_size_ = options.size;
+    return shm_.IsValid();
+  }
+
+  // This function theoretically can block on the disk. Both profiling of real
+  // users and local instrumentation shows that this is a real problem.
+  // https://code.google.com/p/chromium/issues/detail?id=466437
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
+  ScopedFILE fp;
+  ScopedFD readonly_fd;
+
+  FilePath path;
+  bool result = CreateAnonymousSharedMemory(options, &fp, &readonly_fd, &path);
+  if (!result)
+    return false;
+
+  if (!fp) {
+    PLOG(ERROR) << "Creating shared memory in " << path.value() << " failed";
+    return false;
+  }
+
+  // Get current size.
+  struct stat stat;
+  if (fstat(fileno(fp.get()), &stat) != 0)
+    return false;
+  const size_t current_size = stat.st_size;
+  if (current_size != options.size) {
+    if (HANDLE_EINTR(ftruncate(fileno(fp.get()), options.size)) != 0)
+      return false;
+  }
   requested_size_ = options.size;
-  return shm_.IsValid();
+
+  int mapped_file = -1;
+  result = PrepareMapFile(std::move(fp), std::move(readonly_fd), &mapped_file,
+                          &readonly_mapped_file_);
+
+  shm_ = SharedMemoryHandle(FileDescriptor(mapped_file, false));
+  return result;
 }
 
 bool SharedMemory::MapAt(off_t offset, size_t bytes) {
@@ -154,6 +211,7 @@ bool SharedMemory::MapAt(off_t offset, size_t bytes) {
     mapped_size_ = bytes;
     DCHECK_EQ(0U, reinterpret_cast<uintptr_t>(memory_) &
                       (SharedMemory::MAP_MINIMUM_ALIGNMENT - 1));
+    mapped_memory_mechanism_ = shm_.type_;
   } else {
     memory_ = NULL;
   }
@@ -165,16 +223,30 @@ bool SharedMemory::Unmap() {
   if (memory_ == NULL)
     return false;
 
-  mach_vm_deallocate(mach_task_self(),
-                     reinterpret_cast<mach_vm_address_t>(memory_),
-                     mapped_size_);
+  switch (mapped_memory_mechanism_) {
+    case SharedMemoryHandle::POSIX:
+      munmap(memory_, mapped_size_);
+      break;
+    case SharedMemoryHandle::MACH:
+      mach_vm_deallocate(mach_task_self(),
+                         reinterpret_cast<mach_vm_address_t>(memory_),
+                         mapped_size_);
+      break;
+  }
+
   memory_ = NULL;
   mapped_size_ = 0;
   return true;
 }
 
 SharedMemoryHandle SharedMemory::handle() const {
-  return shm_;
+  switch (shm_.type_) {
+    case SharedMemoryHandle::POSIX:
+      return SharedMemoryHandle(
+          FileDescriptor(shm_.file_descriptor_.fd, false));
+    case SharedMemoryHandle::MACH:
+      return shm_;
+  }
 }
 
 SharedMemoryHandle SharedMemory::TakeHandle() {
@@ -186,33 +258,70 @@ SharedMemoryHandle SharedMemory::TakeHandle() {
 void SharedMemory::Close() {
   shm_.Close();
   shm_ = SharedMemoryHandle();
+  if (shm_.type_ == SharedMemoryHandle::POSIX) {
+    if (readonly_mapped_file_ > 0) {
+      if (IGNORE_EINTR(close(readonly_mapped_file_)) < 0)
+        PLOG(ERROR) << "close";
+      readonly_mapped_file_ = -1;
+    }
+  }
 }
 
-bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
-                                        SharedMemoryHandle* new_handle,
-                                        bool close_self,
-                                        ShareMode share_mode) {
-  DCHECK(shm_.IsValid());
+bool SharedMemory::Share(SharedMemoryHandle* new_handle, ShareMode share_mode) {
+  if (shm_.type_ == SharedMemoryHandle::MACH) {
+    DCHECK(shm_.IsValid());
+
+    bool success = false;
+    switch (share_mode) {
+      case SHARE_CURRENT_MODE:
+        *new_handle = shm_.Duplicate();
+        success = true;
+        break;
+      case SHARE_READONLY:
+        success = MakeMachSharedMemoryHandleReadOnly(new_handle, shm_, memory_);
+        break;
+    }
+
+    if (success)
+      new_handle->SetOwnershipPassesToIPC(true);
+
+    return success;
+  }
 
-  bool success = false;
+  int handle_to_dup = -1;
   switch (share_mode) {
     case SHARE_CURRENT_MODE:
-      *new_handle = shm_.Duplicate();
-      success = true;
+      handle_to_dup = shm_.file_descriptor_.fd;
       break;
     case SHARE_READONLY:
-      success = MakeMachSharedMemoryHandleReadOnly(new_handle, shm_, memory_);
+      // We could imagine re-opening the file from /dev/fd, but that can't make
+      // it readonly on Mac: https://codereview.chromium.org/27265002/#msg10
+      CHECK_GE(readonly_mapped_file_, 0);
+      handle_to_dup = readonly_mapped_file_;
       break;
   }
 
-  if (success)
-    new_handle->SetOwnershipPassesToIPC(true);
+  const int new_fd = HANDLE_EINTR(dup(handle_to_dup));
+  if (new_fd < 0) {
+    DPLOG(ERROR) << "dup() failed.";
+    return false;
+  }
+
+  new_handle->file_descriptor_.fd = new_fd;
+  new_handle->type_ = SharedMemoryHandle::POSIX;
+
+  return true;
+}
 
+bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
+                                        SharedMemoryHandle* new_handle,
+                                        bool close_self,
+                                        ShareMode share_mode) {
+  bool success = Share(new_handle, share_mode);
   if (close_self) {
     Unmap();
     Close();
   }
-
   return success;
 }
 

base/memory/shared_memory_posix.cc

@@ -14,12 +14,13 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
+#include "base/memory/shared_memory_helper.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/safe_strerror.h"
 #include "base/process/process_metrics.h"
-#include "base/profiler/scoped_tracker.h"
 #include "base/scoped_generic.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 
 #if defined(OS_ANDROID)
@@ -29,77 +30,6 @@
 
 namespace base {
 
-namespace {
-
-struct ScopedPathUnlinkerTraits {
-  static FilePath* InvalidValue() { return nullptr; }
-
-  static void Free(FilePath* path) {
-    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466437
-    // is fixed.
-    tracked_objects::ScopedTracker tracking_profile(
-        FROM_HERE_WITH_EXPLICIT_FUNCTION(
-            "466437 SharedMemory::Create::Unlink"));
-    if (unlink(path->value().c_str()))
-      PLOG(WARNING) << "unlink";
-  }
-};
-
-// Unlinks the FilePath when the object is destroyed.
-typedef ScopedGeneric<FilePath*, ScopedPathUnlinkerTraits> ScopedPathUnlinker;
-
-#if !defined(OS_ANDROID)
-// Makes a temporary file, fdopens it, and then unlinks it. |fp| is populated
-// with the fdopened FILE. |readonly_fd| is populated with the opened fd if
-// options.share_read_only is true. |path| is populated with the location of
-// the file before it was unlinked.
-// Returns false if there's an unhandled failure.
-bool CreateAnonymousSharedMemory(const SharedMemoryCreateOptions& options,
-                                 ScopedFILE* fp,
-                                 ScopedFD* readonly_fd,
-                                 FilePath* path) {
-  // It doesn't make sense to have a open-existing private piece of shmem
-  DCHECK(!options.open_existing_deprecated);
-  // Q: Why not use the shm_open() etc. APIs?
-  // A: Because they're limited to 4mb on OS X.  FFFFFFFUUUUUUUUUUU
-  FilePath directory;
-  ScopedPathUnlinker path_unlinker;
-  if (GetShmemTempDir(options.executable, &directory)) {
-    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466437
-    // is fixed.
-    tracked_objects::ScopedTracker tracking_profile(
-        FROM_HERE_WITH_EXPLICIT_FUNCTION(
-            "466437 SharedMemory::Create::OpenTemporaryFile"));
-    fp->reset(base::CreateAndOpenTemporaryFileInDir(directory, path));
-
-    // Deleting the file prevents anyone else from mapping it in (making it
-    // private), and prevents the need for cleanup (once the last fd is
-    // closed, it is truly freed).
-    if (*fp)
-      path_unlinker.reset(path);
-  }
-
-  if (*fp) {
-    if (options.share_read_only) {
-      // TODO(erikchen): Remove ScopedTracker below once
-      // http://crbug.com/466437 is fixed.
-      tracked_objects::ScopedTracker tracking_profile(
-          FROM_HERE_WITH_EXPLICIT_FUNCTION(
-              "466437 SharedMemory::Create::OpenReadonly"));
-      // Also open as readonly so that we can ShareReadOnlyToProcess.
-      readonly_fd->reset(HANDLE_EINTR(open(path->value().c_str(), O_RDONLY)));
-      if (!readonly_fd->is_valid()) {
-        DPLOG(ERROR) << "open(\"" << path->value() << "\", O_RDONLY) failed";
-        fp->reset();
-        return false;
-      }
-    }
-  }
-  return true;
-}
-#endif  // !defined(OS_ANDROID)
-}
-
 SharedMemory::SharedMemory()
     : mapped_file_(-1),
       readonly_mapped_file_(-1),
@@ -185,11 +115,6 @@ bool SharedMemory::GetSizeFromSharedMemoryHandle(
 // In case we want to delete it later, it may be useful to save the value
 // of mem_filename after FilePathForMemoryName().
 bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466437
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile1(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "466437 SharedMemory::Create::Start"));
   DCHECK_EQ(-1, mapped_file_);
   if (options.size == 0) return false;
 
@@ -292,7 +217,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
     return false;
   }
 
-  return PrepareMapFile(std::move(fp), std::move(readonly_fd));
+  return PrepareMapFile(std::move(fp), std::move(readonly_fd), &mapped_file_,
+                        &readonly_mapped_file_);
 }
 
 // Our current implementation of shmem is with mmap()ing of files.
@@ -324,7 +250,8 @@ bool SharedMemory::Open(const std::string& name, bool read_only) {
     DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed";
     return false;
   }
-  return PrepareMapFile(std::move(fp), std::move(readonly_fd));
+  return PrepareMapFile(std::move(fp), std::move(readonly_fd), &mapped_file_,
+                        &readonly_mapped_file_);
 }
 #endif  // !defined(OS_ANDROID)
 
@@ -401,44 +328,6 @@ void SharedMemory::Close() {
 }
 
 #if !defined(OS_ANDROID)
-bool SharedMemory::PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd) {
-  DCHECK_EQ(-1, mapped_file_);
-  DCHECK_EQ(-1, readonly_mapped_file_);
-  if (fp == NULL)
-    return false;
-
-  // This function theoretically can block on the disk, but realistically
-  // the temporary files we create will just go into the buffer cache
-  // and be deleted before they ever make it out to disk.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-
-  struct stat st = {};
-  if (fstat(fileno(fp.get()), &st))
-    NOTREACHED();
-  if (readonly_fd.is_valid()) {
-    struct stat readonly_st = {};
-    if (fstat(readonly_fd.get(), &readonly_st))
-      NOTREACHED();
-    if (st.st_dev != readonly_st.st_dev || st.st_ino != readonly_st.st_ino) {
-      LOG(ERROR) << "writable and read-only inodes don't match; bailing";
-      return false;
-    }
-  }
-
-  mapped_file_ = HANDLE_EINTR(dup(fileno(fp.get())));
-  if (mapped_file_ == -1) {
-    if (errno == EMFILE) {
-      LOG(WARNING) << "Shared memory creation failed; out of file descriptors";
-      return false;
-    } else {
-      NOTREACHED() << "Call to dup failed, errno=" << errno;
-    }
-  }
-  readonly_mapped_file_ = readonly_fd.release();
-
-  return true;
-}
-
 // For the given shmem named |mem_name|, return a filename to mmap()
 // (and possibly create).  Modifies |filename|.  Return false on
 // error, or true of we are happy.

base/memory/shared_memory_unittest.cc

@@ -316,8 +316,6 @@ TEST(SharedMemoryTest, AnonymousPrivate) {
   }
 }
 
-// The Mach functionality is tested in shared_memory_mac_unittest.cc.
-#if !(defined(OS_MACOSX) && !defined(OS_IOS))
 TEST(SharedMemoryTest, ShareReadOnly) {
   StringPiece contents = "Hello World";
 
@@ -325,6 +323,10 @@ TEST(SharedMemoryTest, ShareReadOnly) {
   SharedMemoryCreateOptions options;
   options.size = contents.size();
   options.share_read_only = true;
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The Mach functionality is tested in shared_memory_mac_unittest.cc.
+  options.type = SharedMemoryHandle::POSIX;
+#endif
   ASSERT_TRUE(writable_shmem.Create(options));
   ASSERT_TRUE(writable_shmem.Map(options.size));
   memcpy(writable_shmem.memory(), contents.data(), contents.size());
@@ -400,7 +402,6 @@ TEST(SharedMemoryTest, ShareReadOnly) {
 #error Unexpected platform; write a test that tries to make 'handle' writable.
 #endif  // defined(OS_POSIX) || defined(OS_WIN)
 }
-#endif  // !(defined(OS_MACOSX) && !defined(OS_IOS))
 
 TEST(SharedMemoryTest, ShareToSelf) {
   StringPiece contents = "Hello World";
@@ -474,7 +475,7 @@ TEST(SharedMemoryTest, MapTwice) {
   EXPECT_EQ(old_address, memory.memory());
 }
 
-#if defined(OS_POSIX) && !(defined(OS_MACOSX) && !defined(OS_IOS))
+#if defined(OS_POSIX)
 // This test is not applicable for iOS (crbug.com/399384).
 #if !defined(OS_IOS)
 // Create a shared memory object, mmap it, and mprotect it to PROT_EXEC.
@@ -485,6 +486,10 @@ TEST(SharedMemoryTest, AnonymousExecutable) {
   SharedMemoryCreateOptions options;
   options.size = kTestSize;
   options.executable = true;
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The Mach functionality is tested in shared_memory_mac_unittest.cc.
+  options.type = SharedMemoryHandle::POSIX;
+#endif
 
   EXPECT_TRUE(shared_memory.Create(options));
   EXPECT_TRUE(shared_memory.Map(shared_memory.requested_size()));
@@ -518,6 +523,10 @@ TEST(SharedMemoryTest, FilePermissionsAnonymous) {
   SharedMemory shared_memory;
   SharedMemoryCreateOptions options;
   options.size = kTestSize;
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The Mach functionality is tested in shared_memory_mac_unittest.cc.
+  options.type = SharedMemoryHandle::POSIX;
+#endif
   // Set a file mode creation mask that gives all permissions.
   ScopedUmaskSetter permissive_mask(S_IWGRP | S_IWOTH);
 
@@ -540,6 +549,10 @@ TEST(SharedMemoryTest, FilePermissionsNamed) {
   SharedMemory shared_memory;
   SharedMemoryCreateOptions options;
   options.size = kTestSize;
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The Mach functionality is tested in shared_memory_mac_unittest.cc.
+  options.type = SharedMemoryHandle::POSIX;
+#endif
 
   // Set a file mode creation mask that gives all permissions.
   ScopedUmaskSetter permissive_mask(S_IWGRP | S_IWOTH);
@@ -556,7 +569,7 @@ TEST(SharedMemoryTest, FilePermissionsNamed) {
 }
 #endif  // !defined(OS_ANDROID)
 
-#endif  // defined(OS_POSIX) && !(defined(OS_MACOSX) && !defined(OS_IOS))
+#endif  // defined(OS_POSIX)
 
 // Map() will return addresses which are aligned to the platform page size, this
 // varies from platform to platform though.  Since we'd like to advertise a