New issue
Advanced search Search tips

Issue 842037 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Clarify guarantees around return values from mojo::WrapPlatformX

Project Member Reported by dcheng@chromium.org, May 11 2018

Issue description

The guarantees aren't as clear as they could be. Ideally, we would guarantee that these functions would never fail (i.e. if we pass in a valid handle, we guarantee a valid handle is returned; if we pass in an invalid handle, we guaranteed an invalid handle is returned).

Currently, there's some uncertainty around it which leads to some code checking for failures "just in case":
https://chromium-review.googlesource.com/c/chromium/src/+/1041661/6/services/audio/loopback_stream.cc#83

And other code that just DCHECKs:
https://cs.chromium.org/chromium/src/services/audio/input_stream.cc?rcl=4a4f739bcda82268926827d28eb058e2f716537e&l=156
 

Comment 1 by roc...@chromium.org, May 14 2018

I'm going to be touching these APIs in an upcoming CL, so I'll keep this in mind when I do that.

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

Cc: -roc...@chromium.org
Owner: roc...@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by roc...@chromium.org, May 15 2018

If we're OK with assuming that we'll never run out of handles -- which seems roughly equivalent to never running out of memory -- then we can effectively guarantee that these won't fail.

Comment 4 by dcheng@chromium.org, May 15 2018

System handles or Mojo handles? I assume the latter?

Comment 5 by roc...@chromium.org, May 15 2018

The latter, whose limit is 4M.
Project Member

Comment 6 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88be045ef0409b129a50955fb89d69c616e31ca2

commit 88be045ef0409b129a50955fb89d69c616e31ca2
Author: Ken Rockot <rockot@chromium.org>
Date: Wed May 16 17:31:04 2018

Update Mojo platform handle wrapping APIs

Updates platform handle wrapping/unwrapping API signatures to be more
extensible for a stable ABI, and adds support to shared memory region
wrapping/unwrapping for writable regions, which may be represented by
multiple platform handles on some platforms.

TBR=bajones@chromium.org

Bug:  826213 ,842037
Change-Id: Ia0e5da7b7e0f4c41853eb3a8d4a12da47071dd8c
Reviewed-on: https://chromium-review.googlesource.com/1058440
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559165}
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/device/vr/oculus/oculus_render_loop.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/device/vr/openvr/openvr_render_loop.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/ipc/ipc_message_attachment.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/embedder/entrypoints.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/core.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/core.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/message_pipe_unittest.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/platform_wrapper_unittest.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/edk/system/shared_buffer_dispatcher.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/README.md
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/platform_handle.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/thunks.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/c/system/thunks.h
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/base/shared_memory_mojom_traits.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/system/platform_handle.cc
[modify] https://crrev.com/88be045ef0409b129a50955fb89d69c616e31ca2/mojo/public/cpp/system/platform_handle.h

Owner: rockot@google.com

Sign in to add a comment