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

Issue 597385 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Shell's instance mapping is too simplistic

Project Member Reported by sky@chromium.org, Mar 23 2016

Issue description

For each unique app started by the shell an Instance is maintained. Instances form a tree. For example. If A connects to B and B was not running then B is added as a child of A. When an Instance is destroyed (say a connection error results), then all descendant apps (Instances) are destroyed. Continuing with the example above if there is a connection error to A then B is destroyed as well.

This model proves problematic is some scenarios. Consider if another app is started, C, from the root. Then we end up with a graph like this:

   root
   /  \
  A    C
  |
  B

If C attempts to connect to B then the shell does not create a new app (Instance) and C gets a handle to B directly. If A goes away, then B is implicitly destroyed, and C gets a connection error to B.

The mojo world allows any app to connect to another app, so I'm not sure if having the first app that establishes a connection imply ownership.

The specific case biting me is tests. I have the browser_test launcher connect to mojo:mash_session. Each test runs in its own process (each test is handed a message pipe to the shell). The problem is the launch ordering is non-deterministic. This means who ends up starting mus is random. It may be the first test, or it may be the browser test launcher. If the first test triggers creation of the Instance for mus, then mus is owned by the first test and destroyed when the first test completes. This means any other tests that happen to be running are hosed as they don't deal well with mus going away.

Having the test harness connect directly to mus may help things, but it's still non-deterministic without an ack that mus has actually started.

mus has the all_users capability, so we could special case these. By which I mean make such apps never be added to another Instance. This would fix the specific case I'm hitting.

I'm not sure that is enough though. Seems like we may want ref counting. In the example above, if both A and C connect to B then B should only be destroyed when both A and C are destroyed. Of course I suspect it would be easy to induce cycles with ref counting of connections, so I'm not sure that is good either...
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 23 2016

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

commit 8526ab53583e2cc6390fb4693132b151d2481ea1
Author: sky <sky@chromium.org>
Date: Wed Mar 23 22:31:15 2016

Changes lifetime of apps with the "all_users" capability

This changes makes it so "all_users" apps are owned by the shell.

BUG=597385
TEST=none
R=ben@chromium.org

Review URL: https://codereview.chromium.org/1831623002

Cr-Commit-Position: refs/heads/master@{#382951}

[modify] https://crrev.com/8526ab53583e2cc6390fb4693132b151d2481ea1/mojo/shell/shell.cc

Sign in to add a comment