Minimize chance of lock contention on MojoUnmapBuffer

Change-Id: I7f15880761ab173ecebac694d042b0048707acae
Reviewed-on: https://chromium-review.googlesource.com/850836Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Darin Fisher <darin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527115}

mojo/edk/system/core.cc

@@ -1005,8 +1005,14 @@ MojoResult Core::MapBuffer(MojoHandle buffer_handle,
 
 MojoResult Core::UnmapBuffer(void* buffer) {
   RequestContext request_context;
-  base::AutoLock lock(mapping_table_lock_);
-  return mapping_table_.RemoveMapping(buffer);
+  std::unique_ptr<PlatformSharedBufferMapping> mapping;
+  MojoResult result;
+  // Destroy |mapping| while not holding the lock.
+  {
+    base::AutoLock lock(mapping_table_lock_);
+    result = mapping_table_.RemoveMapping(buffer, &mapping);
+  }
+  return result;
 }
 
 MojoResult Core::WrapPlatformHandle(const MojoPlatformHandle* platform_handle,

mojo/edk/system/mapping_table.cc

@@ -34,13 +34,14 @@ MojoResult MappingTable::AddMapping(
   return MOJO_RESULT_OK;
 }
 
-MojoResult MappingTable::RemoveMapping(void* address) {
+MojoResult MappingTable::RemoveMapping(
+    void* address,
+    std::unique_ptr<PlatformSharedBufferMapping>* mapping) {
   AddressToMappingMap::iterator it = address_to_mapping_map_.find(address);
   if (it == address_to_mapping_map_.end())
     return MOJO_RESULT_INVALID_ARGUMENT;
-  PlatformSharedBufferMapping* mapping_to_delete = it->second;
+  mapping->reset(it->second);
   address_to_mapping_map_.erase(it);
-  delete mapping_to_delete;
   return MOJO_RESULT_OK;
 }
 

mojo/edk/system/mapping_table.h

@@ -39,7 +39,9 @@ class MOJO_SYSTEM_IMPL_EXPORT MappingTable {
   // Tries to add a mapping. (Takes ownership of the mapping in all cases; on
   // failure, it will be destroyed.)
   MojoResult AddMapping(std::unique_ptr<PlatformSharedBufferMapping> mapping);
-  MojoResult RemoveMapping(void* address);
+  MojoResult RemoveMapping(
+      void* address,
+      std::unique_ptr<PlatformSharedBufferMapping>* mapping);
 
  private:
   friend bool internal::ShutdownCheckNoLeaks(Core*);

mojo/public/c/system/buffer.h

@@ -148,8 +148,8 @@ MOJO_SYSTEM_EXPORT MojoResult MojoDuplicateBufferHandle(
 // |*buffer| points to memory with the requested part of the buffer. On
 // failure |*buffer| it is not modified.
 //
-// A single buffer handle may have multiple active mappings The permissions
-// (e.g., writable or executable) of the returned memory depend on th
+// A single buffer handle may have multiple active mappings. The permissions
+// (e.g., writable or executable) of the returned memory depend on the
 // properties of the buffer and properties attached to the buffer handle, as
 // well as |flags|.
 //