Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(669)

Unified Diff: base/memory/shared_memory_mac.cc

Issue 1897623002: Remove POSIX shared memory. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Comments from amistry. Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/memory/shared_memory_handle_mac.cc ('k') | base/memory/shared_memory_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/memory/shared_memory_mac.cc
diff --git a/base/memory/shared_memory_mac.cc b/base/memory/shared_memory_mac.cc
index 6dd957621c78add7ec9840839e9efa8ed5d4b3b8..c9882fd0153e646e99556654df4641532f3e7f82 100644
--- a/base/memory/shared_memory_mac.cc
+++ b/base/memory/shared_memory_mac.cc
@@ -4,33 +4,22 @@
#include "base/memory/shared_memory.h"
-#include <errno.h>
-#include <fcntl.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/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 "build/build_config.h"
-#if defined(OS_MACOSX)
-#include "base/mac/foundation_util.h"
-#endif // OS_MACOSX
-
namespace base {
namespace {
@@ -78,81 +67,15 @@ bool MakeMachSharedMemoryHandleReadOnly(SharedMemoryHandle* new_handle,
return true;
}
-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;
-
-// 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) {
- // 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(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;
-}
-
} // namespace
SharedMemoryCreateOptions::SharedMemoryCreateOptions()
- : type(SharedMemoryHandle::MACH),
- size(0),
+ : size(0),
executable(false),
share_read_only(false) {}
SharedMemory::SharedMemory()
- : mapped_memory_mechanism_(SharedMemoryHandle::POSIX),
- readonly_mapped_file_(-1),
+ : readonly_mapped_file_(-1),
mapped_size_(0),
memory_(NULL),
read_only_(false),
@@ -160,7 +83,6 @@ SharedMemory::SharedMemory()
SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
: shm_(handle),
- mapped_memory_mechanism_(SharedMemoryHandle::POSIX),
readonly_mapped_file_(-1),
mapped_size_(0),
memory_(NULL),
@@ -189,7 +111,8 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
// static
size_t SharedMemory::GetHandleLimit() {
- return GetMaxFds();
+ // This should be effectively unlimited on OS X.
+ return 10000;
}
// static
@@ -198,27 +121,10 @@ SharedMemoryHandle SharedMemory::DuplicateHandle(
return handle.Duplicate();
}
-// static
-int SharedMemory::GetFdFromSharedMemoryHandle(
- const SharedMemoryHandle& handle) {
- return handle.GetFileDescriptor().fd;
-}
-
bool SharedMemory::CreateAndMapAnonymous(size_t size) {
return CreateAnonymous(size) && Map(size);
}
-bool SharedMemory::CreateAndMapAnonymousPosix(size_t size) {
- return CreateAnonymousPosix(size) && Map(size);
-}
-
-bool SharedMemory::CreateAnonymousPosix(size_t size) {
- SharedMemoryCreateOptions options;
- options.type = SharedMemoryHandle::POSIX;
- options.size = size;
- return Create(options);
-}
-
// static
bool SharedMemory::GetSizeFromSharedMemoryHandle(
const SharedMemoryHandle& handle,
@@ -240,42 +146,9 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
return false;
- 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;
- }
+ shm_ = SharedMemoryHandle(options.size);
requested_size_ = options.size;
-
- return PrepareMapFile(std::move(fp), std::move(readonly_fd));
+ return shm_.IsValid();
}
bool SharedMemory::MapAt(off_t offset, size_t bytes) {
@@ -291,7 +164,6 @@ 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_.GetType();
} else {
memory_ = NULL;
}
@@ -303,142 +175,49 @@ bool SharedMemory::Unmap() {
if (memory_ == NULL)
return false;
- 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;
- }
-
+ mach_vm_deallocate(mach_task_self(),
+ reinterpret_cast<mach_vm_address_t>(memory_),
+ mapped_size_);
memory_ = NULL;
mapped_size_ = 0;
return true;
}
SharedMemoryHandle SharedMemory::handle() const {
- switch (shm_.GetType()) {
- case SharedMemoryHandle::POSIX:
- return SharedMemoryHandle(shm_.GetFileDescriptor().fd, false);
- case SharedMemoryHandle::MACH:
- return shm_;
- }
+ return shm_;
}
void SharedMemory::Close() {
shm_.Close();
shm_ = SharedMemoryHandle();
- if (shm_.GetType() == SharedMemoryHandle::POSIX) {
- if (readonly_mapped_file_ > 0) {
- if (IGNORE_EINTR(close(readonly_mapped_file_)) < 0)
- PLOG(ERROR) << "close";
- readonly_mapped_file_ = -1;
- }
- }
-}
-
-bool SharedMemory::PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd) {
- DCHECK(!shm_.IsValid());
- 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;
- }
- }
-
- int 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;
- }
- }
- shm_ = SharedMemoryHandle(mapped_file, false);
- readonly_mapped_file_ = readonly_fd.release();
-
- return true;
}
bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
SharedMemoryHandle* new_handle,
bool close_self,
ShareMode share_mode) {
- if (shm_.GetType() == 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;
- }
+ DCHECK(shm_.IsValid());
- int handle_to_dup = -1;
+ bool success = false;
switch (share_mode) {
case SHARE_CURRENT_MODE:
- handle_to_dup = shm_.GetFileDescriptor().fd;
+ *new_handle = shm_.Duplicate();
+ success = true;
break;
case SHARE_READONLY:
- // 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_;
+ success = MakeMachSharedMemoryHandleReadOnly(new_handle, shm_, memory_);
break;
}
- const int new_fd = HANDLE_EINTR(dup(handle_to_dup));
- if (new_fd < 0) {
- if (close_self) {
- Unmap();
- Close();
- }
- DPLOG(ERROR) << "dup() failed.";
- return false;
- }
-
- new_handle->SetFileHandle(new_fd, true);
+ if (success)
+ new_handle->SetOwnershipPassesToIPC(true);
if (close_self) {
Unmap();
Close();
}
- return true;
+ return success;
}
} // namespace base
« no previous file with comments | « base/memory/shared_memory_handle_mac.cc ('k') | base/memory/shared_memory_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698