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 similar to cl/150893002, but takes a slightly different approach to how to
make sure everything happens in the right order. In particular, it avoids closing
the socket (and calling OnChannelError()) from Send().
BUG=338709
Review URL: https://codereview.chromium.org/172773002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252430 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 0e8a7916cb24dc7617d254b3d4bd48441fb7723a
diff --git a/ipc/ipc_channel_posix_unittest.cc b/ipc/ipc_channel_posix_unittest.cc
index dbd854e..5157a01 100644
--- a/ipc/ipc_channel_posix_unittest.cc
+++ b/ipc/ipc_channel_posix_unittest.cc
@@ -41,7 +41,9 @@
};
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() {}
@@ -61,9 +63,7 @@
virtual void OnChannelError() OVERRIDE {
status_ = CHANNEL_ERROR;
- if (!quit_only_on_message_) {
- QuitRunLoop();
- }
+ QuitRunLoop();
}
virtual void OnChannelDenied() OVERRIDE {
@@ -83,7 +83,13 @@
STATUS status() { return status_; }
void QuitRunLoop() {
- base::MessageLoopForIO::current()->QuitNow();
+ 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());
+ }
}
private:
@@ -186,7 +192,7 @@
// 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, base::MessageLoop::QuitClosure(), delay);
+ loop->PostDelayedTask(FROM_HERE, loop->QuitClosure(), delay);
loop->Run();
}
@@ -228,6 +234,47 @@
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)));
+ SpinRunLoop(TestTimeouts::action_max_timeout());
+ 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());
+ SpinRunLoop(TestTimeouts::action_max_timeout());
+ ASSERT_EQ(IPCChannelPosixTestListener::CHANNEL_ERROR, out_listener.status());
+}
+
TEST_F(IPCChannelPosixTest, AdvancedConnected) {
// Test creating a connection to an external process.
IPCChannelPosixTestListener listener(false);