diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-23 00:11:17 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-23 00:11:17 +0000 |
commit | 679545860b7a5bcf0cc2b2343d3b3099952bb48a (patch) | |
tree | 07dc430d60e0b75d326b9970bf9fad9de9cffab5 | |
parent | 34858db7c8327eca9abc00c44aa43b5ff9344349 (diff) | |
download | chromium_src-679545860b7a5bcf0cc2b2343d3b3099952bb48a.zip chromium_src-679545860b7a5bcf0cc2b2343d3b3099952bb48a.tar.gz chromium_src-679545860b7a5bcf0cc2b2343d3b3099952bb48a.tar.bz2 |
Revert 35189 (caused test_shell_tests crashes on valgrind bot):
Take 2: Reenable TCMalloc on Linux and make POSIX signal handling async signal safe.
Reinstates r34096 and r34036, which were reverted by r34632 due to hanging ui_tests and browser_tests on bots.
I've been unable to repro on the trybots despite hitting them with a ton of try jobs, so I'm trying on the buildbots again.
BUG=http://crbug.com/30501
Review URL: http://codereview.chromium.org/505068
TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/517001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35192 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | build/common.gypi | 2 | ||||
-rw-r--r-- | chrome/browser/browser_main.cc | 135 |
2 files changed, 32 insertions, 105 deletions
diff --git a/build/common.gypi b/build/common.gypi index e41baf1..d1caf37 100644 --- a/build/common.gypi +++ b/build/common.gypi @@ -187,7 +187,7 @@ 'linux_strip_binary%': 0, # Enable TCMalloc. - 'linux_use_tcmalloc%': 1, + 'linux_use_tcmalloc%': 0, # Set to select the Title Case versions of strings in GRD files. 'use_titlecase_in_grd_files%': 0, diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index f4528a5..ff1d17d 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -17,7 +17,6 @@ #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" @@ -73,7 +72,6 @@ #include <errno.h> #include <signal.h> #include <sys/resource.h> -#include "base/eintr_wrapper.h" #endif #if defined(USE_LINUX_BREAKPAD) @@ -171,99 +169,30 @@ void RunUIMessageLoop(BrowserProcess* browser_process) { void SIGCHLDHandler(int signal) { } -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) { +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)); + // 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(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."); -} - -// 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); -} + CHECK(sigaction(expected_signal, &action, NULL) == 0); -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))) { + if (posted) { + LOG(INFO) << "Posted task to UI thread; resetting signal " + << expected_signal << " handler"; + } else { // 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 @@ -276,11 +205,26 @@ void ShutdownDetector::ThreadMain() { // 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) { @@ -419,23 +363,6 @@ 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); |