Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.

* Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to logging.h)
* Don't directly post a task to the UI loop.  Write to a magic pipe.  Read this from a separate thread which will post to a task to the UI loop.
BUG=http://crbug.com/29240

Review URL: http://codereview.chromium.org/460094

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34036 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 152 insertions, 31 deletions

diff --git a/base/logging.cc b/base/logging.cc
index af513be..706dbb1 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -5,9 +5,14 @@
 #include "base/logging.h"
 
 #if defined(OS_WIN)
+#include <io.h>
 #include <windows.h>
 typedef HANDLE FileHandle;
 typedef HANDLE MutexHandle;
+// Windows warns on using write().  It prefers _write().
+#define write(fd, buf, count) _write(fd, buf, static_cast<unsigned int>(count))
+// Windows doesn't define STDERR_FILENO.  Define it here.
+#define STDERR_FILENO 2
 #elif defined(OS_MACOSX)
 #include <CoreFoundation/CoreFoundation.h>
 #include <mach/mach.h>
@@ -37,6 +42,7 @@ typedef pthread_mutex_t* MutexHandle;
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/debug_util.h"
+#include "base/eintr_wrapper.h"
 #include "base/lock_impl.h"
 #if defined(OS_POSIX)
 #include "base/safe_strerror_posix.h"
@@ -684,6 +690,37 @@ void CloseLogFile() {
   log_file = NULL;
 }
 
