Pass the shared memory handle in to StatsTable rather than consulting the GlobaDescriptors.
Remove the IPC dependency from base and updates the DEPS files to prevent this from happening again (ipc is in the root dir's deps file so is implicitly allowed here).
The Chrome unit tests did a bunch of work to delete the stats file when running under Posix. But this filename is never used on Posix, so I deleted this code. The unit tests now use an anonymous stats table which this code now supports.
The stats table unit test is disabled. It does not work on Posix systems at all because handle sharing is not set up. I put a Windows ifdef around this code (leaving it disabled) so I could use the Windows-style initializers, and verified that the test passes on Windows if I undisable it.
R=jam@chromium.org
Review URL: https://codereview.chromium.org/224713017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262540 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 4e9b3d53c28910dc0d9543a88cffbf0928a9b6dc
diff --git a/base/metrics/stats_table.cc b/base/metrics/stats_table.cc
index f09e736..03158fd 100644
--- a/base/metrics/stats_table.cc
+++ b/base/metrics/stats_table.cc
@@ -14,12 +14,6 @@
#include "base/threading/platform_thread.h"
#include "base/threading/thread_local_storage.h"
-#if defined(OS_POSIX)
-#include "base/posix/global_descriptors.h"
-#include "errno.h"
-#include "ipc/ipc_descriptors.h"
-#endif
-
namespace base {
// The StatsTable uses a shared memory segment that is laid out as follows
@@ -105,7 +99,7 @@
// Construct a new Internal based on expected size parameters, or
// return NULL on failure.
- static Internal* New(const std::string& name,
+ static Internal* New(const StatsTable::TableIdentifier& table,
int size,
int max_threads,
int max_counters);
@@ -151,8 +145,9 @@
}
// Create or open the SharedMemory used by the stats table.
- static SharedMemory* CreateSharedMemory(const std::string& name,
- int size);
+ static SharedMemory* CreateSharedMemory(
+ const StatsTable::TableIdentifier& table,
+ int size);
// Initializes the table on first access. Sets header values
// appropriately and zeroes all counters.
@@ -174,11 +169,12 @@
};
// static
-StatsTable::Internal* StatsTable::Internal::New(const std::string& name,
- int size,
- int max_threads,
- int max_counters) {
- scoped_ptr<SharedMemory> shared_memory(CreateSharedMemory(name, size));
+StatsTable::Internal* StatsTable::Internal::New(
+ const StatsTable::TableIdentifier& table,
+ int size,
+ int max_threads,
+ int max_counters) {
+ scoped_ptr<SharedMemory> shared_memory(CreateSharedMemory(table, size));
if (!shared_memory.get())
return NULL;
if (!shared_memory->Map(size))
@@ -200,16 +196,14 @@
}
// static
-SharedMemory* StatsTable::Internal::CreateSharedMemory(const std::string& name,
- int size) {
+SharedMemory* StatsTable::Internal::CreateSharedMemory(
+ const StatsTable::TableIdentifier& table,
+ int size) {
#if defined(OS_POSIX)
- GlobalDescriptors* global_descriptors = GlobalDescriptors::GetInstance();
- if (global_descriptors->MaybeGet(kStatsTableSharedMemFd) != -1) {
- // Open the shared memory file descriptor passed by the browser process.
- FileDescriptor file_descriptor(
- global_descriptors->Get(kStatsTableSharedMemFd), false);
- return new SharedMemory(file_descriptor, false);
- }
+ // Check for existing table.
+ if (table.fd != -1)
+ return new SharedMemory(table, false);
+
// Otherwise we need to create it.
scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
if (!shared_memory->CreateAnonymous(size))
@@ -217,15 +211,22 @@
return shared_memory.release();
#elif defined(OS_WIN)
scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
- if (!shared_memory->CreateNamedDeprecated(name, true, size))
- return NULL;
+ if (table.empty()) {
+ // Create an anonymous table.
+ if (!shared_memory->CreateAnonymous(size))
+ return NULL;
+ } else {
+ // Create a named table for sharing between processes.
+ if (!shared_memory->CreateNamedDeprecated(table, true, size))
+ return NULL;
+ }
return shared_memory.release();
#endif
}
void StatsTable::Internal::InitializeTable(void* memory, int size,
- int max_counters,
- int max_threads) {
+ int max_counters,
+ int max_threads) {
// Zero everything.
memset(memory, 0, size);
@@ -286,7 +287,8 @@
// We keep a singleton table which can be easily accessed.
StatsTable* global_table = NULL;
-StatsTable::StatsTable(const std::string& name, int max_threads,
+StatsTable::StatsTable(const TableIdentifier& table,
+ int max_threads,
int max_counters)
: internal_(NULL),
tls_index_(SlotReturnFunction) {
@@ -298,7 +300,7 @@
AlignedSize(max_threads * sizeof(int)) +
AlignedSize((sizeof(int) * (max_counters * max_threads)));
- internal_ = Internal::New(name, table_size, max_threads, max_counters);
+ internal_ = Internal::New(table, table_size, max_threads, max_counters);
if (!internal_)
DPLOG(ERROR) << "StatsTable did not initialize";
diff --git a/base/metrics/stats_table.h b/base/metrics/stats_table.h
index 49ba79f..719e630 100644
--- a/base/metrics/stats_table.h
+++ b/base/metrics/stats_table.h
@@ -28,24 +28,45 @@
#include "base/memory/shared_memory.h"
#include "base/synchronization/lock.h"
#include "base/threading/thread_local_storage.h"
+#include "build/build_config.h"
+
+#if defined(OS_POSIX)
+#include "base/file_descriptor_posix.h"
+#endif
namespace base {
class BASE_EXPORT StatsTable {
public:
- // Create a new StatsTable.
- // If a StatsTable already exists with the specified name, this StatsTable
- // will use the same shared memory segment as the original. Otherwise,
- // a new StatsTable is created and all counters are zeroed.
+ // Identifies a StatsTable. We often want to share these between processes.
//
- // name is the name of the StatsTable to use.
+ // On Windows, we use a named shared memory segment so the table identifier
+ // should be a relatively unique string identifying the table to use. An
+ // empty string can be used to use an anonymous shared memory segment for
+ // cases where the table does not need to be shared between processes.
+ //
+ // Posix does not support named memory so we explicitly share file
+ // descriptors. On Posix, pass a default-constructed file descriptor if a
+ // handle doesn't already exist, and a new one will be created.
+ //
+ // If a table doesn't already exist with the given identifier, a new one will
+ // be created with zeroed counters.
+#if defined(OS_POSIX)
+ typedef FileDescriptor TableIdentifier;
+#elif defined(OS_WIN)
+ typedef std::string TableIdentifier;
+#endif
+
+ // Create a new StatsTable.
//
// max_threads is the maximum number of threads the table will support.
// If the StatsTable already exists, this number is ignored.
//
// max_counters is the maximum number of counters the table will support.
// If the StatsTable already exists, this number is ignored.
- StatsTable(const std::string& name, int max_threads, int max_counters);
+ StatsTable(const TableIdentifier& table,
+ int max_threads,
+ int max_counters);
// Destroys the StatsTable. When the last StatsTable is destroyed
// (across all processes), the StatsTable is removed from disk.
diff --git a/base/metrics/stats_table_unittest.cc b/base/metrics/stats_table_unittest.cc
index 3fcb099..53a4731 100644
--- a/base/metrics/stats_table_unittest.cc
+++ b/base/metrics/stats_table_unittest.cc
@@ -18,21 +18,14 @@
namespace base {
class StatsTableTest : public MultiProcessTest {
- public:
- void DeleteShmem(const std::string& name) {
- SharedMemory mem;
- mem.Delete(name);
- }
};
// Open a StatsTable and verify that we can write to each of the
// locations in the table.
TEST_F(StatsTableTest, VerifySlots) {
- const std::string kTableName = "VerifySlotsStatTable";
const int kMaxThreads = 1;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
// Register a single thread.
std::string thread_name = "mainThread";
@@ -55,8 +48,6 @@
// 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.
@@ -117,11 +108,9 @@
#endif
TEST_F(StatsTableTest, MAYBE_MultipleThreads) {
// Create a stats table.
- const std::string kTableName = "MultipleThreadStatTable";
const int kMaxThreads = 20;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
StatsTable::set_current(&table);
EXPECT_EQ(0, table.CountThreadsRegistered());
@@ -166,10 +155,11 @@
EXPECT_EQ((kMaxThreads % 2) * kThreadLoops,
table.GetCounterValue(name));
EXPECT_EQ(0, table.CountThreadsRegistered());
-
- DeleteShmem(kTableName);
}
+// This multiprocess test only runs on Windows. On Posix, the shared memory
+// handle is not sent between the processes properly.
+#if defined(OS_WIN)
const std::string kMPTableName = "MultipleProcessStatTable";
MULTIPROCESS_TEST_MAIN(StatsTableMultipleProcessMain) {
@@ -199,7 +189,6 @@
// Create a stats table.
const int kMaxProcs = 20;
const int kMaxCounter = 5;
- DeleteShmem(kMPTableName);
StatsTable table(kMPTableName, kMaxProcs, kMaxCounter);
StatsTable::set_current(&table);
EXPECT_EQ(0, table.CountThreadsRegistered());
@@ -241,9 +230,8 @@
EXPECT_EQ(-kMaxProcs * kThreadLoops,
table.GetCounterValue(name));
EXPECT_EQ(0, table.CountThreadsRegistered());
-
- DeleteShmem(kMPTableName);
}
+#endif
class MockStatsCounter : public StatsCounter {
public:
@@ -255,11 +243,9 @@
// Test some basic StatsCounter operations
TEST_F(StatsTableTest, StatsCounter) {
// Create a stats table.
- const std::string kTableName = "StatTable";
const int kMaxThreads = 20;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
StatsTable::set_current(&table);
MockStatsCounter foo("foo");
@@ -295,8 +281,6 @@
EXPECT_EQ(-1, table.GetCounterValue("c:foo"));
foo.Subtract(-1);
EXPECT_EQ(0, table.GetCounterValue("c:foo"));
-
- DeleteShmem(kTableName);
}
class MockStatsCounterTimer : public StatsCounterTimer {
@@ -311,11 +295,9 @@
// Test some basic StatsCounterTimer operations
TEST_F(StatsTableTest, StatsCounterTimer) {
// Create a stats table.
- const std::string kTableName = "StatTable";
const int kMaxThreads = 20;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
StatsTable::set_current(&table);
MockStatsCounterTimer bar("bar");
@@ -340,17 +322,14 @@
bar.Stop();
EXPECT_GT(table.GetCounterValue("t:bar"), 0);
EXPECT_LE(kDuration.InMilliseconds() * 2, table.GetCounterValue("t:bar"));
- DeleteShmem(kTableName);
}
// Test some basic StatsRate operations
TEST_F(StatsTableTest, StatsRate) {
// Create a stats table.
- const std::string kTableName = "StatTable";
const int kMaxThreads = 20;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
StatsTable::set_current(&table);
StatsRate baz("baz");
@@ -375,17 +354,14 @@
baz.Stop();
EXPECT_EQ(2, table.GetCounterValue("c:baz"));
EXPECT_LE(kDuration.InMilliseconds() * 2, table.GetCounterValue("t:baz"));
- DeleteShmem(kTableName);
}
// Test some basic StatsScope operations
TEST_F(StatsTableTest, StatsScope) {
// Create a stats table.
- const std::string kTableName = "StatTable";
const int kMaxThreads = 20;
const int kMaxCounter = 5;
- DeleteShmem(kTableName);
- StatsTable table(kTableName, kMaxThreads, kMaxCounter);
+ StatsTable table(StatsTable::TableIdentifier(), kMaxThreads, kMaxCounter);
StatsTable::set_current(&table);
StatsCounterTimer foo("foo");
@@ -417,8 +393,6 @@
EXPECT_LE(kDuration.InMilliseconds() * 2, table.GetCounterValue("t:foo"));
EXPECT_LE(kDuration.InMilliseconds() * 2, table.GetCounterValue("t:bar"));
EXPECT_EQ(2, table.GetCounterValue("c:bar"));
-
- DeleteShmem(kTableName);
}
} // namespace base