New issue
Advanced search Search tips

Issue 772098 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

mash: Bring down all processes when ash crashes

Project Member Reported by jamescook@chromium.org, Oct 5 2017

Issue description

Forked from  issue 771801 

* Run chrome --mash
* Find the ash process (it will be --type=utility, but you'll need to look at log spam to find the pid)
* kill -SIGSEGV <pid>

Everything should quit, but it doesn't.

Note that testing Ctrl-C in the main terminal is not sufficient. That will kill the browser, which will bring down ash and appear to work.

This probably broke with sky's recent change to move service manager back into browser.

 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 13 2017

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

commit ed46d84d726dac5c1bf3f5998384c6c175fa1231
Author: James Cook <jamescook@chromium.org>
Date: Fri Oct 13 19:41:17 2017

cros/mash: Terminate the browser process if the ash process crashes

The window manager process (ash) caches some state about the browser
process, and the browser caches some state about ash. We eventually
want to make these processes restartable, but for now we need to
terminate the browser if ash crashes. When the browser terminates on
Chrome OS the OS-level session manager daemon will restart it.

This broke when we moved the mojo service manager back into the
browser process.

Also clean up the old ShouldTerminateServiceManagerOnInstanceQuit
support we were using for the standalone and background run modes
for service manager, since we're not using those modes right now.

Ctrl-Alt-Shift-K to crash ash, see the browser exit too

Bug:  678683 ,  772098 
Test: unit_tests, run chrome --mash --ash-debug-shortcuts and press
Change-Id: Icc62d46b36039c12a72ba3281babd234553a3f79
Reviewed-on: https://chromium-review.googlesource.com/716930
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508782}
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/app/chrome_main_delegate.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/browser/mash_service_registry.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/chrome/browser/mash_service_registry.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/app/content_service_manager_main_delegate.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/app/content_service_manager_main_delegate.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/public/app/content_main_delegate.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/public/app/content_main_delegate.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/content/public/browser/content_browser_client.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/background/background_service_manager.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/background/background_service_manager.h
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/background/tests/background_service_manager_unittest.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/embedder/main.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/embedder/main_delegate.cc
[modify] https://crrev.com/ed46d84d726dac5c1bf3f5998384c6c175fa1231/services/service_manager/embedder/main_delegate.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 17 2017

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

commit 5b2b39851880d6726513c841ce38fabcc2650567
Author: James Cook <jamescook@chromium.org>
Date: Tue Oct 17 03:04:21 2017

Add ServiceManagerContextTest.TerminateOnServiceQuit

For mash, certain services are considered critical (window
server, window manager). The browser is terminated if one
of those services goes down.

* Add support for mojo packaged service manifest overlays
  to content_shell
* Add the test "echo" service to content_shell so it can
  be used in content_browsertests
* Test that terminating the echo service can trigger
  browser termination.

Bug:  772098 
Test: content_browsertests
Change-Id: Ie4c8af6956b8800806f9dbd88812af008b6541fb
Reviewed-on: https://chromium-review.googlesource.com/720157
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509252}
[add] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/browser/service_manager/service_manager_context_browsertest.cc
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/BUILD.gn
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/DEPS
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/browser/OWNERS
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/browser/content_shell_browser_manifest_overlay.json
[add] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/browser/content_shell_packaged_services_manifest_overlay.json
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/browser/shell_content_browser_client.cc
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/browser/shell_content_browser_client.h
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/shell/shell_resources.grd
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/content/test/BUILD.gn
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/services/test/echo/echo_service.cc
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/services/test/echo/echo_service.h
[modify] https://crrev.com/5b2b39851880d6726513c841ce38fabcc2650567/services/test/echo/public/interfaces/echo.mojom

Status: Fixed (was: Started)

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment