Chromium Code Reviews| Index: base/memory/shared_memory_mac.cc |
| diff --git a/base/memory/shared_memory_mac.cc b/base/memory/shared_memory_mac.cc |
| index a8f09555d96de9b6ab420b2123f5999cd8bf9223..feac578b8536ec5b78135ace589a46a70ae97961 100644 |
| --- a/base/memory/shared_memory_mac.cc |
| +++ b/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 { |
| @@ -70,10 +81,17 @@ bool MakeMachSharedMemoryHandleReadOnly(SharedMemoryHandle* new_handle, |
| } // 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 +119,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 +128,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 +148,52 @@ 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(mapped_file); |
| + return result; |
| } |
| bool SharedMemory::MapAt(off_t offset, size_t bytes) { |
| @@ -154,6 +209,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 +221,29 @@ 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(shm_.file_descriptor_.fd); |
| + case SharedMemoryHandle::MACH: |
| + return shm_; |
| + } |
| } |
| SharedMemoryHandle SharedMemory::TakeHandle() { |
| @@ -186,34 +255,76 @@ 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()); |
| + 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); |
| + |
| + if (close_self) { |
| + Unmap(); |
| + Close(); |
| + } |
| + |
| + 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) { |
| + if (close_self) { |
| + Unmap(); |
| + Close(); |
| + } |
| + DPLOG(ERROR) << "dup() failed."; |
|
Robert Sesek
2016/12/07 16:39:42
The DPLOG should go ahead of the if because errno
lawrencewu
2016/12/07 19:44:26
Done.
|
| + return false; |
| + } |
| + |
| + new_handle->file_descriptor_.fd = new_fd; |
| + new_handle->type_ = SharedMemoryHandle::POSIX; |
| if (close_self) { |
| Unmap(); |
| Close(); |
| } |
| - return success; |
| + return true; |
| } |
| } // namespace base |