File-descriptors cannot be transferred by Mojo on Fuchsia if their description is shared with another fd |
||||
Issue descriptionjamesr@ found that fd-passing was failing across Mojo in cases in which the passed fd had been dup()ed from another existing fd (which is actually very common due to the way that the prepare-for-transfer code is structured. This fails in the current mojo::edk::Channel implementation for Fuchsia, because the UnwrapPlatformHandle() call takes ownership of the underlying primitive handles using zx_transfer_fd(), which disallows transfers from shared file descriptions, since that would result in the other file descriptors referencing it becoming broken. For our purposes, we can attempt to transfer(), and fall-back to clone()ing to work-around the issue without imposing clone() overhead on every passed file-descriptor.
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2a865c7451560887659cce5b27c8d5f363e0051 commit c2a865c7451560887659cce5b27c8d5f363e0051 Author: James Robinson <jamesr@google.com> Date: Thu Oct 19 00:55:49 2017 [fuchsia][mojo/edk] Clone FD if transfer fails When trying to send a platform handle represented as a file descriptor cross-process on Fuchsia, we attempt to transfer the FD in-place which is more efficient than the usual clone + send + close pattern. In some cases the transfer operation fails, however, so we have to make a fresh clone to transfer. Bug: 776163 Change-Id: I6d25e19ee7fef194261f5d1e624a4d8de291d6b3 Reviewed-on: https://chromium-review.googlesource.com/727121 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#509951} [modify] https://crrev.com/c2a865c7451560887659cce5b27c8d5f363e0051/mojo/edk/system/channel_fuchsia.cc
,
Oct 19 2017
Leaving this bug open for me to fix the UnwrapPlatformHandle() error handling in WriteNoLock().
,
Oct 19 2017
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e905d43e899d51df20ce94df28e9c34ee3b72de5 commit e905d43e899d51df20ce94df28e9c34ee3b72de5 Author: Wez <wez@chromium.org> Date: Thu Oct 19 03:54:44 2017 Revert "[fuchsia][mojo/edk] Clone FD if transfer fails" This reverts commit c2a865c7451560887659cce5b27c8d5f363e0051. Reason for revert: This causes some IPC and Mojo tests to start failing (see crbug.com/776242 ). Original change's description: > [fuchsia][mojo/edk] Clone FD if transfer fails > > When trying to send a platform handle represented as a file descriptor > cross-process on Fuchsia, we attempt to transfer the FD in-place which > is more efficient than the usual clone + send + close pattern. In some > cases the transfer operation fails, however, so we have to make a fresh > clone to transfer. > > Bug: 776163 > Change-Id: I6d25e19ee7fef194261f5d1e624a4d8de291d6b3 > Reviewed-on: https://chromium-review.googlesource.com/727121 > Commit-Queue: Wez <wez@chromium.org> > Reviewed-by: Wez <wez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#509951} TBR=wez@chromium.org,jamesr@chromium.org Change-Id: Icd42bdf3e3d7438b486fd24d4aee5b7d25be32b9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 776163 , 776242 Reviewed-on: https://chromium-review.googlesource.com/727027 Reviewed-by: Wez <wez@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#509992} [modify] https://crrev.com/e905d43e899d51df20ce94df28e9c34ee3b72de5/mojo/edk/system/channel_fuchsia.cc
,
Oct 19 2017
,
Oct 19 2017
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/160e435e49268b4b62441d01a5266951eeb5c9d2 commit 160e435e49268b4b62441d01a5266951eeb5c9d2 Author: Wez <wez@chromium.org> Date: Thu Oct 19 16:53:11 2017 [fuchsia][mojo/edk] Clone FD if transfer fails When trying to send a platform handle represented as a file descriptor cross-process on Fuchsia, we attempt to transfer the FD in-place which is more efficient than the usual clone + send + close pattern. In some cases the transfer operation fails, however, so we have to make a fresh clone to transfer. This is a re-land of jamesr@'s original fix ( https://chromium-review.googlesource.com/c/chromium/src/+/727027) with a correction to the checking of the transfer() & clone() results. Bug: 776163 , 776242 Change-Id: Ie06a8e2e9d914b74fbc0002b32ab4fbfaa936f48 Reviewed-on: https://chromium-review.googlesource.com/727257 Reviewed-by: Scott Graham <scottmg@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#510110} [modify] https://crrev.com/160e435e49268b4b62441d01a5266951eeb5c9d2/mojo/edk/system/channel_fuchsia.cc
,
Oct 19 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jamesr@google.com
, Oct 19 2017