From 3a2da9eb0deb330d7cd85bfc40f3761875e68b4e Mon Sep 17 00:00:00 2001
From: Pavel Labath <labath@google.com>
Date: Fri, 6 Feb 2015 11:32:52 +0000
Subject: [PATCH] Fix TestProcesslaunch regression caused by D7372

Summary:
After closing all the leaked file descriptors to the inferior tty, the following problem occured:
- when stdin, stdout and stderr are redirected, there are no slave descriptors open (which is good)
- lldb has a reader thread, which attempts to read from the master end of the tty
- this thread receives an EOF
- in response, it closes it's master end
- as this is the last open file descriptor for the master end, this deletes the tty and sends
  SIGHUP to the inferior (this is bad)

I fix this problem by making sure the master end remains open for the duration of the inferior
process by storing a copy of the file descriptor in ProcessMonitor. I create a copy to avoid
ownership issues with the reading thread.

Reviewers: ovyalov, emaste

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D7440

llvm-svn: 228391
---
 .../source/Plugins/Process/FreeBSD/ProcessMonitor.cpp |  9 ++++-----
 lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.h  |  3 ++-
 lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp  |  9 ++++-----
 lldb/source/Plugins/Process/Linux/ProcessMonitor.h    |  3 ++-
 lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp    | 11 ++++++++++-
 5 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
index d78d75090353..b33f83303971 100644
--- a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -1587,11 +1587,10 @@ ProcessMonitor::StopMonitor()
     StopOpThread();
     sem_destroy(&m_operation_pending);
     sem_destroy(&m_operation_done);
-
-    // Note: ProcessPOSIX passes the m_terminal_fd file descriptor to
-    // Process::SetSTDIOFileDescriptor, which in turn transfers ownership of
-    // the descriptor to a ConnectionFileDescriptor object.  Consequently
-    // even though still has the file descriptor, we shouldn't close it here.
+    if (m_terminal_fd >= 0) {
+        close(m_terminal_fd);
+        m_terminal_fd = -1;
+    }
 }
 
 // FIXME: On Linux, when a new thread is created, we receive to notifications,
diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.h b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.h
index 6cc7d63ff814..4ae963c89a2f 100644
--- a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.h
+++ b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.h
@@ -79,7 +79,8 @@ public:
     /// Reads from this file descriptor yield both the standard output and
     /// standard error of this debugee.  Even if stderr and stdout were
     /// redirected on launch it may still happen that data is available on this
-    /// descriptor (if the inferior process opens /dev/tty, for example).
+    /// descriptor (if the inferior process opens /dev/tty, for example). This descriptor is
+    /// closed after a call to StopMonitor().
     ///
     /// If this monitor was attached to an existing process this method returns
     /// -1.
diff --git a/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp b/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
index 81eb4d634a75..b2754bcc1a37 100644
--- a/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
+++ b/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp
@@ -2345,11 +2345,10 @@ ProcessMonitor::StopMonitor()
     StopOpThread();
     sem_destroy(&m_operation_pending);
     sem_destroy(&m_operation_done);
-
-    // Note: ProcessPOSIX passes the m_terminal_fd file descriptor to
-    // Process::SetSTDIOFileDescriptor, which in turn transfers ownership of
-    // the descriptor to a ConnectionFileDescriptor object.  Consequently
-    // even though still has the file descriptor, we shouldn't close it here.
+    if (m_terminal_fd >= 0) {
+        close(m_terminal_fd);
+        m_terminal_fd = -1;
+    }
 }
 
 void
diff --git a/lldb/source/Plugins/Process/Linux/ProcessMonitor.h b/lldb/source/Plugins/Process/Linux/ProcessMonitor.h
index 27beaef96067..0543e930d408 100644
--- a/lldb/source/Plugins/Process/Linux/ProcessMonitor.h
+++ b/lldb/source/Plugins/Process/Linux/ProcessMonitor.h
@@ -84,7 +84,8 @@ public:
     /// Reads from this file descriptor yield both the standard output and
     /// standard error of this debugee.  Even if stderr and stdout were
     /// redirected on launch it may still happen that data is available on this
-    /// descriptor (if the inferior process opens /dev/tty, for example).
+    /// descriptor (if the inferior process opens /dev/tty, for example). This descriptor is
+    /// closed after a call to StopMonitor().
     ///
     /// If this monitor was attached to an existing process this method returns
     /// -1.
diff --git a/lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp b/lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
index 0e5ab5a8d8b1..882fac75c9a0 100644
--- a/lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
+++ b/lldb/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
@@ -252,7 +252,16 @@ ProcessPOSIX::DoLaunch (Module *module,
     if (!error.Success())
         return error;
 
-    SetSTDIOFileDescriptor(m_monitor->GetTerminalFD());
+    int terminal = m_monitor->GetTerminalFD();
+    if (terminal >= 0) {
+        // The reader thread will close the file descriptor when done, so we pass it a copy.
+        int stdio = fcntl(terminal, F_DUPFD_CLOEXEC, 0);
+        if (stdio == -1) {
+            error.SetErrorToErrno();
+            return error;
+        }
+        SetSTDIOFileDescriptor(stdio);
+    }
 
     SetID(m_monitor->GetPID());
     return error;