summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/browser_main.cc19
-rw-r--r--chrome/browser/process_singleton.h12
-rw-r--r--chrome/browser/process_singleton_linux.cc42
-rw-r--r--chrome/browser/process_singleton_linux_uitest.cc60
-rw-r--r--chrome/browser/process_singleton_mac.cc5
-rw-r--r--chrome/browser/process_singleton_win.cc7
-rw-r--r--chrome/test/data/valgrind/ui_tests.gtest.txt1
7 files changed, 120 insertions, 26 deletions
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 1ccb00f..1bf8121 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -949,7 +949,7 @@ int BrowserMain(const MainFunctionParams& parameters) {
// new one. NotifyOtherProcess will currently give the other process up to
// 20 seconds to respond. Note that this needs to be done before we attempt
// to read the profile.
- switch (process_singleton.NotifyOtherProcess()) {
+ switch (process_singleton.NotifyOtherProcessOrCreate()) {
case ProcessSingleton::PROCESS_NONE:
// No process already running, fall through to starting a new one.
break;
@@ -964,6 +964,14 @@ int BrowserMain(const MainFunctionParams& parameters) {
case ProcessSingleton::PROFILE_IN_USE:
return ResultCodes::PROFILE_IN_USE;
+ case ProcessSingleton::LOCK_ERROR:
+ LOG(ERROR) << "Failed to create a ProcessSingleton for your profile "
+ "directory. This means that running multiple instances "
+ "would start multiple browser processes rather than "
+ "opening a new window in the existing process. Aborting "
+ "now to avoid profile corruption.";
+ return ResultCodes::PROFILE_IN_USE;
+
default:
NOTREACHED();
}
@@ -1028,15 +1036,6 @@ int BrowserMain(const MainFunctionParams& parameters) {
if (CheckMachineLevelInstall())
return ResultCodes::MACHINE_LEVEL_INSTALL_EXISTS;
- if (!process_singleton.Create()) {
- LOG(ERROR) << "Failed to create a ProcessSingleton for your profile "
- "directory. This means that running multiple instances "
- "would start multiple browser processes rather than opening "
- "a new window in the existing process. Aborting now to "
- "avoid profile corruption.";
- return ResultCodes::PROFILE_IN_USE;
- }
-
// Create the TranslateManager singleton.
Singleton<TranslateManager>::get();
diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h
index 4ae0b10..d2ee14f 100644
--- a/chrome/browser/process_singleton.h
+++ b/chrome/browser/process_singleton.h
@@ -37,6 +37,7 @@ class ProcessSingleton : public NonThreadSafe {
PROCESS_NONE,
PROCESS_NOTIFIED,
PROFILE_IN_USE,
+ LOCK_ERROR,
};
explicit ProcessSingleton(const FilePath& user_data_dir);
@@ -52,11 +53,20 @@ class ProcessSingleton : public NonThreadSafe {
// first one, so this function won't find it.
NotifyResult NotifyOtherProcess();
+ // Notify another process, if available. Otherwise sets ourselves as the
+ // singleton instance. Returns PROCESS_NONE if we became the singleton
+ // instance.
+ NotifyResult NotifyOtherProcessOrCreate();
+
#if defined(OS_POSIX) && !defined(OS_MACOSX)
// Exposed for testing. We use a timeout on Linux, and in tests we want
// this timeout to be short.
NotifyResult NotifyOtherProcessWithTimeout(const CommandLine& command_line,
- int timeout_seconds);
+ int timeout_seconds,
+ bool kill_unresponsive);
+ NotifyResult NotifyOtherProcessWithTimeoutOrCreate(
+ const CommandLine& command_line,
+ int timeout_seconds);
#endif
// Sets ourself up as the singleton instance. Returns true on success. If
diff --git a/chrome/browser/process_singleton_linux.cc b/chrome/browser/process_singleton_linux.cc
index fdf4c2b..2a1a01c 100644
--- a/chrome/browser/process_singleton_linux.cc
+++ b/chrome/browser/process_singleton_linux.cc
@@ -648,12 +648,14 @@ ProcessSingleton::~ProcessSingleton() {
ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
return NotifyOtherProcessWithTimeout(*CommandLine::ForCurrentProcess(),
- kTimeoutInSeconds);
+ kTimeoutInSeconds,
+ true);
}
ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout(
const CommandLine& cmd_line,
- int timeout_seconds) {
+ int timeout_seconds,
+ bool kill_unresponsive) {
DCHECK_GE(timeout_seconds, 0);
int socket;
@@ -710,7 +712,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout(
if (retries == timeout_seconds) {
// Retries failed. Kill the unresponsive chrome process and continue.
- if (!KillProcessByLockPath(lock_path_.value()))
+ if (!kill_unresponsive || !KillProcessByLockPath(lock_path_.value()))
return PROFILE_IN_USE;
return PROCESS_NONE;
}
@@ -741,7 +743,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout(
// Send the message
if (!WriteToSocket(socket, to_send.data(), to_send.length())) {
// Try to kill the other process, because it might have been dead.
- if (!KillProcessByLockPath(lock_path_.value()))
+ if (!kill_unresponsive || !KillProcessByLockPath(lock_path_.value()))
return PROFILE_IN_USE;
return PROCESS_NONE;
}
@@ -757,7 +759,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout(
// Failed to read ACK, the other process might have been frozen.
if (len <= 0) {
- if (!KillProcessByLockPath(lock_path_.value()))
+ if (!kill_unresponsive || !KillProcessByLockPath(lock_path_.value()))
return PROFILE_IN_USE;
return PROCESS_NONE;
}
@@ -775,6 +777,34 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout(
return PROCESS_NOTIFIED;
}
+ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() {
+ return NotifyOtherProcessWithTimeoutOrCreate(
+ *CommandLine::ForCurrentProcess(),
+ kTimeoutInSeconds);
+}
+
+ProcessSingleton::NotifyResult
+ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate(
+ const CommandLine& command_line,
+ int timeout_seconds) {
+ NotifyResult result = NotifyOtherProcessWithTimeout(command_line,
+ timeout_seconds, true);
+ if (result != PROCESS_NONE)
+ return result;
+ if (Create())
+ return PROCESS_NONE;
+ // If the Create() failed, try again to notify. (It could be that another
+ // instance was starting at the same time and managed to grab the lock before
+ // we did.)
+ // This time, we don't want to kill anything if we aren't successful, since we
+ // aren't going to try to take over the lock ourselves.
+ result = NotifyOtherProcessWithTimeout(command_line, timeout_seconds, false);
+ if (result != PROCESS_NONE)
+ return result;
+
+ return LOCK_ERROR;
+}
+
bool ProcessSingleton::Create() {
int sock;
sockaddr_un addr;
@@ -796,8 +826,6 @@ bool ProcessSingleton::Create() {
if (ReadLink(lock_path_.value()) != symlink_content) {
// If we failed to create the lock, most likely another instance won the
// startup race.
- // TODO(mattm): If the other instance is on the same host, we could try
- // to notify it rather than just failing.
errno = saved_errno;
PLOG(ERROR) << "Failed to create " << lock_path_.value();
return false;
diff --git a/chrome/browser/process_singleton_linux_uitest.cc b/chrome/browser/process_singleton_linux_uitest.cc
index ef42966..3f7ddd8 100644
--- a/chrome/browser/process_singleton_linux_uitest.cc
+++ b/chrome/browser/process_singleton_linux_uitest.cc
@@ -30,14 +30,14 @@ namespace {
typedef UITest ProcessSingletonLinuxTest;
-// A helper method to call ProcessSingleton::NotifyOtherProcess().
-// |url| will be added to CommandLine for current process, so that it can be
-// sent to browser process by ProcessSingleton::NotifyOtherProcess().
-ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url,
- int timeout_ms) {
+ProcessSingleton* CreateProcessSingleton() {
FilePath user_data_dir;
PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+ return new ProcessSingleton(user_data_dir);
+}
+
+CommandLine CommandLineForUrl(const std::string& url) {
// Hack: mutate the current process's command line so we don't show a dialog.
// Note that this only works if we have no loose values on the command line,
// but that's fine for unit tests. In a UI test we disable error dialogs
@@ -48,11 +48,28 @@ ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url,
CommandLine new_cmd_line(*cmd_line);
new_cmd_line.AppendLooseValue(ASCIIToWide(url));
+ return new_cmd_line;
+}
- ProcessSingleton process_singleton(user_data_dir);
+// A helper method to call ProcessSingleton::NotifyOtherProcess().
+// |url| will be added to CommandLine for current process, so that it can be
+// sent to browser process by ProcessSingleton::NotifyOtherProcess().
+ProcessSingleton::NotifyResult NotifyOtherProcess(const std::string& url,
+ int timeout_ms) {
+ scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+ return process_singleton->NotifyOtherProcessWithTimeout(
+ CommandLineForUrl(url), timeout_ms / 1000, true);
+}
- return process_singleton.NotifyOtherProcessWithTimeout(
- new_cmd_line, timeout_ms / 1000);
+// A helper method to call ProcessSingleton::NotifyOtherProcessOrCreate().
+// |url| will be added to CommandLine for current process, so that it can be
+// sent to browser process by ProcessSingleton::NotifyOtherProcessOrCreate().
+ProcessSingleton::NotifyResult NotifyOtherProcessOrCreate(
+ const std::string& url,
+ int timeout_ms) {
+ scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+ return process_singleton->NotifyOtherProcessWithTimeoutOrCreate(
+ CommandLineForUrl(url), timeout_ms / 1000);
}
} // namespace
@@ -188,3 +205,30 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessDifferingHost) {
EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE,
NotifyOtherProcess(url, action_timeout_ms()));
}
+
+// Test that we fail when lock says process is on another host and we can't
+// notify it over the socket.
+TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessOrCreate_DifferingHost) {
+ base::ProcessId pid = browser_process_id();
+
+ ASSERT_GE(pid, 1);
+
+ // Kill the browser process, so that it does not respond on the socket.
+ kill(pid, SIGKILL);
+ // Wait for a while to make sure the browser process is actually killed.
+ EXPECT_FALSE(CrashAwareSleep(sleep_timeout_ms()));
+
+ FilePath lock_path = user_data_dir().Append(chrome::kSingletonLockFilename);
+ EXPECT_EQ(0, unlink(lock_path.value().c_str()));
+ EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path.value().c_str()));
+
+ std::string url("about:blank");
+ EXPECT_EQ(ProcessSingleton::PROFILE_IN_USE,
+ NotifyOtherProcessOrCreate(url, action_timeout_ms()));
+}
+
+// Test that Create fails when another browser is using the profile directory.
+TEST_F(ProcessSingletonLinuxTest, CreateFailsWithExistingBrowser) {
+ scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+ EXPECT_FALSE(process_singleton->Create());
+}
diff --git a/chrome/browser/process_singleton_mac.cc b/chrome/browser/process_singleton_mac.cc
index 50148a9..23d317c 100644
--- a/chrome/browser/process_singleton_mac.cc
+++ b/chrome/browser/process_singleton_mac.cc
@@ -32,6 +32,11 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
return PROCESS_NONE;
}
+ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() {
+ // This space intentionally left blank.
+ return PROCESS_NONE;
+}
+
bool ProcessSingleton::Create() {
// This space intentionally left blank.
return true;
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 09dec13..6454f66 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -156,6 +156,13 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
return PROCESS_NONE;
}
+ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() {
+ NotifyResult result = NotifyOtherProcess();
+ if (result != PROCESS_NONE)
+ return result;
+ return Create() ? PROCESS_NONE : PROFILE_IN_USE;
+}
+
// For windows, there is no need to call Create() since the call is made in
// the constructor but to avoid having more platform specific code in
// browser_main.cc we tolerate a second call which will do nothing.
diff --git a/chrome/test/data/valgrind/ui_tests.gtest.txt b/chrome/test/data/valgrind/ui_tests.gtest.txt
index a61c639..04c8f85 100644
--- a/chrome/test/data/valgrind/ui_tests.gtest.txt
+++ b/chrome/test/data/valgrind/ui_tests.gtest.txt
@@ -32,6 +32,7 @@ TabRestoreUITest.*
# See http://crbug.com/25176
ProcessSingletonLinuxTest.NotifyOtherProcessFailure
ProcessSingletonLinuxTest.NotifyOtherProcessDifferingHost
+ProcessSingletonLinuxTest.NotifyOtherProcessOrCreate_DifferingHost
# These tests fail under valgrind.
# See http://crbug.com/29579.
generated by cgit v1.1 at 2025-01-10 09:18:35 +0000