[Lldb-commits] [PATCH] D41008: MainLoop: avoid infinite loop when pty slave gets closed

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.

For ptys (at least on Linux), the end-of-file (closing of the slave FD)
is signalled by the POLLHUP flag. We were ignoring this flag, which
meant that when this happened, we would spin in a loop, continuously
calling poll(2) and not making any progress.

This makes sure we treat POLLHUP as a read event (reading will return
0), and we call the registered callback when it happens. This is the
behavior our clients expect (and is consistent with how select(2)
works).


https://reviews.llvm.org/D41008

Files:
  source/Host/common/MainLoop.cpp
  unittests/Host/MainLoopTest.cpp


Index: unittests/Host/MainLoopTest.cpp
===================================================================
--- unittests/Host/MainLoopTest.cpp
+++ unittests/Host/MainLoopTest.cpp
@@ -8,6 +8,8 @@
 
//===----------------------------------------------------------------------===//
 
 #include "lldb/Host/MainLoop.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "gtest/gtest.h"
 #include <future>
@@ -107,6 +109,24 @@
 }
 
 #ifdef LLVM_ON_UNIX
+TEST_F(MainLoopTest, DetectsEOF) {
+  lldb_utility::PseudoTerminal term;
+  ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
+  ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
+  auto conn = llvm::make_unique<ConnectionFileDescriptor>(
+      term.ReleaseMasterFileDescriptor(), true);
+
+  Status error;
+  MainLoop loop;
+  auto handle =
+      loop.RegisterReadObject(conn->GetReadObject(), make_callback(), error);
+  ASSERT_TRUE(error.Success());
+  term.CloseSlaveFileDescriptor();
+
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(1u, callback_count);
+}
+
 TEST_F(MainLoopTest, Signal) {
   MainLoop loop;
   Status error;
Index: source/Host/common/MainLoop.cpp
===================================================================
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -221,7 +221,7 @@
   for (const auto &handle : fds) {
 #else
   for (const auto &fd : read_fds) {
-    if ((fd.revents & POLLIN) == 0)
+    if ((fd.revents & (POLLIN | POLLHUP)) == 0)
       continue;
     IOObject::WaitableHandle handle = fd.fd;
 #endif



Index: unittests/Host/MainLoopTest.cpp
===================================================================
--- unittests/Host/MainLoopTest.cpp
+++ unittests/Host/MainLoopTest.cpp
@@ -8,6 +8,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/MainLoop.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/common/TCPSocket.h"
 #include "gtest/gtest.h"
 #include <future>
@@ -107,6 +109,24 @@
 }
 
 #ifdef LLVM_ON_UNIX
+TEST_F(MainLoopTest, DetectsEOF) {
+  lldb_utility::PseudoTerminal term;
+  ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
+  ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
+  auto conn = llvm::make_unique<ConnectionFileDescriptor>(
+      term.ReleaseMasterFileDescriptor(), true);
+
+  Status error;
+  MainLoop loop;
+  auto handle =
+      loop.RegisterReadObject(conn->GetReadObject(), make_callback(), error);
+  ASSERT_TRUE(error.Success());
+  term.CloseSlaveFileDescriptor();
+
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(1u, callback_count);
+}
+
 TEST_F(MainLoopTest, Signal) {
   MainLoop loop;
   Status error;
Index: source/Host/common/MainLoop.cpp
===================================================================
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -221,7 +221,7 @@
   for (const auto &handle : fds) {
 #else
   for (const auto &fd : read_fds) {
-    if ((fd.revents & POLLIN) == 0)
+    if ((fd.revents & (POLLIN | POLLHUP)) == 0)
       continue;
     IOObject::WaitableHandle handle = fd.fd;
 #endif

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits