MainLoop: avoid infinite loop when pty slave gets closed

[labath]

Summary

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).

[davide]

LGTM. @jingham ? I'll try this change on macOS to make sure it won't break anything.

[labath]

Darwin uses a kqueue-based implementation of this, so it definitely won't *break* anything, although I'd appreciate it if you can check whether the test passes there. My cursory reading of the kqueue man page makes me believe it should not be affected by this.

source/Host/common/MainLoop.cpp

#ifdef __ANDROID__
   std::vector<IOObject::WaitableHandle> fds;
   for (const auto &fd : loop.m_read_fds)
     if (FD_ISSET(fd.first, &read_fd_set))
       fds.push_back(fd.first);
 
   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
     if (loop.m_terminate_request)
       return;
 
     loop.ProcessReadObject(handle);
   }

unittests/Host/MainLoopTest.cpp

 //===-- MainLoopTest.cpp ----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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>
 
 using namespace lldb_private;
 
 namespace {
 class MainLoopTest : public testing::Test {
TEST_F(MainLoopTest, TerminatesImmediately) {
   auto handle1 = loop.RegisterReadObject(socketpair[1], make_callback(), error);
   ASSERT_TRUE(error.Success());
 
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
 
 #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;
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());
   kill(getpid(), SIGUSR1);
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
 #endif