diff options
author | kmackay <kmackay@chromium.org> | 2015-12-29 11:08:10 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-29 19:09:55 +0000 |
commit | 318de4b00ef94836116758d2bba1892fb0eb246c (patch) | |
tree | d39138d347256c2f8bed25aacce368da6935e206 | |
parent | 5fc45b53137022c58150279fb62a0c2ef18bff80 (diff) | |
download | chromium_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.cc | 25 |
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); |