summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkmackay <kmackay@chromium.org>2015-12-29 11:08:10 -0800
committerCommit bot <commit-bot@chromium.org>2015-12-29 19:09:55 +0000
commit318de4b00ef94836116758d2bba1892fb0eb246c (patch)
treed39138d347256c2f8bed25aacce368da6935e206
parent5fc45b53137022c58150279fb62a0c2ef18bff80 (diff)
downloadchromium_src-318de4b00ef94836116758d2bba1892fb0eb246c.zip
chromium_src-318de4b00ef94836116758d2bba1892fb0eb246c.tar.gz
chromium_src-318de4b00ef94836116758d2bba1892fb0eb246c.tar.bz2
[Chromecast] Make SIGTERM/SIGINT handler safer
* Use signal-safe logging. * Ignore signals that are not to the main thread. As far as I can tell, the runloop's quit closure is safe to run in a signal handler, so that should be OK. BUG= internal b/26253582 Review URL: https://codereview.chromium.org/1542233002 Cr-Commit-Position: refs/heads/master@{#367089}
-rw-r--r--chromecast/browser/cast_browser_main_parts.cc25
1 files changed, 17 insertions, 8 deletions
diff --git a/chromecast/browser/cast_browser_main_parts.cc b/chromecast/browser/cast_browser_main_parts.cc
index 14f2331..a703723 100644
--- a/chromecast/browser/cast_browser_main_parts.cc
+++ b/chromecast/browser/cast_browser_main_parts.cc
@@ -5,6 +5,7 @@
#include "chromecast/browser/cast_browser_main_parts.h"
#include <stddef.h>
+#include <string.h>
#include <string>
@@ -76,15 +77,21 @@ namespace {
#if !defined(OS_ANDROID)
int kSignalsToRunClosure[] = { SIGTERM, SIGINT, };
-
// Closure to run on SIGTERM and SIGINT.
-base::Closure* g_signal_closure = NULL;
+base::Closure* g_signal_closure = nullptr;
+base::PlatformThreadId g_main_thread_id;
void RunClosureOnSignal(int signum) {
- LOG(ERROR) << "Got signal " << signum;
+ if (base::PlatformThread::CurrentId() != g_main_thread_id) {
+ RAW_LOG(INFO, "Received signal on non-main thread\n");
+ return;
+ }
+
+ char message[48] = "Received close signal: ";
+ strncat(message, sys_siglist[signum], sizeof(message) - strlen(message) - 1);
+ RAW_LOG(INFO, message);
+
DCHECK(g_signal_closure);
- // Expect main thread got this signal. Otherwise, weak_ptr of run_loop will
- // crash the process.
g_signal_closure->Run();
}
@@ -92,8 +99,10 @@ void RegisterClosureOnSignal(const base::Closure& closure) {
DCHECK(!g_signal_closure);
DCHECK_GT(arraysize(kSignalsToRunClosure), 0U);
- // Allow memory leak by intention.
+ // Memory leak on purpose, since |g_signal_closure| should live until
+ // process exit.
g_signal_closure = new base::Closure(closure);
+ g_main_thread_id = base::PlatformThread::CurrentId();
struct sigaction sa_new;
memset(&sa_new, 0, sizeof(sa_new));
@@ -101,9 +110,9 @@ void RegisterClosureOnSignal(const base::Closure& closure) {
sigfillset(&sa_new.sa_mask);
sa_new.sa_flags = SA_RESTART;
- for (size_t i = 0; i < arraysize(kSignalsToRunClosure); i++) {
+ for (int sig : kSignalsToRunClosure) {
struct sigaction sa_old;
- if (sigaction(kSignalsToRunClosure[i], &sa_new, &sa_old) == -1) {
+ if (sigaction(sig, &sa_new, &sa_old) == -1) {
NOTREACHED();
} else {
DCHECK_EQ(sa_old.sa_handler, SIG_DFL);