If I were to change the EDK such that SetParentPipeHandle must never be called *after* mojo::edk::CreateMessagePipe(PlatformHandle) has been called, would that adversely affect any existing ARC code?

I assume SetParentPipeHandle handle is always called first in practice, and I could leverage that to keep the RMPB fix relatively simple.

We always call SetParentPipeHandle before CreateMessagePipe, so it's a fair assumption to make. Even if we figured out we didn't, we control all the call sites, so it would be a straightforward change.

One more question then: if all the call sites are easy enough to change, can we just move over to using CreateParentMessagePipe and CreateChildMessagePipe?

Note that these are EDK node "parent" and "child" relationships, not necessarily OS process relationships. This API only requires some string passing, and it doesn't suffer from the same race condition.

We could avoid adding new complexity by simply deleting RemoteMessagePipeBootstrap. 

But if there's additional trouble I'm overlooking (e.g. synchronizing changes on either side of the container), maybe it still makes sense to fix RMPB.

Yeah, that might be possible too. I vaguely remember I had trouble connecting stuff using that API, but I forgot why. I'll try changing them back, see if anything breaks, and report back.

Changing to Create{Child,Parent}MessagePipe worked as expected. There is just one call site that's a bit problematic due to Chrome/container drift: https://cs.chromium.org/chromium/src/components/arc/arc_session.cc?q=arcsession&sq=package:chromium&l=486

I *might* be able to sneak the token in the initial message where I originally send the pipe https://cs.chromium.org/chromium/src/components/arc/arc_session.cc?q=arcsession&sq=package:chromium&l=451 . Deprecating that one might take a few weeks, but it seems to be doable.

OK. So far my ideas for a RMPB fix all fall short, as they end up requiring an EDK update on both sides of the container. Removing it might be the best option at this point.

No, I was not able to sneak the token :( So here's the plan:

* We stop using CreateMessagePipe in Android as soon as possible.
* Early next week, I'll change the Android-side bootstrap implementation so it can handle either a one-byte message with one file descriptor (which we use to call CreateMessagePipe), or a 32-byte token with no file descriptors.
* A few weeks later (maybe a full release cycle?) I change ArcSession to only send the child token instead of the pipe.
* Some time later, I remove the legacy implementation from the Android side so that only token-passing occurs. This would get rid of all the callers of CreateMessagePipe in our project.

Does that sound okay with you?

That sounds like a great plan!

I already have the code up for review to remove RMPB (mentioned in #c6), so I'll assign this to myself.

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

commit 50abf80ac895397ecf2e35194d2d1b06595a496c
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jan 25 01:04:53 2017

arc: Switch to mojo::edk::CreateParentMessagePipe

This change should get rid of the very last mojo::edk::CreateMessagePipe
in ARC.

BUG= 674724 
TEST=ARC still boots

Review-Url: https://codereview.chromium.org/2583263002
Cr-Commit-Position: refs/heads/master@{#445888}

[modify] https://crrev.com/50abf80ac895397ecf2e35194d2d1b06595a496c/components/arc/arc_session.cc

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

commit 8e3c814a822afe01b3893d48cb0dd7cb4e132b74
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jan 25 19:12:24 2017

[mojo] Delete RemoteMessagePipeBootstrap

RemoteMessagePipeBootstrap is racy since it has no FIFO relationship
with mojo::edk::NodeController. Since there are no more callers of
mojo::edk::CreateMessagePipe(), we can simply remove that class and
always use mojo::edk::Create{Parent,Child}MessagePipe() instead, which
does not suffer from the race.

BUG= 674724 
TEST=git cl try

Review-Url: https://codereview.chromium.org/2596363002
Cr-Commit-Position: refs/heads/master@{#446077}

[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/embedder/embedder.cc
[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/embedder/embedder.h
[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/system/BUILD.gn
[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/system/core.cc
[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/system/core.h
[modify] https://crrev.com/8e3c814a822afe01b3893d48cb0dd7cb4e132b74/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[delete] https://crrev.com/3e45ac858cc5d25a1185bd4ecddc9632d41b390c/mojo/edk/system/remote_message_pipe_bootstrap.cc
[delete] https://crrev.com/3e45ac858cc5d25a1185bd4ecddc9632d41b390c/mojo/edk/system/remote_message_pipe_bootstrap.h