Commit 2f1fa693 authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

Assume session is ending when receiving SIGTERM on Linux

See the comments added in this CL for details.

BUG=429404
R=sky

Change-Id: Icc5bad8e70853a62f76640b111a89f9e6b487d4b
Reviewed-on: https://chromium-review.googlesource.com/c/1481125
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636525}
parent 3c575f83
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include <string.h> #include <string.h>
#include <unistd.h> #include <unistd.h>
#include <utility>
#include "base/callback.h" #include "base/callback.h"
#include "base/debug/leak_annotations.h" #include "base/debug/leak_annotations.h"
#include "base/logging.h" #include "base/logging.h"
...@@ -74,7 +76,7 @@ class ShutdownDetector : public base::PlatformThread::Delegate { ...@@ -74,7 +76,7 @@ class ShutdownDetector : public base::PlatformThread::Delegate {
public: public:
ShutdownDetector( ShutdownDetector(
int shutdown_fd, int shutdown_fd,
const base::Closure& shutdown_callback, base::OnceCallback<void(int)> shutdown_callback,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
~ShutdownDetector() override; ~ShutdownDetector() override;
...@@ -83,7 +85,7 @@ class ShutdownDetector : public base::PlatformThread::Delegate { ...@@ -83,7 +85,7 @@ class ShutdownDetector : public base::PlatformThread::Delegate {
private: private:
const int shutdown_fd_; const int shutdown_fd_;
const base::Closure shutdown_callback_; base::OnceCallback<void(int)> shutdown_callback_;
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_; const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
DISALLOW_COPY_AND_ASSIGN(ShutdownDetector); DISALLOW_COPY_AND_ASSIGN(ShutdownDetector);
...@@ -91,13 +93,13 @@ class ShutdownDetector : public base::PlatformThread::Delegate { ...@@ -91,13 +93,13 @@ class ShutdownDetector : public base::PlatformThread::Delegate {
ShutdownDetector::ShutdownDetector( ShutdownDetector::ShutdownDetector(
int shutdown_fd, int shutdown_fd,
const base::Closure& shutdown_callback, base::OnceCallback<void(int)> shutdown_callback,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
: shutdown_fd_(shutdown_fd), : shutdown_fd_(shutdown_fd),
shutdown_callback_(shutdown_callback), shutdown_callback_(std::move(shutdown_callback)),
task_runner_(task_runner) { task_runner_(task_runner) {
CHECK_NE(shutdown_fd_, -1); CHECK_NE(shutdown_fd_, -1);
CHECK(!shutdown_callback.is_null()); CHECK(!shutdown_callback_.is_null());
CHECK(task_runner_); CHECK(task_runner_);
} }
...@@ -146,7 +148,8 @@ void ShutdownDetector::ThreadMain() { ...@@ -146,7 +148,8 @@ void ShutdownDetector::ThreadMain() {
} while (bytes_read < sizeof(signal)); } while (bytes_read < sizeof(signal));
VLOG(1) << "Handling shutdown for signal " << signal << "."; VLOG(1) << "Handling shutdown for signal " << signal << ".";
if (!task_runner_->PostTask(FROM_HERE, shutdown_callback_)) { if (!task_runner_->PostTask(
FROM_HERE, base::BindOnce(std::move(shutdown_callback_), signal))) {
// Without a valid task runner to post the exit task to, there aren't many // Without a valid task runner to post the exit task to, there aren't many
// options. Raise the signal again. The default handler will pick it up // options. Raise the signal again. The default handler will pick it up
// and cause an ungraceful exit. // and cause an ungraceful exit.
...@@ -172,7 +175,7 @@ void ShutdownDetector::ThreadMain() { ...@@ -172,7 +175,7 @@ void ShutdownDetector::ThreadMain() {
} // namespace } // namespace
void InstallShutdownSignalHandlers( void InstallShutdownSignalHandlers(
const base::Closure& shutdown_callback, base::OnceCallback<void(int)> shutdown_callback,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) { const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
int pipefd[2]; int pipefd[2];
int ret = pipe(pipefd); int ret = pipe(pipefd);
...@@ -191,7 +194,7 @@ void InstallShutdownSignalHandlers( ...@@ -191,7 +194,7 @@ void InstallShutdownSignalHandlers(
const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 4; const size_t kShutdownDetectorThreadStackSize = PTHREAD_STACK_MIN * 4;
#endif #endif
ShutdownDetector* detector = new ShutdownDetector( ShutdownDetector* detector = new ShutdownDetector(
g_shutdown_pipe_read_fd, shutdown_callback, task_runner); g_shutdown_pipe_read_fd, std::move(shutdown_callback), task_runner);
// PlatformThread does not delete its delegate. // PlatformThread does not delete its delegate.
ANNOTATE_LEAKING_OBJECT_PTR(detector); ANNOTATE_LEAKING_OBJECT_PTR(detector);
if (!base::PlatformThread::CreateNonJoinable(kShutdownDetectorThreadStackSize, if (!base::PlatformThread::CreateNonJoinable(kShutdownDetectorThreadStackSize,
...@@ -211,15 +214,15 @@ void InstallShutdownSignalHandlers( ...@@ -211,15 +214,15 @@ void InstallShutdownSignalHandlers(
struct sigaction action; struct sigaction action;
memset(&action, 0, sizeof(action)); memset(&action, 0, sizeof(action));
action.sa_handler = SIGTERMHandler; action.sa_handler = SIGTERMHandler;
CHECK(sigaction(SIGTERM, &action, nullptr) == 0); CHECK_EQ(0, sigaction(SIGTERM, &action, nullptr));
// Also handle SIGINT - when the user terminates the browser via Ctrl+C. If // Also handle SIGINT - when the user terminates the browser via Ctrl+C. If
// the browser process is being debugged, GDB will catch the SIGINT first. // the browser process is being debugged, GDB will catch the SIGINT first.
action.sa_handler = SIGINTHandler; action.sa_handler = SIGINTHandler;
CHECK(sigaction(SIGINT, &action, nullptr) == 0); CHECK_EQ(0, sigaction(SIGINT, &action, nullptr));
// And SIGHUP, for when the terminal disappears. On shutdown, many Linux // And SIGHUP, for when the terminal disappears. On shutdown, many Linux
// distros send SIGHUP, SIGTERM, and then SIGKILL. // distros send SIGHUP, SIGTERM, and then SIGKILL.
action.sa_handler = SIGHUPHandler; action.sa_handler = SIGHUPHandler;
CHECK(sigaction(SIGHUP, &action, nullptr) == 0); CHECK_EQ(0, sigaction(SIGHUP, &action, nullptr));
} }
...@@ -16,7 +16,7 @@ class SingleThreadTaskRunner; ...@@ -16,7 +16,7 @@ class SingleThreadTaskRunner;
// signals like SIGTERM, SIGINT and SIGTERM. |shutdown_callback| is invoked on // signals like SIGTERM, SIGINT and SIGTERM. |shutdown_callback| is invoked on
// |task_runner| which is usually the main thread's task runner. // |task_runner| which is usually the main thread's task runner.
void InstallShutdownSignalHandlers( void InstallShutdownSignalHandlers(
const base::Closure& shutdown_callback, base::OnceCallback<void(int)> shutdown_callback,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner); const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
#endif // CHROME_APP_SHUTDOWN_SIGNAL_HANDLERS_POSIX_H_ #endif // CHROME_APP_SHUTDOWN_SIGNAL_HANDLERS_POSIX_H_
...@@ -40,7 +40,7 @@ void SIGCHLDHandler(int signal) { ...@@ -40,7 +40,7 @@ void SIGCHLDHandler(int signal) {
class ExitHandler { class ExitHandler {
public: public:
// Invokes exit when appropriate. // Invokes exit when appropriate.
static void ExitWhenPossibleOnUIThread(); static void ExitWhenPossibleOnUIThread(int signal);
private: private:
ExitHandler(); ExitHandler();
...@@ -64,13 +64,38 @@ class ExitHandler { ...@@ -64,13 +64,38 @@ class ExitHandler {
}; };
// static // static
void ExitHandler::ExitWhenPossibleOnUIThread() { void ExitHandler::ExitWhenPossibleOnUIThread(int signal) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (SessionRestore::IsRestoringSynchronously()) { if (SessionRestore::IsRestoringSynchronously()) {
// ExitHandler takes care of deleting itself. // ExitHandler takes care of deleting itself.
new ExitHandler(); new ExitHandler();
} else { } else {
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
switch (signal) {
case SIGINT:
case SIGHUP:
// SIGINT gets sent when the user types Ctrl+C, but the session is
// likely not going away, so try to exit gracefully. SIGHUP is sent on
// most systems as a first warning of shutdown. If the process takes
// too long to quit, the next signal is usually SIGTERM.
Exit();
break;
case SIGTERM:
// SIGTERM is usually sent instead of SIGKILL to gracefully shutdown
// processes. But most systems use it as a shutdown warning, so
// conservatively assume that the session is ending. If the process
// still doesn't quit within a bounded time, most systems will finally
// send SIGKILL, which we're unable to install a signal handler for.
// TODO(thomasanderson): Try to distinguish if the session is really
// ending or not. Maybe there's a systemd or DBus API to query.
chrome::SessionEnding();
break;
default:
NOTREACHED();
}
#else
Exit(); Exit();
#endif
} }
} }
...@@ -125,7 +150,7 @@ int ChromeBrowserMainPartsPosix::PreEarlyInitialization() { ...@@ -125,7 +150,7 @@ int ChromeBrowserMainPartsPosix::PreEarlyInitialization() {
struct sigaction action; struct sigaction action;
memset(&action, 0, sizeof(action)); memset(&action, 0, sizeof(action));
action.sa_handler = SIGCHLDHandler; action.sa_handler = SIGCHLDHandler;
CHECK(sigaction(SIGCHLD, &action, NULL) == 0); CHECK_EQ(0, sigaction(SIGCHLD, &action, NULL));
return service_manager::RESULT_CODE_NORMAL_EXIT; return service_manager::RESULT_CODE_NORMAL_EXIT;
} }
...@@ -135,7 +160,7 @@ void ChromeBrowserMainPartsPosix::PostMainMessageLoopStart() { ...@@ -135,7 +160,7 @@ void ChromeBrowserMainPartsPosix::PostMainMessageLoopStart() {
// Exit in response to SIGINT, SIGTERM, etc. // Exit in response to SIGINT, SIGTERM, etc.
InstallShutdownSignalHandlers( InstallShutdownSignalHandlers(
base::Bind(&ExitHandler::ExitWhenPossibleOnUIThread), base::BindOnce(&ExitHandler::ExitWhenPossibleOnUIThread),
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI})); base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI}));
} }
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "chrome/test/base/in_process_browser_test.h"
#include <signal.h> #include <signal.h>
#include "base/bind.h" #include "base/bind.h"
...@@ -65,7 +63,7 @@ class FirstRunInternalPosixTest : public InProcessBrowserTest { ...@@ -65,7 +63,7 @@ class FirstRunInternalPosixTest : public InProcessBrowserTest {
// Send a signal to myself. This should post a task for the next run loop // Send a signal to myself. This should post a task for the next run loop
// iteration to set browser_shutdown::IsTryingToQuit(), and interrupt the // iteration to set browser_shutdown::IsTryingToQuit(), and interrupt the
// RunLoop. // RunLoop.
raise(SIGTERM); raise(SIGINT);
inspected_state_ = true; inspected_state_ = true;
} }
...@@ -77,7 +75,7 @@ class FirstRunInternalPosixTest : public InProcessBrowserTest { ...@@ -77,7 +75,7 @@ class FirstRunInternalPosixTest : public InProcessBrowserTest {
// Test the first run flow for showing the modal dialog that surfaces the first // Test the first run flow for showing the modal dialog that surfaces the first
// run dialog. Ensure browser startup safely handles a signal while the modal // run dialog. Ensure browser startup safely handles a signal while the modal
// RunLoop is running. // RunLoop is running.
IN_PROC_BROWSER_TEST_F(FirstRunInternalPosixTest, HandleSigterm) { IN_PROC_BROWSER_TEST_F(FirstRunInternalPosixTest, HandleSigint) {
// Never reached. PreMainMessageLoopRunImpl() should return before this task // Never reached. PreMainMessageLoopRunImpl() should return before this task
// is run. // is run.
ADD_FAILURE() << "Should never be called"; ADD_FAILURE() << "Should never be called";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment