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

Issue 697146 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

chrome --mash needs to shut down cleanly on SIGTERM

Project Member Reported by jamescook@chromium.org, Feb 28 2017

Issue description

Repro:
* Run chrome --mash on device
* stop ui
* 20 second hang, then chrome shuts down

/var/log/messages shows:

2017-02-28T11:03:02.994770-08:00 WARNING kernel: [  571.986834] init: debugd main process (5970) killed by TERM signal
2017-02-28T11:03:03.013348-08:00 INFO session_manager[5885]: [INFO:session_manager_impl.cc(284)] emitting D-Bus signal SessionStateChanged:stopping
2017-02-28T11:03:03.013479-08:00 INFO session_manager[5885]: [INFO:browser_job.cc(157)] Terminating process: 
2017-02-28T11:03:03.013501-08:00 INFO session_manager[5885]: [INFO:system_utils_impl.cc(110)] Sending 15 to 5911 as 1000
<20 second timeout>
2017-02-28T11:03:23.035943-08:00 WARNING kernel: [  592.021008] init: ui main process (5885) killed by KILL signal
2017-02-28T11:03:23.485378-08:00 CRIT ui-post-stop-unkillable[7582]:  6016 Zl   [chrome] <defunct>

chrome without --mash responds to SIGTERM with a clean shutdown:

https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main_posix.cc?type=cs&l=305
https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main_posix.cc?type=cs&q=ExitCleanly&l=156

However, chrome --mash doesn't seem to have a signal handler for SIGTERM. It probably needs one.

This may be the source of shutdown crashes like issue 692227 mash: Zygote crash/CHECK during browser shutdown in desktopui_MashLogin and  issue 696696 	desktopui_MashLogin | FAIL: Autotest client terminated unexpectedly: DUT rebooted during the test run.

 
session_manager on Chrome OS is responsible for sending the SIGTERM for normal shutdown, btw.

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 2 2017

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

commit f154aac6ce85da803ff8263ab287564c619359be
Author: jamescook <jamescook@chromium.org>
Date: Thu Mar 02 03:30:03 2017

mash: Cleanly exit on SIGTERM, SIGINT, SIGHUP

This is needed for the Chrome OS session_manager to cleanly shutdown chrome
on device shutdown and in tests. It's also useful on Linux desktop to cleanly
exit chrome in response to control-C.

* Refactor ShutdownDetector and signal handlers out of
ChromeBrowserMainPartsPosix. Make it run a shutdown callback.
* Use shutdown callback to exit the chrome --mash root process run loop.

BUG= 697146 ,692227, 696696 
TEST=stop ui closes chrome quickly, session_manager doesn't send signal 9
anymore, control-C on Linux desktop cleanly exists chrome

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

[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/BUILD.gn
[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/mash/BUILD.gn
[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/mash/DEPS
[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/mash/mash_runner.cc
[add] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/shutdown_signal_handlers_posix.cc
[add] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/app/shutdown_signal_handlers_posix.h
[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/browser/BUILD.gn
[modify] https://crrev.com/f154aac6ce85da803ff8263ab287564c619359be/chrome/browser/chrome_browser_main_posix.cc

Status: Fixed (was: Started)

Comment 4 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 5 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 6 by laforge@google.com, Nov 1 2017

Components: -Internals>ServiceManager Internals>Services>ServiceManager

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

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

Sign in to add a comment