New issue
Advanced search Search tips

Issue 796272 link

Starred by 1 user
Status: Archived
Owner:
derat@chromium.org
Closed: Dec 2017
Cc:
derat@chromium.org
mnissler@chromium.org
emaxx@chromium.org
igorcov@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Better logging for browser job kill from session_manager

Project Member Reported by emaxx@chromium.org, Dec 19 2017 
Currently, there are some cases when session_manager prints the following to the logs:

  INFO session_manager: [INFO:browser_job.cc(164)] Terminating process:

(with nothing after ":")

This makes it harder to investigate bugreports where the reason why the browser was killed is unclear - e.g. issue 777532.

We should investigate whether that logging can be enhanced - ideally, so that the message details are always non-empty.

Comment 1 by derat@chromium.org, Dec 19 2017

Owner: derat@chromium.org
Status: Started (was: Available)
Sure, seems reasonable. From glancing at the code, I'd guess that this is mostly just the normal shutdown path, though.

Comment 2 by derat@chromium.org, Dec 19 2017 

session_manager has a bad case of inheritance-instead-of-composition that makes this a bit painful. :-/

Comment 3 by derat@chromium.org, Dec 19 2017 

https://crrev.com/c/834798 out for review.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 20 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9be847ec2b5d99825a36b096d36d509455aa4ead

commit 9be847ec2b5d99825a36b096d36d509455aa4ead
Author: Daniel Erat <derat@chromium.org>
Date: Wed Dec 20 13:49:35 2017

login: Make session_manager log reasons for killing Chrome.

To aid in debugging, make session_manager always log the
reason why it's killing Chrome. This change also passes
through reasons when we're killing other managed processes
and containers, although they aren't currently logged.

BUG= chromium:796272 
TEST=manual: ran "restart ui" and saw "Terminating process:
     exiting cleanly" in /var/log/messages

Change-Id: Ie4774a4e8841b8cbb613cad3794786548c1b72ef
Reviewed-on: https://chromium-review.googlesource.com/834798
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/key_generator.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/session_manager_impl.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/fake_container_manager.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/fake_termina_manager.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/android_oci_wrapper_unittest.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/termina_manager_impl.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/session_manager_service.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/session_manager_service.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/termina_manager_impl.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/job_manager.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/android_oci_wrapper.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/android_oci_wrapper.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/child_exit_handler_unittest.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/key_generator.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/vpd_process_impl.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/container_manager_impl.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/vpd_process_impl.h
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/container_manager_impl.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/fake_container_manager.cc
[modify] https://crrev.com/9be847ec2b5d99825a36b096d36d509455aa4ead/login_manager/fake_termina_manager.cc

Comment 5 by emaxx@chromium.org, Dec 20 2017 

Thanks for such a quick fix, Dan!

Comment 6 by derat@chromium.org, Dec 20 2017

Status: Fixed (was: Started)

Comment 7 by dchan@google.com, Jul 30

Status: Archived (was: Fixed)
►

Sign in to add a comment