session_manager leaves signals blocked when spawning Chrome |
||||||||||
Issue descriptionPlatform 9346.0, chrome 59.0.3036.0 While debugging issue 699143 (a problem with sign out on chrome --mash) I noticed that the chrome process starts up with a signal handler mask installed. (See man 2 sigprocmask). In particular the mask was blocking SIGTERM. Child processes inherit this signal mask from their parents. Chrome must be inheriting it from session_manager. It seems like a bug that session_manager launches processes with a signal blocked. This normally doesn't cause problems for chrome because ContentMainRunnerImpl::Initialize() calls a function SetupSignalHandlers() that happens to wipe the mask. However, chrome --mash doesn't use content, so we were not resetting the mask. It's not obvious where the signal mask is coming from. session_manager spawns children in child_job.cc ForkAndExec but there's nothing there about blocking signals: https://cs.corp.google.com/chromeos_public/src/platform2/login_manager/child_job.cc?type=cs
,
Mar 8 2017
Hmm. Those are quite suspiciously the same signals that session_manager_service.cc installs handlers for:
const int kSignals[] = {SIGTERM, SIGINT, SIGHUP};
I think I see the problem. SessionManagerService::SetUpHandlers() calls sigaction() to set SIGUSR1 and SIGALRM to SIG_IGN, but it also uses libbrillo's AsynchronousSignalHandler to install handlers for SIGTERM, SIGINT, and SIGHUP.
ChildJobInterface::Subprocess::ForkAndExec() calls SessionManagerService::RevertHandlers() before execve() in the forked process, but RevertHandlers() is a static method -- it sets SIGUSR1 and SIGALRM back to SIG_DFL, but it doesn't destroy the AsynchronousSignalHandler, so the other three signals still have handlers.
I think that ChildJobInterface either needs to destroy AsynchronousSignalHandler or manually clear the handlers.
,
Mar 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/043b638699e0d9854aa53dfc0eeac1f8b7be16b4 commit 043b638699e0d9854aa53dfc0eeac1f8b7be16b4 Author: jamescook <jamescook@chromium.org> Date: Thu Mar 09 00:11:28 2017 chromeos/mash: Reset signal handler mask on chrome --mash startup This fixes an issue where the system tray "sign out" item does not cleanly sign out of the session. chrome inherits its signal handler masks from the parent process. These masks are not guaranteed to be in any particular state, so they need to be cleared on startup. Otherwise the SIGTERM signal is blocked, so the signal handler doesn't run, so session_manager cannot cleanly shut down chrome. For non-mash chrome this is done as part of content main initialization, but mash chrome doesn't run that code. BUG= 699143 , 699777 TEST=System tray > Sign out takes you back to login screen promptly. Also, "stop ui" on device, look at /var/log/messages, see signal 15 being sent and chrome immediately exiting (and no signal 6 sent to kill it). Review-Url: https://codereview.chromium.org/2731283008 Cr-Commit-Position: refs/heads/master@{#455603} [modify] https://crrev.com/043b638699e0d9854aa53dfc0eeac1f8b7be16b4/chrome/app/mash/mash_runner.cc
,
Apr 10 2017
This would be a good bug for someone wanting to get some familiarity with the session_manager code (which I've inherited). #2 describes what seems to be going on; the fix is likely to be pretty small.
,
Apr 14 2017
,
Apr 14 2017
Starting on this early next week.
,
Apr 17 2017
,
Apr 18 2017
,
Apr 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/d8494036ddaa76cb3149529db2cd8144593dd4f5 commit d8494036ddaa76cb3149529db2cd8144593dd4f5 Author: Jason D. Clinton <jclinton@chromium.org> Date: Sat Apr 22 04:57:51 2017 login: Clear session_manager signal handlers and masks after child fork See linked bug: processes spawned by session_manager have their handlers set to the state of session_manager at the time of fork() which includes some handlers that have been installed by Brillo. This sets everything to POSIX safe defaults. BUG= chromium:699777 TEST=Build and deploy image to Peppy, sent masked signals to child processes of session_manager, no signals were blocked. Change-Id: I27aff4b6928a61897ee56285788378c3436dd531 Reviewed-on: https://chromium-review.googlesource.com/479466 Commit-Ready: Jason Clinton <jclinton@chromium.org> Tested-by: Jason Clinton <jclinton@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/d8494036ddaa76cb3149529db2cd8144593dd4f5/login_manager/child_job.cc [modify] https://crrev.com/d8494036ddaa76cb3149529db2cd8144593dd4f5/login_manager/session_manager_service.cc [modify] https://crrev.com/d8494036ddaa76cb3149529db2cd8144593dd4f5/login_manager/session_manager_service.h
,
Apr 23 2017
,
May 30 2017
,
Jul 5 2017
,
Aug 1 2017
,
Jan 22 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by jamescook@chromium.org
, Mar 8 2017