diff options
author | hubbe@chromium.org <hubbe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-18 17:42:20 +0000 |
---|---|---|
committer | hubbe@chromium.org <hubbe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-18 17:42:20 +0000 |
commit | 13ffc273c7279bf25c3428ca6e65b755018ddb9c (patch) | |
tree | b6b6059b2e2b33f7a6a728de0ed68330314f10af /ipc | |
parent | 46accd1f08351756b154330cd516f4182d33a0f1 (diff) | |
download | chromium_src-13ffc273c7279bf25c3428ca6e65b755018ddb9c.zip chromium_src-13ffc273c7279bf25c3428ca6e65b755018ddb9c.tar.gz chromium_src-13ffc273c7279bf25c3428ca6e65b755018ddb9c.tar.bz2 |
Revert of Fix posix IPC channel hanging problem. (https://codereview.chromium.org/150893002/)
Reason for revert:
https://crash.corp.google.com/samples?q=reportid=%2702eaaf63ed6fdda9%27
https://crash.corp.google.com/samples?q=reportid=%272ac5bc4dd7c49ced%27
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.
>
> This is cl/30133002 + a memory leak fix.
>
> BUG=338709
>
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=250964
TBR=shess@chromium.org,cpu@chromium.org,hubbe@google.com
NOTREECHECKS=true
NOTRY=true
BUG=338709
Review URL: https://codereview.chromium.org/170863002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251791 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ipc')
-rw-r--r-- | ipc/ipc_channel_posix.cc | 19 | ||||
-rw-r--r-- | ipc/ipc_channel_posix_unittest.cc | 53 |
2 files changed, 6 insertions, 66 deletions
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc index 09b72d5..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; } @@ -520,18 +519,10 @@ bool Channel::ChannelImpl::Send(Message* message) { Logging::GetInstance()->OnSendMessage(message, ""); #endif // IPC_MESSAGE_LOG_ENABLED - if (!waiting_connect_ && pipe_ == -1) { - delete message; - return false; - } - message->TraceMessageBegin(); output_queue_.push(message); if (!is_blocked_on_write_ && !waiting_connect_) { - if (!ProcessOutgoingMessages()) { - ClosePipeOnError(); - return false; - } + return ProcessOutgoingMessages(); } return true; @@ -716,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); |