From 891ccf8ab73ea3f68852b9544d2db90ad70a9d65 Mon Sep 17 00:00:00 2001 From: "hubbe@chromium.org" Date: Thu, 30 Jan 2014 23:00:38 +0000 Subject: Revert of Fix posix IPC channel hanging problem. (https://codereview.chromium.org/30133002/) Reason for revert: Seems to cause some leaks. Reverting while I investigate. http://build.chromium.org/p/chromium.memory/builders/Linux%20ASAN%20Tests%20%282%29/builds/22398 Original issue's description: > Fix posix IPC channel hanging problem. > > If a channel closes right before a send call, listeners might not be notified of > the problem, which can cause hangs. This CL fixes that and adds a test that makes > sure that this does not happen in the future. > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248060 TBR=cpu@chromium.org,shess@chromium.org,hubbe@google.com NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/150883002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@248067 0039d316-1c4b-4281-b951-d872f2087c98 --- ipc/ipc_channel_posix.cc | 17 +++---------- ipc/ipc_channel_posix_unittest.cc | 53 +++------------------------------------ 2 files changed, 6 insertions(+), 64 deletions(-) (limited to 'ipc') diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc index e539d4f..8788532 100644 --- a/ipc/ipc_channel_posix.cc +++ b/ipc/ipc_channel_posix.cc @@ -31,7 +31,6 @@ #include "base/posix/global_descriptors.h" #include "base/process/process_handle.h" #include "base/rand_util.h" -#include "base/run_loop.h" #include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/synchronization/lock.h" @@ -328,7 +327,7 @@ bool Channel::ChannelImpl::CreatePipe( bool Channel::ChannelImpl::Connect() { if (server_listen_pipe_ == -1 && pipe_ == -1) { - DLOG(WARNING) << "Channel creation failed: " << pipe_name_; + DLOG(INFO) << "Channel creation failed: " << pipe_name_; return false; } @@ -516,9 +515,6 @@ bool Channel::ChannelImpl::Send(Message* message) { << " with type " << message->type() << " (" << output_queue_.size() << " in queue)"; - if (!waiting_connect_ && pipe_ == -1) - return false; - #ifdef IPC_MESSAGE_LOG_ENABLED Logging::GetInstance()->OnSendMessage(message, ""); #endif // IPC_MESSAGE_LOG_ENABLED @@ -526,10 +522,7 @@ bool Channel::ChannelImpl::Send(Message* message) { message->TraceMessageBegin(); output_queue_.push(message); if (!is_blocked_on_write_ && !waiting_connect_) { - if (!ProcessOutgoingMessages()) { - ClosePipeOnError(); - return false; - } + return ProcessOutgoingMessages(); } return true; @@ -714,11 +707,7 @@ bool Channel::ChannelImpl::AcceptConnection() { // In server mode we will send a hello message when we receive one from a // client. waiting_connect_ = false; - if (!ProcessOutgoingMessages()) { - ClosePipeOnError(); - return false; - } - return true; + return ProcessOutgoingMessages(); } else if (mode_ & MODE_SERVER_FLAG) { waiting_connect_ = true; return true; diff --git a/ipc/ipc_channel_posix_unittest.cc b/ipc/ipc_channel_posix_unittest.cc index fa983f6..dbd854e 100644 --- a/ipc/ipc_channel_posix_unittest.cc +++ b/ipc/ipc_channel_posix_unittest.cc @@ -41,9 +41,7 @@ class IPCChannelPosixTestListener : public IPC::Listener { }; IPCChannelPosixTestListener(bool quit_only_on_message) - : status_(DISCONNECTED), - quit_only_on_message_(quit_only_on_message) { - } + : status_(DISCONNECTED), quit_only_on_message_(quit_only_on_message) {} virtual ~IPCChannelPosixTestListener() {} @@ -85,13 +83,7 @@ class IPCChannelPosixTestListener : public IPC::Listener { STATUS status() { return status_; } void QuitRunLoop() { - base::MessageLoopForIO* loop = base::MessageLoopForIO::current(); - if (loop->is_running()) { - loop->QuitNow(); - } else { - // Die as soon as Run is called. - loop->PostTask(FROM_HERE, loop->QuitClosure()); - } + base::MessageLoopForIO::current()->QuitNow(); } private: @@ -194,7 +186,7 @@ void IPCChannelPosixTest::SpinRunLoop(base::TimeDelta delay) { // in the case of a bad test. Usually, the run loop will quit sooner than // that because all tests use a IPCChannelPosixTestListener which quits the // current run loop on any channel activity. - loop->PostDelayedTask(FROM_HERE, loop->QuitClosure(), delay); + loop->PostDelayedTask(FROM_HERE, base::MessageLoop::QuitClosure(), delay); loop->Run(); } @@ -236,45 +228,6 @@ TEST_F(IPCChannelPosixTest, BasicConnected) { ASSERT_FALSE(channel2.AcceptsConnections()); } -// If a connection closes right before a Send() call, we may end up closing -// the connection without notifying the listener, which can cause hangs in -// sync_message_filter and others. Make sure the listener is notified. -TEST_F(IPCChannelPosixTest, SendHangTest) { - IPCChannelPosixTestListener out_listener(true); - IPCChannelPosixTestListener in_listener(true); - IPC::ChannelHandle in_handle("IN"); - IPC::Channel in_chan(in_handle, IPC::Channel::MODE_SERVER, &in_listener); - base::FileDescriptor out_fd(in_chan.TakeClientFileDescriptor(), false); - IPC::ChannelHandle out_handle("OUT", out_fd); - IPC::Channel out_chan(out_handle, IPC::Channel::MODE_CLIENT, &out_listener); - ASSERT_TRUE(in_chan.Connect()); - ASSERT_TRUE(out_chan.Connect()); - in_chan.Close(); // simulate remote process dying at an unfortunate time. - // Send will fail, because it cannot write the message. - ASSERT_FALSE(out_chan.Send(new IPC::Message( - 0, // routing_id - kQuitMessage, // message type - IPC::Message::PRIORITY_NORMAL))); - ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, out_listener.status()); -} - -// If a connection closes right before a Connect() call, we may end up closing -// the connection without notifying the listener, which can cause hangs in -// sync_message_filter and others. Make sure the listener is notified. -TEST_F(IPCChannelPosixTest, AcceptHangTest) { - IPCChannelPosixTestListener out_listener(true); - IPCChannelPosixTestListener in_listener(true); - IPC::ChannelHandle in_handle("IN"); - IPC::Channel in_chan(in_handle, IPC::Channel::MODE_SERVER, &in_listener); - base::FileDescriptor out_fd(in_chan.TakeClientFileDescriptor(), false); - IPC::ChannelHandle out_handle("OUT", out_fd); - IPC::Channel out_chan(out_handle, IPC::Channel::MODE_CLIENT, &out_listener); - ASSERT_TRUE(in_chan.Connect()); - in_chan.Close(); // simulate remote process dying at an unfortunate time. - ASSERT_FALSE(out_chan.Connect()); - ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, out_listener.status()); -} - TEST_F(IPCChannelPosixTest, AdvancedConnected) { // Test creating a connection to an external process. IPCChannelPosixTestListener listener(false); -- cgit v1.1