Properly honor base::SharedMemory semantics for name="" to mean
new/private shared memory on POSIX. Transition base::SharedMemory
implementation to file/mmap() to prevent leaking of wired kernel
resources and allow easier cleanup. Enable one more shared_memory
unit test for POSIX. Enable stats_table_unittest.cc for Mac, and
modify it so it cleans up.
Review URL: http://codereview.chromium.org/19724
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9114 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 9e51af9018550b6b23802f66469310f5d1790ab9
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 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 @@
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_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 @@
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 @@
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 @@
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 @@
#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 @@
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 @@
#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/shared_memory.h b/base/shared_memory.h
index ea60fa63..d96299a 100644
--- a/base/shared_memory.h
+++ b/base/shared_memory.h
@@ -24,6 +24,10 @@
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 @@
// 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 @@
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 @@
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,44 +63,157 @@
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();
- return false;
- }
-
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);
+ }
+
+ // Doesn't exist, so success.
+ return true;
+}
+
bool SharedMemory::Open(const std::wstring &name, bool read_only) {
read_only_ = 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) {
- DCHECK(mapped_file_ == -1);
+// 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;
- name_ = L"/" + name;
+ // 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);
- 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)
+ FilePath temp_dir;
+ if (file_util::GetShmemTempDir(&temp_dir) == false)
return false;
- posix_name += kSemaphoreSuffix;
- lock_ = sem_open(posix_name.c_str(), O_CREAT, posix_mode, 1);
+ 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);
+
+ 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());
+ }
+
+ 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);
+
+ 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 @@
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
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 @@
}
#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/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 @@
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 @@
// 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 @@
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 @@
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 @@
// 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 @@
EXPECT_EQ(-kMaxProcs * kThreadLoops,
table.GetCounterValue(name));
EXPECT_EQ(0, table.CountThreadsRegistered());
+
+ DeleteShmem(kMPTableName);
}
class MockStatsCounter : public StatsCounter {
@@ -235,6 +248,7 @@
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 @@
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 @@
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 @@
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