+void RawLog(int level, const char* message) {
+  if (level >= min_log_level) {
+    size_t bytes_written = 0;
+    const size_t message_len = strlen(message);
+    int rv;
+    while (bytes_written < message_len) {
+      rv = HANDLE_EINTR(
+          write(STDERR_FILENO, message + bytes_written,
+                message_len - bytes_written));
+      if (rv < 0) {
+        // Give up, nothing we can do now.
+        break;
+      }
+      bytes_written += rv;
+    }
+
+    if (message_len > 0 && message[message_len - 1] != '\n') {
+      do {
+        rv = HANDLE_EINTR(write(STDERR_FILENO, "\n", 1));
+        if (rv < 0) {
+          // Give up, nothing we can do now.
+          break;
+        }
+      } while (rv != 1);
+    }
+  }
+
+  if (level == LOG_FATAL)
+    DebugUtil::BreakDebugger();
+}
+
 }  // namespace logging
 
 std::ostream& operator<<(std::ostream& out, const wchar_t* wstr) {
diff --git a/base/logging.h b/base/logging.h
index 4fbe483..cac9ca3 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -772,6 +772,17 @@ class ErrnoLogMessage {
 //       after this call.
 void CloseLogFile();
 
+// Async signal safe logging mechanism.
+void RawLog(int level, const char* message);
+
+#define RAW_LOG(level, message) logging::RawLog(logging::LOG_ ## level, message)
+
+#define RAW_CHECK(condition)                                                   \
+  do {                                                                         \
+    if (!(condition))                                                          \
+      logging::RawLog(logging::LOG_FATAL, "Check failed: " #condition "\n");   \
+  } while (0)
+
 }  // namespace logging
 
 // These functions are provided as a convenience for logging, which is where we
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 17d2089..766fb91 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -17,6 +17,7 @@
 #include "base/lazy_instance.h"
 #include "base/scoped_nsautorelease_pool.h"
 #include "base/path_service.h"
+#include "base/platform_thread.h"
 #include "base/process_util.h"
 #include "base/string_piece.h"
 #include "base/string_util.h"
@@ -72,6 +73,7 @@
 #include <errno.h>
 #include <signal.h>
 #include <sys/resource.h>
+#include "base/eintr_wrapper.h"
 #endif
 
 #if defined(USE_LINUX_BREAKPAD)
@@ -168,30 +170,99 @@ void RunUIMessageLoop(BrowserProcess* browser_process) {
 void SIGCHLDHandler(int signal) {
 }
 
-// Common code between SIG{HUP, INT, TERM}Handler.
-void GracefulShutdownHandler(int signal, const int expected_signal) {
-  DCHECK_EQ(signal, expected_signal);
-  LOG(INFO) << "Addressing signal " << expected_signal << " on "
-            << PlatformThread::CurrentId();
-
-  bool posted = ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableFunction(BrowserList::CloseAllBrowsersAndExit));
+int g_shutdown_pipe_write_fd = -1;
+int g_shutdown_pipe_read_fd = -1;
 
+// Common code between SIG{HUP, INT, TERM}Handler.
+void GracefulShutdownHandler(int signal) {
   // Reinstall the default handler.  We had one shot at graceful shutdown.
   struct sigaction action;
   memset(&action, 0, sizeof(action));
   action.sa_handler = SIG_DFL;
-  CHECK(sigaction(expected_signal, &action, NULL) == 0);
+  CHECK(sigaction(signal, &action, NULL) == 0);
+
+  RAW_CHECK(g_shutdown_pipe_write_fd != -1);
+  RAW_CHECK(g_shutdown_pipe_read_fd != -1);
+  size_t bytes_written = 0;
+  do {
+    int rv = HANDLE_EINTR(
+        write(g_shutdown_pipe_write_fd,
+              reinterpret_cast<const char*>(&signal) + bytes_written,
+              sizeof(signal) - bytes_written));
+    RAW_CHECK(rv >= 0);
+    bytes_written += rv;
+  } while (bytes_written < sizeof(signal));
+
+  RAW_LOG(INFO,
+          "Successfully wrote to shutdown pipe, resetting signal handler.");
+}
 
-  if (posted) {
-    LOG(INFO) << "Posted task to UI thread; resetting signal "
-              << expected_signal << " handler";
-  } else {
+// See comment in BrowserMain, where sigaction is called.
+void SIGHUPHandler(int signal) {
+  RAW_CHECK(signal == SIGHUP);
+  RAW_LOG(INFO, "Handling SIGHUP.");
+  GracefulShutdownHandler(signal);
+}
+
+// See comment in BrowserMain, where sigaction is called.
+void SIGINTHandler(int signal) {
+  RAW_CHECK(signal == SIGINT);
+  RAW_LOG(INFO, "Handling SIGINT.");
+  GracefulShutdownHandler(signal);
+}
+
+// See comment in BrowserMain, where sigaction is called.
+void SIGTERMHandler(int signal) {
+  RAW_CHECK(signal == SIGTERM);
+  RAW_LOG(INFO, "Handling SIGTERM.");
+  GracefulShutdownHandler(signal);
+}
+
+class ShutdownDetector : public PlatformThread::Delegate {
+ public:
+  explicit ShutdownDetector(int shutdown_fd);
+
+  virtual void ThreadMain();
+
+ private:
+  const int shutdown_fd_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShutdownDetector);
+};
+
+ShutdownDetector::ShutdownDetector(int shutdown_fd)
+    : shutdown_fd_(shutdown_fd) {
+  CHECK(shutdown_fd_ != -1);
+}
+
+void ShutdownDetector::ThreadMain() {
+  int signal;
+  size_t bytes_read = 0;
+  ssize_t ret;
+  do {
+    ret = HANDLE_EINTR(
+        read(shutdown_fd_,
+             reinterpret_cast<char*>(&signal) + bytes_read,
+             sizeof(signal) - bytes_read));
+    if (ret < 0) {
+      NOTREACHED() << "Unexpected error: " << strerror(errno);
+      break;
+    } else if (ret == 0) {
+      NOTREACHED() << "Unexpected closure of shutdown pipe.";
+      break;
+    }
+    bytes_read += ret;
+  } while (bytes_read < sizeof(signal));
+
+  LOG(INFO) << "Handling shutdown for signal " << signal << ".";
+
+  if (!ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableFunction(BrowserList::CloseAllBrowsersAndExit))) {
     // Without a UI thread to post the exit task to, there aren't many
     // options.  Raise the signal again.  The default handler will pick it up
     // and cause an ungraceful exit.
-    LOG(WARNING) << "No UI thread, exiting ungracefully";
+    LOG(WARNING) << "No UI thread, exiting ungracefully.";
     kill(getpid(), signal);
 
     // The signal may be handled on another thread.  Give that a chance to
@@ -204,26 +275,11 @@ void GracefulShutdownHandler(int signal, const int expected_signal) {
     // normally used to indicate an exit by this signal's default handler.
     // This mechanism isn't a de jure standard, but even in the worst case, it
     // should at least result in an immediate exit.
-    LOG(WARNING) << "Still here, exiting really ungracefully";
+    LOG(WARNING) << "Still here, exiting really ungracefully.";
     _exit(signal | (1 << 7));
   }
 }
 
-// See comment in BrowserMain, where sigaction is called.
-void SIGHUPHandler(int signal) {
-  GracefulShutdownHandler(signal, SIGHUP);
-}
-
-// See comment in BrowserMain, where sigaction is called.
-void SIGINTHandler(int signal) {
-  GracefulShutdownHandler(signal, SIGINT);
-}
-
-// See comment in BrowserMain, where sigaction is called.
-void SIGTERMHandler(int signal) {
-  GracefulShutdownHandler(signal, SIGTERM);
-}
-
 // Sets the file descriptor soft limit to |max_descriptors| or the OS hard
 // limit, whichever is lower.
 void SetFileDescriptorLimit(unsigned int max_descriptors) {
@@ -362,6 +418,23 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // Register the main thread by instantiating it, but don't call any methods.
   ChromeThread main_thread(ChromeThread::UI, MessageLoop::current());
 
+#if defined(OS_POSIX)
+  int pipefd[2];
+  int ret = pipe(pipefd);
+  if (ret < 0) {
+    PLOG(DFATAL) << "Failed to create pipe";
+  } else {
+    g_shutdown_pipe_read_fd = pipefd[0];
+    g_shutdown_pipe_write_fd = pipefd[1];
+    const size_t kShutdownDetectorThreadStackSize = 4096;
+    if (!PlatformThread::CreateNonJoinable(
+        kShutdownDetectorThreadStackSize,
+        new ShutdownDetector(g_shutdown_pipe_read_fd))) {
+      LOG(DFATAL) << "Failed to create shutdown detector task.";
+    }
+  }
+#endif  // defined(OS_POSIX)
+
   FilePath user_data_dir;
 #if defined(OS_WIN)
   PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);