Mojo: Implement MojoUnmapBuffer().
Still to do: more tests.
[email protected]
Review URL: https://codereview.chromium.org/220113003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260832 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
index 4c24e47d..ee5ec8b 100644
--- a/mojo/mojo.gyp
+++ b/mojo/mojo.gyp
@@ -132,6 +132,8 @@
'system/local_data_pipe.h',
'system/local_message_pipe_endpoint.cc',
'system/local_message_pipe_endpoint.h',
+ 'system/mapping_table.cc',
+ 'system/mapping_table.h',
'system/memory.cc',
'system/memory.h',
'system/message_in_transit.cc',
diff --git a/mojo/public/c/system/tests/core_unittest.cc b/mojo/public/c/system/tests/core_unittest.cc
index c9acd00..bb6c458 100644
--- a/mojo/public/c/system/tests/core_unittest.cc
+++ b/mojo/public/c/system/tests/core_unittest.cc
@@ -251,8 +251,7 @@
static_cast<char*>(pointer)[51] = 'y';
// Unmap it.
- // TODO(vtl): Not yet implemented.
- EXPECT_EQ(MOJO_RESULT_UNIMPLEMENTED, MojoUnmapBuffer(pointer));
+ EXPECT_EQ(MOJO_RESULT_OK, MojoUnmapBuffer(pointer));
// Map half of |h1|.
pointer = NULL;
@@ -265,8 +264,7 @@
EXPECT_EQ('y', static_cast<char*>(pointer)[1]);
// Unmap it.
- // TODO(vtl): Not yet implemented.
- EXPECT_EQ(MOJO_RESULT_UNIMPLEMENTED, MojoUnmapBuffer(pointer));
+ EXPECT_EQ(MOJO_RESULT_OK, MojoUnmapBuffer(pointer));
EXPECT_EQ(MOJO_RESULT_OK, MojoClose(h1));
}
diff --git a/mojo/system/constants.h b/mojo/system/constants.h
index 6c287b53..b92eef10 100644
--- a/mojo/system/constants.h
+++ b/mojo/system/constants.h
@@ -10,11 +10,14 @@
namespace mojo {
namespace system {
-// Maximum of open (Mojo) handles.
+// Maximum number of open (Mojo) handles.
// TODO(vtl): This doesn't count "live" handles, some of which may live in
// messages.
const size_t kMaxHandleTableSize = 1000000;
+// Maximum number of active memory mappings.
+const size_t kMaxMappingTableSize = 1000000;
+
const size_t kMaxWaitManyNumHandles = kMaxHandleTableSize;
const size_t kMaxMessageNumBytes = 4 * 1024 * 1024;
diff --git a/mojo/system/core_impl.cc b/mojo/system/core_impl.cc
index f6f28086..12c1be57 100644
--- a/mojo/system/core_impl.cc
+++ b/mojo/system/core_impl.cc
@@ -59,7 +59,7 @@
// (which subclasses can use to protect their data).
//
// The lock ordering is as follows:
-// 1. global handle table lock
+// 1. global handle table lock, global mapping table lock
// 2. |Dispatcher| locks
// 3. secondary object locks
// ...
@@ -503,19 +503,21 @@
return result;
DCHECK(mapping);
- *buffer = mapping->base();
+ void* address = mapping->base();
+ {
+ base::AutoLock locker(mapping_table_lock_);
+ result = mapping_table_.AddMapping(mapping.Pass());
+ }
+ if (result != MOJO_RESULT_OK)
+ return result;
- // TODO(vtl): FIXME -- Record the mapping somewhere, so that it can be
- // unmapped properly. For now, just leak it.
- ignore_result(mapping.release());
-
+ *buffer = address;
return MOJO_RESULT_OK;
}
MojoResult CoreImpl::UnmapBuffer(void* buffer) {
- // TODO(vtl): FIXME
- NOTIMPLEMENTED();
- return MOJO_RESULT_UNIMPLEMENTED;
+ base::AutoLock locker(mapping_table_lock_);
+ return mapping_table_.RemoveMapping(buffer);
}
scoped_refptr<Dispatcher> CoreImpl::GetDispatcher(MojoHandle handle) {
diff --git a/mojo/system/core_impl.h b/mojo/system/core_impl.h
index 63b4b08b..e0cbd1e 100644
--- a/mojo/system/core_impl.h
+++ b/mojo/system/core_impl.h
@@ -11,6 +11,7 @@
#include "base/synchronization/lock.h"
#include "mojo/public/system/core_private.h"
#include "mojo/system/handle_table.h"
+#include "mojo/system/mapping_table.h"
#include "mojo/system/system_impl_export.h"
namespace mojo {
@@ -138,6 +139,9 @@
base::Lock handle_table_lock_; // Protects |handle_table_|.
HandleTable handle_table_;
+ base::Lock mapping_table_lock_; // Protects |mapping_table_|.
+ MappingTable mapping_table_;
+
// ---------------------------------------------------------------------------
DISALLOW_COPY_AND_ASSIGN(CoreImpl);
diff --git a/mojo/system/handle_table.cc b/mojo/system/handle_table.cc
index fc03e2e77..798b6103 100644
--- a/mojo/system/handle_table.cc
+++ b/mojo/system/handle_table.cc
@@ -181,9 +181,6 @@
return MOJO_RESULT_OK;
}
-
-//////////////
-
MojoHandle HandleTable::AddDispatcherNoSizeCheck(
const scoped_refptr<Dispatcher>& dispatcher) {
DCHECK(dispatcher);
diff --git a/mojo/system/mapping_table.cc b/mojo/system/mapping_table.cc
new file mode 100644
index 0000000..39b79bcd
--- /dev/null
+++ b/mojo/system/mapping_table.cc
@@ -0,0 +1,48 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/system/mapping_table.h"
+
+#include "base/logging.h"
+#include "mojo/system/constants.h"
+#include "mojo/system/raw_shared_buffer.h"
+
+namespace mojo {
+namespace system {
+
+MappingTable::MappingTable() {
+}
+
+MappingTable::~MappingTable() {
+ // This should usually not be reached (the only instance should be owned by
+ // the singleton |CoreImpl|, which lives forever), except in tests.
+}
+
+MojoResult MappingTable::AddMapping(
+ scoped_ptr<RawSharedBufferMapping> mapping) {
+ DCHECK(mapping);
+
+ if (address_to_mapping_map_.size() >= kMaxMappingTableSize)
+ return MOJO_RESULT_RESOURCE_EXHAUSTED;
+
+ uintptr_t address = reinterpret_cast<uintptr_t>(mapping->base());
+ DCHECK(address_to_mapping_map_.find(address) ==
+ address_to_mapping_map_.end());
+ address_to_mapping_map_[address] = mapping.release();
+ return MOJO_RESULT_OK;
+}
+
+MojoResult MappingTable::RemoveMapping(void* address) {
+ AddressToMappingMap::iterator it =
+ address_to_mapping_map_.find(reinterpret_cast<uintptr_t>(address));
+ if (it == address_to_mapping_map_.end())
+ return MOJO_RESULT_INVALID_ARGUMENT;
+ RawSharedBufferMapping* mapping_to_delete = it->second;
+ address_to_mapping_map_.erase(it);
+ delete mapping_to_delete;
+ return MOJO_RESULT_OK;
+}
+
+} // namespace system
+} // namespace mojo
diff --git a/mojo/system/mapping_table.h b/mojo/system/mapping_table.h
new file mode 100644
index 0000000..bbae097
--- /dev/null
+++ b/mojo/system/mapping_table.h
@@ -0,0 +1,58 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOJO_SYSTEM_MAPPING_TABLE_H_
+#define MOJO_SYSTEM_MAPPING_TABLE_H_
+
+#include <stdint.h>
+
+#include <vector>
+
+#include "base/containers/hash_tables.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "mojo/public/c/system/core.h"
+#include "mojo/system/system_impl_export.h"
+
+namespace mojo {
+namespace system {
+
+class CoreImpl;
+class RawSharedBufferMapping;
+
+// Test-only function (defined/used in embedder/test_embedder.cc). Declared here
+// so it can be friended.
+namespace internal {
+bool ShutdownCheckNoLeaks(CoreImpl*);
+}
+
+// This class provides the (global) table of memory mappings (owned by
+// |CoreImpl|), which maps mapping base addresses to
+// |RawSharedBuffer::Mapping|s.
+//
+// This class is NOT thread-safe; locking is left to |CoreImpl|.
+class MOJO_SYSTEM_IMPL_EXPORT MappingTable {
+ public:
+ MappingTable();
+ ~MappingTable();
+
+ // Tries to add a mapping. (Takes ownership of the mapping in all cases; on
+ // failure, it will be destroyed.)
+ MojoResult AddMapping(scoped_ptr<RawSharedBufferMapping> mapping);
+ MojoResult RemoveMapping(void* address);
+
+ private:
+ friend bool internal::ShutdownCheckNoLeaks(CoreImpl*);
+
+ typedef base::hash_map<uintptr_t, RawSharedBufferMapping*>
+ AddressToMappingMap;
+ AddressToMappingMap address_to_mapping_map_;
+
+ DISALLOW_COPY_AND_ASSIGN(MappingTable);
+};
+
+} // namespace system
+} // namespace mojo
+
+#endif // MOJO_SYSTEM_MAPPING_TABLE_H_
