summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/automation/automation_provider_observers.cc32
-rw-r--r--chrome/browser/automation/automation_provider_observers.h1
-rw-r--r--chrome/browser/importer/firefox_importer_unittest_utils_mac.cc17
-rw-r--r--chrome/common/logging_chrome_uitest.cc29
-rw-r--r--chrome/test/automation/automation_proxy.cc12
-rw-r--r--chrome/test/automation/automation_proxy.h4
-rw-r--r--chrome/test/automation/proxy_launcher.cc31
-rw-r--r--chrome/test/automation/proxy_launcher.h1
-rw-r--r--content/browser/browser_child_process_host.cc2
-rw-r--r--content/browser/child_process_launcher.cc5
-rw-r--r--content/browser/renderer_host/browser_render_process_host.cc3
-rw-r--r--content/common/gpu/gpu_channel.cc10
-rw-r--r--content/common/gpu/gpu_channel.h2
-rw-r--r--content/common/gpu/gpu_channel_manager.cc4
-rw-r--r--content/plugin/plugin_channel.h4
-rw-r--r--content/plugin/plugin_thread.cc3
-rw-r--r--ipc/ipc_channel.h6
-rw-r--r--ipc/ipc_channel_posix.cc50
-rw-r--r--ipc/ipc_channel_posix.h5
-rw-r--r--ipc/ipc_channel_proxy.cc14
-rw-r--r--ipc/ipc_channel_proxy.h3
21 files changed, 152 insertions, 86 deletions
diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index b5b058a..8377cc8 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -100,6 +100,7 @@ class InitialLoadObserver::TabTime {
InitialLoadObserver::InitialLoadObserver(size_t tab_count,
AutomationProvider* automation)
: automation_(automation->AsWeakPtr()),
+ crashed_tab_count_(0),
outstanding_tab_count_(tab_count),
init_time_(base::TimeTicks::Now()) {
if (outstanding_tab_count_ > 0) {
@@ -107,6 +108,8 @@ InitialLoadObserver::InitialLoadObserver(size_t tab_count,
NotificationService::AllSources());
registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
NotificationService::AllSources());
+ registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+ NotificationService::AllSources());
}
}
@@ -128,12 +131,37 @@ void InitialLoadObserver::Observe(int type,
finished_tabs_.insert(source.map_key());
iter->second.set_stop_time(base::TimeTicks::Now());
}
- if (outstanding_tab_count_ == finished_tabs_.size())
- ConditionMet();
+ }
+ } else if (type == content::NOTIFICATION_RENDERER_PROCESS_CLOSED) {
+ base::TerminationStatus status =
+ Details<RenderProcessHost::RendererClosedDetails>(details)->status;
+ switch (status) {
+ case base::TERMINATION_STATUS_NORMAL_TERMINATION:
+ break;
+
+ case base::TERMINATION_STATUS_ABNORMAL_TERMINATION:
+ case base::TERMINATION_STATUS_PROCESS_WAS_KILLED:
+ case base::TERMINATION_STATUS_PROCESS_CRASHED:
+ crashed_tab_count_++;
+ break;
+
+ case base::TERMINATION_STATUS_STILL_RUNNING:
+ LOG(ERROR) << "Got RENDERER_PROCESS_CLOSED notification, "
+ << "but the process is still running. We may miss further "
+ << "crash notification, resulting in hangs.";
+ break;
+
+ default:
+ LOG(ERROR) << "Unhandled termination status " << status;
+ NOTREACHED();
+ break;
}
} else {
NOTREACHED();
}
+
+ if (finished_tabs_.size() + crashed_tab_count_ >= outstanding_tab_count_)
+ ConditionMet();
}
DictionaryValue* InitialLoadObserver::GetTimingInformation() const {
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index 4aa04b0..c65d208 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -109,6 +109,7 @@ class InitialLoadObserver : public NotificationObserver {
NotificationRegistrar registrar_;
base::WeakPtr<AutomationProvider> automation_;
+ size_t crashed_tab_count_;
size_t outstanding_tab_count_;
base::TimeTicks init_time_;
TabTimeMap loading_tabs_;
diff --git a/chrome/browser/importer/firefox_importer_unittest_utils_mac.cc b/chrome/browser/importer/firefox_importer_unittest_utils_mac.cc
index 09a29a4..83d5283 100644
--- a/chrome/browser/importer/firefox_importer_unittest_utils_mac.cc
+++ b/chrome/browser/importer/firefox_importer_unittest_utils_mac.cc
@@ -7,6 +7,7 @@
#include "base/base_switches.h"
#include "base/command_line.h"
#include "base/file_path.h"
+#include "base/file_util.h"
#include "base/message_loop.h"
#include "base/test/test_timeouts.h"
#include "chrome/browser/importer/firefox_importer_utils.h"
@@ -30,7 +31,7 @@ const char kTestChannelID[] = "T1";
// |handle| - On return, the process handle to use to communicate with the
// child.
bool LaunchNSSDecrypterChildProcess(const FilePath& nss_path,
- const IPC::Channel& channel, base::ProcessHandle* handle) {
+ IPC::Channel* channel, base::ProcessHandle* handle) {
CommandLine cl(*CommandLine::ForCurrentProcess());
cl.AppendSwitchASCII(switches::kTestChildProcess, "NSSDecrypterChildProcess");
@@ -43,13 +44,13 @@ bool LaunchNSSDecrypterChildProcess(const FilePath& nss_path,
dyld_override.second = nss_path.value();
env.push_back(dyld_override);
- base::file_handle_mapping_vector fds_to_map;
- const int ipcfd = channel.GetClientFileDescriptor();
- if (ipcfd > -1) {
- fds_to_map.push_back(std::pair<int,int>(ipcfd, kPrimaryIPCChannel + 3));
- } else {
+ int ipcfd = channel->TakeClientFileDescriptor();
+ if (ipcfd == -1)
return false;
- }
+
+ file_util::ScopedFD client_file_descriptor_closer(&ipcfd);
+ base::file_handle_mapping_vector fds_to_map;
+ fds_to_map.push_back(std::pair<int,int>(ipcfd, kPrimaryIPCChannel + 3));
bool debug_on_start = CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDebugChildren);
@@ -139,7 +140,7 @@ bool FFUnitTestDecryptorProxy::Setup(const FilePath& nss_path) {
// Spawn child and set up sync IPC connection.
bool ret = LaunchNSSDecrypterChildProcess(nss_path,
- *(channel_.get()),
+ channel_.get(),
&child_process_);
return ret && (child_process_ != 0);
}
diff --git a/chrome/common/logging_chrome_uitest.cc b/chrome/common/logging_chrome_uitest.cc
index b72661a..d4bbfa1 100644
--- a/chrome/common/logging_chrome_uitest.cc
+++ b/chrome/common/logging_chrome_uitest.cc
@@ -84,16 +84,14 @@ TEST_F(ChromeLoggingTest, EnvironmentLogFileName) {
#endif
#if !defined(NDEBUG) // We don't have assertions in release builds.
-// Tests whether we correctly fail on browser assertions during tests.
+// Tests whether we correctly fail on renderer assertions during tests.
class AssertionTest : public UITest {
protected:
- AssertionTest() : UITest() {
- // Initial loads will never complete due to assertion.
+ AssertionTest() {
+#if defined(OS_WIN)
+ // TODO(phajdan.jr): Make crash notifications on launch work on Win.
wait_for_initial_loads_ = false;
-
- // We're testing the renderer rather than the browser assertion here,
- // because the browser assertion would flunk the test during SetUp()
- // (since TAU wouldn't be able to find the browser window).
+#endif
launch_arguments_.AppendSwitch(switches::kRendererAssertTest);
}
};
@@ -116,13 +114,11 @@ TEST_F(AssertionTest, Assertion) {
// Only works on Linux in Release mode with CHROME_HEADLESS=1
class CheckFalseTest : public UITest {
protected:
- CheckFalseTest() : UITest() {
- // Initial loads will never complete due to assertion.
+ CheckFalseTest() {
+#if defined(OS_WIN)
+ // TODO(phajdan.jr): Make crash notifications on launch work on Win.
wait_for_initial_loads_ = false;
-
- // We're testing the renderer rather than the browser assertion here,
- // because the browser assertion would flunk the test during SetUp()
- // (since TAU wouldn't be able to find the browser window).
+#endif
launch_arguments_.AppendSwitch(switches::kRendererCheckFalseTest);
}
};
@@ -144,10 +140,11 @@ TEST_F(CheckFalseTest, CheckFails) {
// Tests whether we correctly fail on browser crashes during UI Tests.
class RendererCrashTest : public UITest {
protected:
- RendererCrashTest() : UITest() {
- // Initial loads will never complete due to crash.
+ RendererCrashTest() {
+#if defined(OS_WIN)
+ // TODO(phajdan.jr): Make crash notifications on launch work on Win.
wait_for_initial_loads_ = false;
-
+#endif
launch_arguments_.AppendSwitch(switches::kRendererCrashTest);
}
};
diff --git a/chrome/test/automation/automation_proxy.cc b/chrome/test/automation/automation_proxy.cc
index 4704deb..10bcf6d 100644
--- a/chrome/test/automation/automation_proxy.cc
+++ b/chrome/test/automation/automation_proxy.cc
@@ -432,15 +432,9 @@ scoped_refptr<BrowserProxy> AutomationProxy::GetLastActiveBrowserWindow() {
return ProxyObjectFromHandle<BrowserProxy>(handle);
}
-#if defined(OS_POSIX)
-base::file_handle_mapping_vector AutomationProxy::fds_to_map() const {
- base::file_handle_mapping_vector map;
- const int ipcfd = channel_->GetClientFileDescriptor();
- if (ipcfd > -1)
- map.push_back(std::make_pair(ipcfd, kPrimaryIPCChannel + 3));
- return map;
-}
-#endif // defined(OS_POSIX)
+IPC::SyncChannel* AutomationProxy::channel() {
+ return channel_.get();
+}
bool AutomationProxy::Send(IPC::Message* message) {
return Send(message,
diff --git a/chrome/test/automation/automation_proxy.h b/chrome/test/automation/automation_proxy.h
index a41689d..0fccd90 100644
--- a/chrome/test/automation/automation_proxy.h
+++ b/chrome/test/automation/automation_proxy.h
@@ -222,9 +222,7 @@ class AutomationProxy : public IPC::Channel::Listener,
const std::string& password) WARN_UNUSED_RESULT;
#endif
-#if defined(OS_POSIX)
- base::file_handle_mapping_vector fds_to_map() const;
-#endif
+ IPC::SyncChannel* channel();
// AutomationMessageSender implementation.
virtual bool Send(IPC::Message* message) WARN_UNUSED_RESULT;
diff --git a/chrome/test/automation/proxy_launcher.cc b/chrome/test/automation/proxy_launcher.cc
index 34c4060..469a0f2 100644
--- a/chrome/test/automation/proxy_launcher.cc
+++ b/chrome/test/automation/proxy_launcher.cc
@@ -26,6 +26,7 @@
#include "content/common/child_process_info.h"
#include "content/common/debug_flags.h"
#include "ipc/ipc_channel.h"
+#include "ipc/ipc_descriptors.h"
#include "sql/connection.h"
namespace {
@@ -109,11 +110,11 @@ bool ProxyLauncher::WaitForBrowserLaunch(bool wait_for_initial_loads) {
return false;
}
} else {
- // TODO(phajdan.jr): We should get rid of this sleep, but some tests
- // "rely" on it, e.g. AssertionTest.Assertion and CheckFalseTest.CheckFails.
- // Those tests do not wait in any way until the crash gets noticed,
- // so it's possible for the browser to exit before the tested crash happens.
+#if defined(OS_WIN)
+ // TODO(phajdan.jr): Get rid of this Sleep when logging_chrome_uitest
+ // stops "relying" on it.
base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms());
+#endif
}
if (!automation()->SetFilteredInet(ShouldFilterInet())) {
@@ -198,7 +199,7 @@ bool ProxyLauncher::LaunchBrowser(const LaunchState& state) {
if (!state.setup_profile_callback.is_null())
state.setup_profile_callback.Run();
- if (!LaunchBrowserHelper(state, false, &process_)) {
+ if (!LaunchBrowserHelper(state, true, false, &process_)) {
LOG(ERROR) << "LaunchBrowserHelper failed.";
return false;
}
@@ -210,7 +211,7 @@ bool ProxyLauncher::LaunchBrowser(const LaunchState& state) {
#if !defined(OS_MACOSX)
bool ProxyLauncher::LaunchAnotherBrowserBlockUntilClosed(
const LaunchState& state) {
- return LaunchBrowserHelper(state, true, NULL);
+ return LaunchBrowserHelper(state, false, true, NULL);
}
#endif
@@ -435,7 +436,9 @@ void ProxyLauncher::PrepareTestCommandline(CommandLine* command_line,
command_line->AppendSwitch(switches::kUnlimitedQuotaForFiles);
}
-bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
+bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state,
+ bool main_launch,
+ bool wait,
base::ProcessHandle* process) {
CommandLine command_line(state.command);
@@ -459,8 +462,8 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
<< browser_wrapper;
}
- // TODO(phajdan.jr): Only run it for "main" browser launch.
- browser_launch_time_ = base::TimeTicks::Now();
+ if (main_launch)
+ browser_launch_time_ = base::TimeTicks::Now();
base::LaunchOptions options;
options.wait = wait;
@@ -468,10 +471,14 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
#if defined(OS_WIN)
options.start_hidden = !state.show_window;
#elif defined(OS_POSIX)
+ int ipcfd = -1;
+ file_util::ScopedFD ipcfd_closer(&ipcfd);
base::file_handle_mapping_vector fds;
- if (automation_proxy_.get())
- fds = automation_proxy_->fds_to_map();
- options.fds_to_remap = &fds;
+ if (main_launch && automation_proxy_.get()) {
+ ipcfd = automation_proxy_->channel()->TakeClientFileDescriptor();
+ fds.push_back(std::make_pair(ipcfd, kPrimaryIPCChannel + 3));
+ options.fds_to_remap = &fds;
+ }
#endif
return base::LaunchProcess(command_line, options, process);
diff --git a/chrome/test/automation/proxy_launcher.h b/chrome/test/automation/proxy_launcher.h
index 0e497b7..64f5b2b 100644
--- a/chrome/test/automation/proxy_launcher.h
+++ b/chrome/test/automation/proxy_launcher.h
@@ -166,6 +166,7 @@ class ProxyLauncher {
bool include_testing_id);
bool LaunchBrowserHelper(const LaunchState& state,
+ bool main_launch,
bool wait,
base::ProcessHandle* process) WARN_UNUSED_RESULT;
diff --git a/content/browser/browser_child_process_host.cc b/content/browser/browser_child_process_host.cc
index 184ac82..d3aaac7 100644
--- a/content/browser/browser_child_process_host.cc
+++ b/content/browser/browser_child_process_host.cc
@@ -94,7 +94,7 @@ void BrowserChildProcessHost::Launch(
#elif defined(OS_POSIX)
use_zygote,
environ,
- channel()->GetClientFileDescriptor(),
+ channel()->TakeClientFileDescriptor(),
#endif
cmd_line,
&client_));
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index c3d32a0..e438989 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -7,6 +7,7 @@
#include <utility> // For std::pair.
#include "base/command_line.h"
+#include "base/file_util.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "base/synchronization/lock.h"
@@ -115,10 +116,14 @@ class ChildProcessLauncher::Context
#endif
CommandLine* cmd_line) {
scoped_ptr<CommandLine> cmd_line_deleter(cmd_line);
+
base::ProcessHandle handle = base::kNullProcessHandle;
#if defined(OS_WIN)
handle = sandbox::StartProcessWithAccess(cmd_line, exposed_dir);
#elif defined(OS_POSIX)
+ // We need to close the client end of the IPC channel
+ // to reliably detect child termination.
+ file_util::ScopedFD ipcfd_closer(&ipcfd);
#if defined(OS_POSIX) && !defined(OS_MACOSX)
// On Linux, we need to add some extra file descriptors for crash handling.
diff --git a/content/browser/renderer_host/browser_render_process_host.cc b/content/browser/renderer_host/browser_render_process_host.cc
index 8c3034c..a500892 100644
--- a/content/browser/renderer_host/browser_render_process_host.cc
+++ b/content/browser/renderer_host/browser_render_process_host.cc
@@ -330,7 +330,7 @@ bool BrowserRenderProcessHost::Init(bool is_accessibility_enabled) {
#elif defined(OS_POSIX)
renderer_prefix.empty(),
base::environment_vector(),
- channel_->GetClientFileDescriptor(),
+ channel_->TakeClientFileDescriptor(),
#endif
cmd_line,
this));
@@ -940,6 +940,7 @@ void BrowserRenderProcessHost::OnProcessLaunched() {
OnChannelError();
return;
}
+
child_process_launcher_->SetProcessBackgrounded(backgrounded_);
}
diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc
index 0ad31501..e51d220 100644
--- a/content/common/gpu/gpu_channel.cc
+++ b/content/common/gpu/gpu_channel.cc
@@ -396,11 +396,11 @@ std::string GpuChannel::GetChannelName() {
}
#if defined(OS_POSIX)
-int GpuChannel::GetRendererFileDescriptor() {
- int fd = -1;
- if (channel_.get()) {
- fd = channel_->GetClientFileDescriptor();
+int GpuChannel::TakeRendererFileDescriptor() {
+ if (!channel_.get()) {
+ NOTREACHED();
+ return -1;
}
- return fd;
+ return channel_->TakeClientFileDescriptor();
}
#endif // defined(OS_POSIX)
diff --git a/content/common/gpu/gpu_channel.h b/content/common/gpu/gpu_channel.h
index b6a510f..d51c0bc 100644
--- a/content/common/gpu/gpu_channel.h
+++ b/content/common/gpu/gpu_channel.h
@@ -62,7 +62,7 @@ class GpuChannel : public IPC::Channel::Listener,
std::string GetChannelName();
#if defined(OS_POSIX)
- int GetRendererFileDescriptor();
+ int TakeRendererFileDescriptor();
#endif // defined(OS_POSIX)
base::ProcessHandle renderer_process() const {
diff --git a/content/common/gpu/gpu_channel_manager.cc b/content/common/gpu/gpu_channel_manager.cc
index 900e6eb..5d3aba8 100644
--- a/content/common/gpu/gpu_channel_manager.cc
+++ b/content/common/gpu/gpu_channel_manager.cc
@@ -102,9 +102,9 @@ void GpuChannelManager::OnEstablishChannel(int renderer_id) {
#if defined(OS_POSIX)
// On POSIX, pass the renderer-side FD. Also mark it as auto-close so
// that it gets closed after it has been sent.
- int renderer_fd = channel->GetRendererFileDescriptor();
+ int renderer_fd = channel->TakeRendererFileDescriptor();
DCHECK_NE(-1, renderer_fd);
- channel_handle.socket = base::FileDescriptor(dup(renderer_fd), true);
+ channel_handle.socket = base::FileDescriptor(renderer_fd, true);
#endif
}
diff --git a/content/plugin/plugin_channel.h b/content/plugin/plugin_channel.h
index 4439920..6a5bb74 100644
--- a/content/plugin/plugin_channel.h
+++ b/content/plugin/plugin_channel.h
@@ -51,7 +51,9 @@ class PluginChannel : public PluginChannelBase {
void set_incognito(bool value) { incognito_ = value; }
#if defined(OS_POSIX)
- int renderer_fd() const { return channel_->GetClientFileDescriptor(); }
+ int TakeRendererFileDescriptor() {
+ return channel_->TakeClientFileDescriptor();
+ }
#endif
protected:
diff --git a/content/plugin/plugin_thread.cc b/content/plugin/plugin_thread.cc
index 6bd9b90..75252c6 100644
--- a/content/plugin/plugin_thread.cc
+++ b/content/plugin/plugin_thread.cc
@@ -159,7 +159,8 @@ void PluginThread::OnCreateChannel(int renderer_id,
channel_handle.name = channel->channel_handle().name;
#if defined(OS_POSIX)
// On POSIX, pass the renderer-side FD.
- channel_handle.socket = base::FileDescriptor(channel->renderer_fd(), false);
+ channel_handle.socket =
+ base::FileDescriptor(channel->TakeRendererFileDescriptor(), true);
#endif
channel->set_incognito(incognito);
}
diff --git a/ipc/ipc_channel.h b/ipc/ipc_channel.h
index 8bb35c4..718f2b3 100644
--- a/ipc/ipc_channel.h
+++ b/ipc/ipc_channel.h
@@ -146,8 +146,14 @@ class IPC_EXPORT Channel : public Message::Sender {
// On POSIX an IPC::Channel wraps a socketpair(), this method returns the
// FD # for the client end of the socket.
// This method may only be called on the server side of a channel.
+ // This method can be called on any thread.
int GetClientFileDescriptor() const;
+ // Same as GetClientFileDescriptor, but transfers the ownership of the
+ // file descriptor to the caller.
+ // This method can be called on any thread.
+ int TakeClientFileDescriptor();
+
// On POSIX an IPC::Channel can either wrap an established socket, or it
// can wrap a socket that is listening for connections. Currently an
// IPC::Channel that listens for connections can only accept one connection
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index a88506c..27c62f5 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -102,15 +102,9 @@ class PipeMap {
// Remove the mapping for the given channel id. No error is signaled if the
// channel_id doesn't exist
- void RemoveAndClose(const std::string& channel_id) {
+ void Remove(const std::string& channel_id) {
base::AutoLock locked(lock_);
-
- ChannelToFDMap::iterator i = map_.find(channel_id);
- if (i != map_.end()) {
- if (HANDLE_EINTR(close(i->second)) < 0)
- PLOG(ERROR) << "close " << channel_id;
- map_.erase(i);
- }
+ map_.erase(channel_id);
}
// Insert a mapping from @channel_id to @fd. It's a fatal error to insert a
@@ -403,7 +397,7 @@ bool Channel::ChannelImpl::CreatePipe(
// Case 3 from comment above.
// We only allow one connection.
local_pipe = HANDLE_EINTR(dup(local_pipe));
- PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
+ PipeMap::GetInstance()->Remove(pipe_name_);
} else {
// Case 4a from comment above.
// Guard against inappropriate reuse of the initial IPC channel. If
@@ -426,6 +420,7 @@ bool Channel::ChannelImpl::CreatePipe(
LOG(ERROR) << "Server already exists for " << pipe_name_;
return false;
}
+ base::AutoLock lock(client_pipe_lock_);
if (!SocketPair(&local_pipe, &client_pipe_))
return false;
PipeMap::GetInstance()->Insert(pipe_name_, client_pipe_);
@@ -529,10 +524,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
}
DCHECK(bytes_read);
- if (client_pipe_ != -1) {
- PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
- client_pipe_ = -1;
- }
+ CloseClientFileDescriptor();
// a pointer to an array of |num_wire_fds| file descriptors from the read
const int* wire_fds = NULL;
@@ -916,10 +908,31 @@ bool Channel::ChannelImpl::Send(Message* message) {
return true;
}
-int Channel::ChannelImpl::GetClientFileDescriptor() const {
+int Channel::ChannelImpl::GetClientFileDescriptor() {
+ base::AutoLock lock(client_pipe_lock_);
return client_pipe_;
}
+int Channel::ChannelImpl::TakeClientFileDescriptor() {
+ base::AutoLock lock(client_pipe_lock_);
+ int fd = client_pipe_;
+ if (client_pipe_ != -1) {
+ PipeMap::GetInstance()->Remove(pipe_name_);
+ client_pipe_ = -1;
+ }
+ return fd;
+}
+
+void Channel::ChannelImpl::CloseClientFileDescriptor() {
+ base::AutoLock lock(client_pipe_lock_);
+ if (client_pipe_ != -1) {
+ PipeMap::GetInstance()->Remove(pipe_name_);
+ if (HANDLE_EINTR(close(client_pipe_)) < 0)
+ PLOG(ERROR) << "close " << pipe_name_;
+ client_pipe_ = -1;
+ }
+}
+
bool Channel::ChannelImpl::AcceptsConnections() const {
return server_listen_pipe_ != -1;
}
@@ -1173,10 +1186,7 @@ void Channel::ChannelImpl::Close() {
server_listen_connection_watcher_.StopWatchingFileDescriptor();
}
- if (client_pipe_ != -1) {
- PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
- client_pipe_ = -1;
- }
+ CloseClientFileDescriptor();
}
//------------------------------------------------------------------------------
@@ -1210,6 +1220,10 @@ int Channel::GetClientFileDescriptor() const {
return channel_impl_->GetClientFileDescriptor();
}
+int Channel::TakeClientFileDescriptor() {
+ return channel_impl_->TakeClientFileDescriptor();
+}
+
bool Channel::AcceptsConnections() const {
return channel_impl_->AcceptsConnections();
}
diff --git a/ipc/ipc_channel_posix.h b/ipc/ipc_channel_posix.h
index 06c545c..e141998 100644
--- a/ipc/ipc_channel_posix.h
+++ b/ipc/ipc_channel_posix.h
@@ -57,7 +57,9 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
void Close();
void set_listener(Listener* listener) { listener_ = listener; }
bool Send(Message* message);
- int GetClientFileDescriptor() const;
+ int GetClientFileDescriptor();
+ int TakeClientFileDescriptor();
+ void CloseClientFileDescriptor();
bool AcceptsConnections() const;
bool HasAcceptedConnection() const;
bool GetClientEuid(uid_t* client_euid) const;
@@ -109,6 +111,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
// For a server, the client end of our socketpair() -- the other end of our
// pipe_ that is passed to the client.
int client_pipe_;
+ base::Lock client_pipe_lock_; // Lock that protects |client_pipe_|.
#if defined(IPC_USES_READWRITE)
// Linux/BSD use a dedicated socketpair() for passing file descriptors.
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index dc990c2..fa4d69e 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -376,16 +376,22 @@ void ChannelProxy::ClearIPCMessageLoop() {
#if defined(OS_POSIX) && !defined(OS_NACL)
// See the TODO regarding lazy initialization of the channel in
// ChannelProxy::Init().
-// We assume that IPC::Channel::GetClientFileDescriptorMapping() is thread-safe.
-int ChannelProxy::GetClientFileDescriptor() const {
- Channel *channel = context_.get()->channel_.get();
+int ChannelProxy::GetClientFileDescriptor() {
+ Channel* channel = context_.get()->channel_.get();
// Channel must have been created first.
DCHECK(channel) << context_.get()->channel_id_;
return channel->GetClientFileDescriptor();
}
+int ChannelProxy::TakeClientFileDescriptor() {
+ Channel* channel = context_.get()->channel_.get();
+ // Channel must have been created first.
+ DCHECK(channel) << context_.get()->channel_id_;
+ return channel->TakeClientFileDescriptor();
+}
+
bool ChannelProxy::GetClientEuid(uid_t* client_euid) const {
- Channel *channel = context_.get()->channel_.get();
+ Channel* channel = context_.get()->channel_.get();
// Channel must have been created first.
DCHECK(channel) << context_.get()->channel_id_;
return channel->GetClientEuid(client_euid);
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 792e3d6..4e2ebbf 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -157,7 +157,8 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
#if defined(OS_POSIX)
// Calls through to the underlying channel's methods.
- int GetClientFileDescriptor() const;
+ int GetClientFileDescriptor();
+ int TakeClientFileDescriptor();
bool GetClientEuid(uid_t* client_euid) const;
#endif // defined(OS_POSIX)