diff options
Diffstat (limited to 'base')
-rw-r--r-- | base/base.xcodeproj/project.pbxproj | 6 | ||||
-rw-r--r-- | base/file_util.h | 12 | ||||
-rw-r--r-- | base/file_util_linux.cc | 5 | ||||
-rw-r--r-- | base/file_util_mac.mm | 4 | ||||
-rw-r--r-- | base/file_util_posix.cc | 62 | ||||
-rw-r--r-- | base/file_util_unittest.cc | 46 | ||||
-rw-r--r-- | base/file_util_win.cc | 20 | ||||
-rw-r--r-- | base/shared_memory.h | 17 | ||||
-rw-r--r-- | base/shared_memory_posix.cc | 146 | ||||
-rw-r--r-- | base/shared_memory_unittest.cc | 84 | ||||
-rw-r--r-- | base/shared_memory_win.cc | 5 | ||||
-rw-r--r-- | base/stats_table_unittest.cc | 29 |
12 files changed, 389 insertions, 47 deletions
diff --git a/base/base.xcodeproj/project.pbxproj b/base/base.xcodeproj/project.pbxproj index a5c5f7d..c3c6f86 100644 --- a/base/base.xcodeproj/project.pbxproj +++ b/base/base.xcodeproj/project.pbxproj @@ -42,6 +42,7 @@ 32AC71B80F2E5321002BDDC8 /* jpeg_codec_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 32AC71B50F2E52FC002BDDC8 /* jpeg_codec_unittest.cc */; }; 32AC71BF0F2E5421002BDDC8 /* jpeg_codec.cc in Sources */ = {isa = PBXBuildFile; fileRef = 32AC71B60F2E530F002BDDC8 /* jpeg_codec.cc */; }; 32AC72300F2E64F7002BDDC8 /* libjpeg.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 32AC718E0F2E4F62002BDDC8 /* libjpeg.a */; }; + 33C591920F33B9950026A42A /* stats_table_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 33C591910F33B9950026A42A /* stats_table_unittest.cc */; }; 4D11B59A0E91730200EF7617 /* rand_util.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5940E9172F800EF7617 /* rand_util.cc */; }; 4D11B59B0E91730200EF7617 /* rand_util_posix.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5960E9172F800EF7617 /* rand_util_posix.cc */; }; 4D11B59C0E91730500EF7617 /* rand_util_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5970E9172F800EF7617 /* rand_util_unittest.cc */; }; @@ -205,7 +206,7 @@ isa = PBXContainerItemProxy; containerPortal = 32AC71890F2E4F62002BDDC8 /* libjpeg.xcodeproj */; proxyType = 1; - remoteGlobalIDString = D2AAC045055464E500DB518D /* libjpeg */; + remoteGlobalIDString = D2AAC045055464E500DB518D; remoteInfo = libjpeg; }; 32AC718D0F2E4F62002BDDC8 /* PBXContainerItemProxy */ = { @@ -409,6 +410,7 @@ 32AC71B50F2E52FC002BDDC8 /* jpeg_codec_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = jpeg_codec_unittest.cc; sourceTree = "<group>"; }; 32AC71B60F2E530F002BDDC8 /* jpeg_codec.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = jpeg_codec.cc; sourceTree = "<group>"; }; 32AC71B70F2E530F002BDDC8 /* jpeg_codec.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = jpeg_codec.h; sourceTree = "<group>"; }; + 33C591910F33B9950026A42A /* stats_table_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = stats_table_unittest.cc; sourceTree = "<group>"; }; 38246046390EAED65BB7C380 /* waitable_event_watcher_posix.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = waitable_event_watcher_posix.cc; sourceTree = "<group>"; }; 4D11B5940E9172F800EF7617 /* rand_util.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = rand_util.cc; sourceTree = "<group>"; }; 4D11B5950E9172F800EF7617 /* rand_util.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = rand_util.h; sourceTree = "<group>"; }; @@ -1040,6 +1042,7 @@ E491165C0E48A51E001EE8C3 /* stack_container_unittest.cc */, 825403780D92D2CF0006B936 /* stats_counters.h */, 825403790D92D2CF0006B936 /* stats_table.cc */, + 33C591910F33B9950026A42A /* stats_table_unittest.cc */, 8254037A0D92D2CF0006B936 /* stats_table.h */, 7BD8F7730E65E89800034DE9 /* string16.cc */, 821B91680DAABD7F00F350D7 /* string16.h */, @@ -1556,6 +1559,7 @@ BA5CC5840E788093004EDD45 /* shared_memory_unittest.cc in Sources */, 7BAE30E60E6D939F00C3F750 /* simple_thread_unittest.cc in Sources */, 7B78D39C0E54FE0100609465 /* singleton_unittest.cc in Sources */, + 33C591920F33B9950026A42A /* stats_table_unittest.cc in Sources */, 7B78D39D0E54FE0100609465 /* stack_container_unittest.cc in Sources */, 7B78D39E0E54FE0100609465 /* string_escape_unittest.cc in Sources */, 7B78D39F0E54FE0100609465 /* string_piece_unittest.cc in Sources */, diff --git a/base/file_util.h b/base/file_util.h index ba9f29a..297d257 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -260,15 +260,27 @@ bool IsDirectoryEmpty(const std::wstring& dir_path); bool GetTempDir(FilePath* path); // Deprecated temporary compatibility function. bool GetTempDir(std::wstring* path); +// Get a temporary directory for shared memory files. +// Only useful on POSIX; redirects to GetTempDir() on Windows. +bool GetShmemTempDir(FilePath* path); // Creates a temporary file. The full path is placed in |path|, and the // function returns true if was successful in creating the file. The file will // be empty and all handles closed after this function returns. // TODO(erikkay): rename this function and track down all of the callers. +// (Clarification of erik's comment: the intent is to rename the BlahFileName() +// calls into BlahFile(), since they create temp files (not temp filenames).) bool CreateTemporaryFileName(FilePath* path); // Deprecated temporary compatibility function. bool CreateTemporaryFileName(std::wstring* temp_file); +// Create and open a temporary file. File is opened for read/write. +// The full path is placed in |path|, and the function returns true if +// was successful in creating and opening the file. +FILE* CreateAndOpenTemporaryFile(FilePath* path); +// Like above but for shmem files. Only useful for POSIX. +FILE* CreateAndOpenTemporaryShmemFile(FilePath* path); + // Same as CreateTemporaryFileName but the file is created in |dir|. bool CreateTemporaryFileNameInDir(const std::wstring& dir, std::wstring* temp_file); diff --git a/base/file_util_linux.cc b/base/file_util_linux.cc index 1d13c89..50b774d 100644 --- a/base/file_util_linux.cc +++ b/base/file_util_linux.cc @@ -24,6 +24,11 @@ bool GetTempDir(FilePath* path) { return true; } +bool GetShmemTempDir(FilePath* path) { + *path = FilePath("/dev/shm"); + return true; +} + bool CopyFile(const FilePath& from_path, const FilePath& to_path) { int infile = open(from_path.value().c_str(), O_RDONLY); if (infile < 0) diff --git a/base/file_util_mac.mm b/base/file_util_mac.mm index 3c78fdd..7f81e8a 100644 --- a/base/file_util_mac.mm +++ b/base/file_util_mac.mm @@ -21,6 +21,10 @@ bool GetTempDir(FilePath* path) { return true; } +bool GetShmemTempDir(FilePath* path) { + return GetTempDir(path); +} + bool CopyFile(const FilePath& from_path, const FilePath& to_path) { return (copyfile(from_path.value().c_str(), to_path.value().c_str(), NULL, COPYFILE_ALL) == 0); diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index d44b743..6d3984e 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -38,7 +38,7 @@ std::wstring GetDirectoryFromPath(const std::wstring& path) { return UTF8ToWide(dirname(full_path)); } } - + bool AbsolutePath(FilePath* path) { char full_path[PATH_MAX]; if (realpath(path->value().c_str(), full_path) == NULL) @@ -57,7 +57,7 @@ bool Delete(const FilePath& path, bool recursive) { int test = stat64(path_str, &file_info); if (test != 0) { // The Windows version defines this condition as success. - bool ret = (errno == ENOENT || errno == ENOTDIR); + bool ret = (errno == ENOENT || errno == ENOTDIR); return ret; } if (!S_ISDIR(file_info.st_mode)) @@ -251,45 +251,79 @@ bool GetFileCreationLocalTimeFromHandle(int fd, LPSYSTEMTIME creation_time) { if (!file_handle) return false; - + FILETIME utc_filetime; if (!GetFileTime(file_handle, &utc_filetime, NULL, NULL)) return false; - + FILETIME local_filetime; if (!FileTimeToLocalFileTime(&utc_filetime, &local_filetime)) return false; - + return !!FileTimeToSystemTime(&local_filetime, creation_time); } bool GetFileCreationLocalTime(const std::string& filename, LPSYSTEMTIME creation_time) { ScopedHandle file_handle( - CreateFile(filename.c_str(), GENERIC_READ, + CreateFile(filename.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)); return GetFileCreationLocalTimeFromHandle(file_handle.Get(), creation_time); } #endif -bool CreateTemporaryFileName(FilePath* path) { - if (!GetTempDir(path)) - return false; - - *path = path->Append(kTempFileName); - std::string tmpdir_string = path->value(); +// Creates and opens a temporary file in |directory|, returning the +// file descriptor. |path| is set to the temporary file path. +// Note TODO(erikkay) comment in header for BlahFileName() calls; the +// intent is to rename these files BlahFile() (since they create +// files, not filenames). This function does NOT unlink() the file. +int CreateAndOpenFdForTemporaryFile(FilePath directory, FilePath* path) { + *path = directory.Append(kTempFileName); + const std::string& tmpdir_string = path->value(); // this should be OK since mkstemp just replaces characters in place char* buffer = const_cast<char*>(tmpdir_string.c_str()); - int fd = mkstemp(buffer); + return mkstemp(buffer); +} + +bool CreateTemporaryFileName(FilePath* path) { + FilePath directory; + if (!GetTempDir(&directory)) + return false; + int fd = CreateAndOpenFdForTemporaryFile(directory, path); if (fd < 0) return false; - close(fd); return true; } +FILE* CreateAndOpenTemporaryFile(FilePath* path) { + FilePath directory; + if (!GetTempDir(&directory)) + return false; + + int fd = CreateAndOpenFdForTemporaryFile(directory, path); + if (fd < 0) + return NULL; + + FILE *fp = fdopen(fd, "a+"); + return fp; +} + +FILE* CreateAndOpenTemporaryShmemFile(FilePath* path) { + FilePath directory; + if (!GetShmemTempDir(&directory)) + return false; + + int fd = CreateAndOpenFdForTemporaryFile(directory, path); + if (fd < 0) + return NULL; + + FILE *fp = fdopen(fd, "a+"); + return fp; +} + bool CreateTemporaryFileNameInDir(const std::wstring& dir, std::wstring* temp_file) { // Not implemented yet. diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index 98c2032..a00fc0b 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -688,10 +688,40 @@ TEST_F(FileUtilTest, CreateShortcutTest) { #endif TEST_F(FileUtilTest, CreateTemporaryFileNameTest) { - std::wstring temp_file; - ASSERT_TRUE(file_util::CreateTemporaryFileName(&temp_file)); - EXPECT_TRUE(file_util::PathExists(temp_file)); - EXPECT_TRUE(file_util::Delete(temp_file, false)); + std::wstring temp_files[3]; + for (int i = 0; i < 3; i++) { + ASSERT_TRUE(file_util::CreateTemporaryFileName(&(temp_files[i]))); + EXPECT_TRUE(file_util::PathExists(temp_files[i])); + EXPECT_FALSE(file_util::DirectoryExists(temp_files[i])); + } + for (int i = 0; i < 3; i++) + EXPECT_FALSE(temp_files[i] == temp_files[(i+1)%3]); + for (int i = 0; i < 3; i++) + EXPECT_TRUE(file_util::Delete(temp_files[i], false)); +} + +TEST_F(FileUtilTest, CreateAndOpenTemporaryFileNameTest) { + FilePath names[3]; + FILE *fps[3]; + int i; + + // Create; make sure they are open and exist. + for (i = 0; i < 3; ++i) { + fps[i] = file_util::CreateAndOpenTemporaryFile(&(names[i])); + ASSERT_TRUE(fps[i]); + EXPECT_TRUE(file_util::PathExists(names[i])); + } + + // Make sure all names are unique. + for (i = 0; i < 3; ++i) { + EXPECT_FALSE(names[i] == names[(i+1)%3]); + } + + // Close and delete. + for (i = 0; i < 3; ++i) { + EXPECT_TRUE(file_util::CloseFile(fps[i])); + EXPECT_TRUE(file_util::Delete(names[i], false)); + } } TEST_F(FileUtilTest, CreateNewTempDirectoryTest) { @@ -701,6 +731,12 @@ TEST_F(FileUtilTest, CreateNewTempDirectoryTest) { EXPECT_TRUE(file_util::Delete(temp_dir, false)); } +TEST_F(FileUtilTest, GetShmemTempDirTest) { + FilePath dir; + EXPECT_TRUE(file_util::GetShmemTempDir(&dir)); + EXPECT_TRUE(file_util::DirectoryExists(dir)); +} + TEST_F(FileUtilTest, CreateDirectoryTest) { FilePath test_root = test_dir_.Append(FILE_PATH_LITERAL("create_directory_test")); @@ -1015,7 +1051,7 @@ TEST_F(FileUtilTest, Contains) { #if defined(OS_WIN) EXPECT_TRUE(file_util::ContainsPath(foo, foo_caps.Append(FILE_PATH_LITERAL("bar.txt")))); - EXPECT_TRUE(file_util::ContainsPath(foo, + EXPECT_TRUE(file_util::ContainsPath(foo, FilePath(foo.value() + FILE_PATH_LITERAL("/bar.txt")))); #elif defined(OS_LINUX) EXPECT_FALSE(file_util::ContainsPath(foo, diff --git a/base/file_util_win.cc b/base/file_util_win.cc index 0947827..22c24cd 100644 --- a/base/file_util_win.cc +++ b/base/file_util_win.cc @@ -395,6 +395,10 @@ bool GetTempDir(FilePath* path) { return true; } +bool GetShmemTempDir(FilePath* path) { + return GetTempDir(path); +} + bool CreateTemporaryFileName(FilePath* path) { std::wstring temp_path, temp_file; @@ -409,6 +413,22 @@ bool CreateTemporaryFileName(FilePath* path) { return false; } +// On POSIX we have semantics to create and open a temporary file +// atomically. +// TODO(jrg): is there equivalent call to use on Windows instead of +// going 2-step? +FILE* CreateAndOpenTemporaryFile(FilePath* path) { + + if (!CreateTemporaryFileName(path)) { + return NULL; + } + return OpenFile(*path, "w+"); +} + +FILE* CreateAndOpenTemporaryShmemFile(FilePath* path) { + return CreateAndOpenTemporaryFile(path); +} + bool CreateTemporaryFileNameInDir(const std::wstring& dir, std::wstring* temp_file) { wchar_t temp_name[MAX_PATH + 1]; diff --git a/base/shared_memory.h b/base/shared_memory.h index ea60fa6..d96299a 100644 --- a/base/shared_memory.h +++ b/base/shared_memory.h @@ -24,6 +24,10 @@ typedef HANDLE SharedMemoryHandle; typedef HANDLE SharedMemoryLock; #elif defined(OS_POSIX) typedef int SharedMemoryHandle; +// TODO(port): these semaphores can leak if we crash, causing +// autobuilder problems. Transition to something easier to clean up +// (e.g. lockf/flock). +// TODO(port): make sure what we transition to is fast enough. typedef sem_t* SharedMemoryLock; #endif @@ -51,12 +55,18 @@ class SharedMemory { // If read_only is true, opens the memory as read-only. // If open_existing is true, and the shared memory already exists, // opens the existing shared memory and ignores the size parameter. + // If name is the empty string, use a unique name. // Returns true on success, false on failure. bool Create(const std::wstring& name, bool read_only, bool open_existing, size_t size); + // Deletes resources associated with a shared memory segment based on name. + // Not all platforms require this call. + bool Delete(const std::wstring& name); + // Opens a shared memory segment based on a name. // If read_only is true, opens for read-only access. + // If name is the empty string, use a unique name. // Returns true on success, false on failure. bool Open(const std::wstring& name, bool read_only); @@ -120,7 +130,9 @@ class SharedMemory { private: #if defined(OS_POSIX) - bool CreateOrOpen(const std::wstring &name, int posix_flags); + bool CreateOrOpen(const std::wstring &name, int posix_flags, size_t size); + bool FilenameForMemoryName(const std::wstring &memname, + std::wstring *filename); #endif bool ShareToProcessCommon(ProcessHandle process, SharedMemoryHandle* new_handle, @@ -132,6 +144,9 @@ class SharedMemory { bool read_only_; size_t max_size_; SharedMemoryLock lock_; +#if defined(OS_POSIX) + std::string sem_name_; +#endif DISALLOW_EVIL_CONSTRUCTORS(SharedMemory); }; diff --git a/base/shared_memory_posix.cc b/base/shared_memory_posix.cc index aa0c837..907ef2d 100644 --- a/base/shared_memory_posix.cc +++ b/base/shared_memory_posix.cc @@ -8,6 +8,7 @@ #include <fcntl.h> #include <sys/mman.h> +#include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" @@ -62,15 +63,27 @@ bool SharedMemory::Create(const std::wstring &name, bool read_only, if (!open_existing || mapped_file_ <= 0) posix_flags |= O_CREAT; - if (!CreateOrOpen(name, posix_flags)) + if (!CreateOrOpen(name, posix_flags, size)) return false; - if (ftruncate(mapped_file_, size) != 0) { - Close(); + max_size_ = size; + return true; +} + +// Our current implementation of shmem is with mmap()ing of files. +// These files need to be deleted explicitly. +// In practice this call is only needed for unit tests. +bool SharedMemory::Delete(const std::wstring& name) { + std::wstring mem_filename; + if (FilenameForMemoryName(name, &mem_filename) == false) return false; + + FilePath path(WideToUTF8(mem_filename)); + if (file_util::PathExists(path)) { + return file_util::Delete(path, false); } - max_size_ = size; + // Doesn't exist, so success. return true; } @@ -80,26 +93,127 @@ bool SharedMemory::Open(const std::wstring &name, bool read_only) { int posix_flags = 0; posix_flags |= read_only ? O_RDONLY : O_RDWR; - return CreateOrOpen(name, posix_flags); + return CreateOrOpen(name, posix_flags, 0); } -bool SharedMemory::CreateOrOpen(const std::wstring &name, int posix_flags) { +// For the given shmem named |memname|, return a filename to mmap() +// (and possibly create). Modifies |filename|. Return false on +// error, or true of we are happy. +bool SharedMemory::FilenameForMemoryName(const std::wstring &memname, + std::wstring *filename) { + std::wstring mem_filename; + + // mem_name will be used for a filename; make sure it doesn't + // contain anything which will confuse us. + DCHECK(memname.find_first_of(L"/") == std::string::npos); + DCHECK(memname.find_first_of(L"\0") == std::string::npos); + + FilePath temp_dir; + if (file_util::GetShmemTempDir(&temp_dir) == false) + return false; + + mem_filename = UTF8ToWide(temp_dir.value()); + file_util::AppendToPath(&mem_filename, L"com.google.chrome.shmem." + memname); + *filename = mem_filename; + return true; +} + +// Current expectation is that Cromium only really needs +// unique/private shmem as specified by "name == L"". +// TODO(port): confirm that assumption. +// TODO(jrg): there is no way to "clean up" all unused named shmem if +// we restart from a crash. (That isn't a new problem, but it is a problem.) +bool SharedMemory::CreateOrOpen(const std::wstring &name, + int posix_flags, size_t size) { DCHECK(mapped_file_ == -1); - name_ = L"/" + name; + file_util::ScopedFILE file_closer; + FILE *fp; + + if (name == L"") { + // It doesn't make sense to have a read-only private piece of shmem + DCHECK(posix_flags & (O_RDWR | O_WRONLY)); + + FilePath path; + fp = file_util::CreateAndOpenTemporaryShmemFile(&path); + name_ = UTF8ToWide(path.value()); + + // 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). + file_util::Delete(path, false); + } else { + std::wstring mem_filename; + if (FilenameForMemoryName(name, &mem_filename) == false) + return false; + + name_ = mem_filename; + std::string mode; + switch (posix_flags) { + case (O_RDWR | O_CREAT): + // Careful: "w+" will truncate if it already exists. + mode = "a+"; + break; + case O_RDWR: + mode = "r+"; + break; + case O_RDONLY: + mode = "r"; + break; + default: + NOTIMPLEMENTED(); + break; + } + + fp = file_util::OpenFile(mem_filename, mode.c_str()); + } - mode_t posix_mode = S_IRUSR | S_IWUSR; // owner read/write - std::string posix_name(WideToUTF8(name_)); - mapped_file_ = shm_open(posix_name.c_str(), posix_flags, posix_mode); - if (mapped_file_ < 0) + if (fp == NULL) return false; + file_closer.reset(fp); // close when we go out of scope + + // Make sure the (new) file is the right size. + // According to the man page, "Use of truncate() to extend a file is + // not portable." + if (size && (posix_flags & (O_RDWR | O_CREAT))) { + // Get current size. + size_t current_size = 0; + struct stat stat; + if (fstat(fileno(fp), &stat) != 0) + return false; + current_size = stat.st_size; + // Possibly grow. + if (current_size < size) { + if (fseeko(fp, current_size, SEEK_SET) != 0) + return false; + size_t writesize = size - current_size; + scoped_array<char> buf(new char[writesize]); + memset(buf.get(), 0, writesize); + if (fwrite(buf.get(), 1, writesize, fp) != writesize) { + return false; + } + if (fflush(fp) != 0) + return false; + } else if (current_size > size) { + // possibly shrink. + if ((ftruncate(fileno(fp), size) != 0) || + (fflush(fp) != 0)) { + return false; + } + } + } + + mapped_file_ = dup(fileno(fp)); + DCHECK(mapped_file_ >= 0); - posix_name += kSemaphoreSuffix; - lock_ = sem_open(posix_name.c_str(), O_CREAT, posix_mode, 1); + mode_t posix_mode = S_IRUSR | S_IWUSR; // owner read/write + // name_ that includes a tmpdir easily exceeds SEM_NAME_LEN, + // so we cannot use it directly as the basis for a sem_name_. + sem_name_ = WideToUTF8(name) + kSemaphoreSuffix; + lock_ = sem_open(sem_name_.c_str(), O_CREAT, posix_mode, 1); if (lock_ == SEM_FAILED) { close(mapped_file_); mapped_file_ = -1; - shm_unlink(posix_name.c_str()); lock_ = NULL; return false; } @@ -147,13 +261,11 @@ void SharedMemory::Close() { std::string posix_name(WideToUTF8(name_)); if (mapped_file_ > 0) { close(mapped_file_); - shm_unlink(posix_name.c_str()); mapped_file_ = -1; } if (lock_) { - posix_name += kSemaphoreSuffix; - sem_unlink(posix_name.c_str()); + sem_unlink(sem_name_.c_str()); lock_ = NULL; } } diff --git a/base/shared_memory_unittest.cc b/base/shared_memory_unittest.cc index 0c3e464..bdb6c86 100644 --- a/base/shared_memory_unittest.cc +++ b/base/shared_memory_unittest.cc @@ -4,6 +4,7 @@ #include "base/basictypes.h" #include "base/platform_thread.h" +#include "base/scoped_nsautorelease_pool.h" #include "base/shared_memory.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,12 +22,17 @@ class MultipleThreadMain : public PlatformThread::Delegate { explicit MultipleThreadMain(int16 id) : id_(id) {} ~MultipleThreadMain() {} + static void CleanUp() { + SharedMemory memory; + memory.Delete(test_name_); + } + // PlatformThread::Delegate interface. void ThreadMain() { + ScopedNSAutoreleasePool pool; // noop if not OSX const int kDataSize = 1024; - std::wstring test_name = L"SharedMemoryOpenThreadTest"; SharedMemory memory; - bool rv = memory.Create(test_name, false, true, kDataSize); + bool rv = memory.Create(test_name_, false, true, kDataSize); EXPECT_TRUE(rv); rv = memory.Map(kDataSize); EXPECT_TRUE(rv); @@ -45,9 +51,16 @@ class MultipleThreadMain : public PlatformThread::Delegate { private: int16 id_; + static const std::wstring test_name_; + DISALLOW_COPY_AND_ASSIGN(MultipleThreadMain); }; +const std::wstring MultipleThreadMain::test_name_ = L"SharedMemoryOpenThreadTest"; + +// TODO(port): +// This test requires the ability to pass file descriptors between processes. +// We haven't done that yet in Chrome for POSIX. #if defined(OS_WIN) // Each thread will open the shared memory. Each thread will take the memory, // and keep changing it while trying to lock it, with some small pauses in @@ -104,7 +117,11 @@ TEST(SharedMemoryTest, OpenClose) { // Open two handles to a memory segment, confirm that they are mapped // separately yet point to the same space. SharedMemory memory1; - bool rv = memory1.Open(test_name, false); + bool rv = memory1.Delete(test_name); + EXPECT_TRUE(rv); + rv = memory1.Delete(test_name); + EXPECT_TRUE(rv); + rv = memory1.Open(test_name, false); EXPECT_FALSE(rv); rv = memory1.Create(test_name, false, false, kDataSize); EXPECT_TRUE(rv); @@ -134,12 +151,17 @@ TEST(SharedMemoryTest, OpenClose) { // Close the second memory segment. memory2.Close(); + + rv = memory1.Delete(test_name); + EXPECT_TRUE(rv); + rv = memory2.Delete(test_name); + EXPECT_TRUE(rv); } -#if defined(OS_WIN) // Create a set of 5 threads to each open a shared memory segment and write to // it. Verify that they are always reading/writing consistent data. TEST(SharedMemoryTest, MultipleThreads) { + MultipleThreadMain::CleanUp(); PlatformThreadHandle thread_handles[kNumThreads]; MultipleThreadMain* thread_delegates[kNumThreads]; @@ -156,8 +178,15 @@ TEST(SharedMemoryTest, MultipleThreads) { PlatformThread::Join(thread_handles[index]); delete thread_delegates[index]; } + + MultipleThreadMain::CleanUp(); } +// TODO(port): this test requires the MultipleLockThread class +// (defined above), which requires the ability to pass file +// descriptors between processes. We haven't done that yet in Chrome +// for POSIX. +#if defined(OS_WIN) // Create a set of threads to each open a shared memory segment and write to it // with the lock held. Verify that they are always reading/writing consistent // data. @@ -181,4 +210,51 @@ TEST(SharedMemoryTest, Lock) { } #endif +// Allocate private (unique) shared memory with an empty string for a +// name. Make sure several of them don't point to the same thing as +// we might expect if the names are equal. +TEST(SharedMemoryTest, AnonymousPrivate) { + int i, j; + int count = 4; + bool rv; + const int kDataSize = 8192; + + SharedMemory* memories = new SharedMemory[count]; + int **pointers = new int*[count]; + ASSERT_TRUE(memories); + ASSERT_TRUE(pointers); + + for (i = 0; i < count; i++) { + rv = memories[i].Create(L"", false, true, kDataSize); + EXPECT_TRUE(rv); + rv = memories[i].Map(kDataSize); + EXPECT_TRUE(rv); + int *ptr = static_cast<int*>(memories[i].memory()); + EXPECT_TRUE(ptr); + pointers[i] = ptr; + } + + for (i = 0; i < count; i++) { + // zero out the first int in each except for i; for that one, make it 100. + for (j = 0; j < count; j++) { + if (i == j) + pointers[j][0] = 100; + else + pointers[j][0] = 0; + } + // make sure there is no bleeding of the 100 into the other pointers + for (j = 0; j < count; j++) { + if (i == j) + EXPECT_EQ(100, pointers[j][0]); + else + EXPECT_EQ(0, pointers[j][0]); + } + } + + for (int i = 0; i < count; i++) { + memories[i].Close(); + } +} + + } // namespace base diff --git a/base/shared_memory_win.cc b/base/shared_memory_win.cc index 4c56a70..4ae4752 100644 --- a/base/shared_memory_win.cc +++ b/base/shared_memory_win.cc @@ -66,6 +66,11 @@ bool SharedMemory::Create(const std::wstring &name, bool read_only, return true; } +bool SharedMemory::Delete(const std::wstring& name) { + // intentionally empty -- there is nothing for us to do on Windows. + return true; +} + bool SharedMemory::Open(const std::wstring &name, bool read_only) { DCHECK(mapped_file_ == NULL); diff --git a/base/stats_table_unittest.cc b/base/stats_table_unittest.cc index 59b1215..7c4c4f6 100644 --- a/base/stats_table_unittest.cc +++ b/base/stats_table_unittest.cc @@ -5,6 +5,7 @@ #include "base/multiprocess_test.h" #include "base/platform_thread.h" #include "base/simple_thread.h" +#include "base/shared_memory.h" #include "base/stats_table.h" #include "base/stats_counters.h" #include "base/string_util.h" @@ -19,6 +20,11 @@ namespace base { class StatsTableTest : public MultiProcessTest { + public: + void DeleteShmem(std::string name) { + base::SharedMemory mem; + mem.Delete(UTF8ToWide(name)); + } }; // Open a StatsTable and verify that we can write to each of the @@ -27,6 +33,7 @@ TEST_F(StatsTableTest, VerifySlots) { const std::string kTableName = "VerifySlotsStatTable"; const int kMaxThreads = 1; const int kMaxCounter = 5; + DeleteShmem(kTableName); StatsTable table(kTableName, kMaxThreads, kMaxCounter); // Register a single thread. @@ -50,6 +57,8 @@ TEST_F(StatsTableTest, VerifySlots) { // Try to allocate an additional counter. Verify it fails. int counter_id = table.FindCounter(counter_base_name); EXPECT_EQ(counter_id, 0); + + DeleteShmem(kTableName); } // CounterZero will continually be set to 0. @@ -104,6 +113,7 @@ TEST_F(StatsTableTest, MultipleThreads) { const std::string kTableName = "MultipleThreadStatTable"; const int kMaxThreads = 20; const int kMaxCounter = 5; + DeleteShmem(kTableName); StatsTable table(kTableName, kMaxThreads, kMaxCounter); StatsTable::set_current(&table); @@ -149,16 +159,18 @@ TEST_F(StatsTableTest, MultipleThreads) { EXPECT_EQ((kMaxThreads % 2) * kThreadLoops, table.GetCounterValue(name)); EXPECT_EQ(0, table.CountThreadsRegistered()); + + DeleteShmem(kTableName); } -const std::string kTableName = "MultipleProcessStatTable"; +const std::string kMPTableName = "MultipleProcessStatTable"; MULTIPROCESS_TEST_MAIN(StatsTableMultipleProcessMain) { // Each process will open the shared memory and set counters // concurrently in a loop. We'll use some pauses to // mixup the scheduling. - StatsTable table(kTableName, 0, 0); + StatsTable table(kMPTableName, 0, 0); StatsTable::set_current(&table); StatsCounter zero_counter(kCounterZero); StatsCounter lucky13_counter(kCounter1313); @@ -177,12 +189,11 @@ MULTIPROCESS_TEST_MAIN(StatsTableMultipleProcessMain) { // Create a few processes and have them poke on their counters. TEST_F(StatsTableTest, MultipleProcesses) { // Create a stats table. - const std::string kTableName = "MultipleProcessStatTable"; const int kMaxProcs = 20; const int kMaxCounter = 5; - StatsTable table(kTableName, kMaxProcs, kMaxCounter); + DeleteShmem(kMPTableName); + StatsTable table(kMPTableName, kMaxProcs, kMaxCounter); StatsTable::set_current(&table); - EXPECT_EQ(0, table.CountThreadsRegistered()); // Spin up a set of processes to go bang on the various counters. @@ -220,6 +231,8 @@ TEST_F(StatsTableTest, MultipleProcesses) { EXPECT_EQ(-kMaxProcs * kThreadLoops, table.GetCounterValue(name)); EXPECT_EQ(0, table.CountThreadsRegistered()); + + DeleteShmem(kMPTableName); } class MockStatsCounter : public StatsCounter { @@ -235,6 +248,7 @@ TEST_F(StatsTableTest, StatsCounter) { const std::string kTableName = "StatTable"; const int kMaxThreads = 20; const int kMaxCounter = 5; + DeleteShmem(kTableName); StatsTable table(kTableName, kMaxThreads, kMaxCounter); StatsTable::set_current(&table); @@ -271,6 +285,8 @@ TEST_F(StatsTableTest, StatsCounter) { EXPECT_EQ(-1, table.GetCounterValue("c:foo")); foo.Decrement(-1); EXPECT_EQ(0, table.GetCounterValue("c:foo")); + + DeleteShmem(kTableName); } class MockStatsCounterTimer : public StatsCounterTimer { @@ -348,6 +364,7 @@ TEST_F(StatsTableTest, StatsScope) { const std::string kTableName = "StatTable"; const int kMaxThreads = 20; const int kMaxCounter = 5; + DeleteShmem(kTableName); StatsTable table(kTableName, kMaxThreads, kMaxCounter); StatsTable::set_current(&table); @@ -378,6 +395,8 @@ TEST_F(StatsTableTest, StatsScope) { EXPECT_LE(1000, table.GetCounterValue("t:foo")); EXPECT_LE(1000, table.GetCounterValue("t:bar")); EXPECT_EQ(2, table.GetCounterValue("c:bar")); + + DeleteShmem(kTableName); } } // namespace base |