New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796771 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

Mojo shared memory wrapping and unwrapping operations are not great

Project Member Reported by roc...@chromium.org, Dec 21 2017

Issue description

On POSIX, shared memory is backed by 1 or 2 FDs, with the second being present if read-only sharing is supported. Wrap and unwrap APIs to go between SharedMemoryHandle and Mojo shared buffer handles are insufficient since they can wrap/unwrap at most one FD.

A Mojo shared buffer is more appropriately modeled by a base::SharedMemory than by a base::SharedMemoryHandle, and we should probably do the following:

 a) update the C APIs to support wrapping and unwrapping a separate
    read-only platform handle in addition to the regular one. Both may be
    used on supported POSIX systems, and one or the other may be used on
    other platforms.
    
 b) update the C++ APIs to wrap and unwrap base::SharedMemory instead of
    base::SharedMemoryHandle, doing the right thing for e.g. read-only
    memory objects, objects which have both kinds of handles, etc.

This is a bit labor intensive but I think may be necessary to fully address the rampant misuse of shared buffer APIs today. Many bits of code which nominally appeared to share read-only handles were not in
fact doing so, and converting them to do so is non-trivial due to a
loss of information when wrapping or unwrapping handles.

In the limit, it would be nice to eventually fix the base::SharedMemory so that it looks more like the Mojo shared buffer API. Namely:

 a) make it move-only
 b) make it clonable as read-write or read-only
 c) introduce a mapping scoper like mojo::ScopedSharedBufferMapping

Then we could even update Mojo bindings to use base::SharedMemory in place of mojom's handle<shared_buffer>, and use of Mojo shared buffer handles would be reserved for lower-level code not using bindings.

 

Comment 1 by roc...@chromium.org, Dec 21 2017

Components: Internals>Mojo

Comment 2 by roc...@chromium.org, May 17 2018

Status: Fixed (was: Started)
Calling this fixed because we've moved all Mojo stuff to the new shared memory APIs.

Sign in to add a comment