Fix TestProcesslaunch regression caused by D7372

[labath]

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.

[emaste]

Can you apply the ProcessMonitor.{cpp,h} changes to FreeBSD as well?

[labath]

Applying the changes to FreeBSD, as requested. Note that I currently have no means of testing
this. @emaste, could you verify?

[clayborg]

I am hitting accept as I can't verify any of this as we don't use Process monitor on MacOSX. I am good with this as long as all others are.

[emaste]

Applying the changes to FreeBSD, as requested. Note that I currently have no means of testing
this. @emaste, could you verify?

Thanks! I applied it locally and it works for me.

[ovyalov]

lgtm

[clayborg]

lgtm

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h

public:
     GetProcess() { return *m_process; }
 
     /// Returns a file descriptor to the controlling terminal of the inferior
     /// process.
     ///
     /// 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.
     int
     GetTerminalFD() const { return m_terminal_fd; }
 
     /// Reads @p size bytes from address @vm_adder in the inferior process
     /// address space.

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

 
 void
 ProcessMonitor::StopMonitor()
 {
     StopMonitoringChildProcess();
     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,
 // (1) a SIGTRAP|PTRACE_EVENT_CLONE from the main process thread with the
 // child thread id as additional information, and (2) a SIGSTOP|SI_USER from
 // the new child thread indicating that it has is stopped because we attached.
 // We have no guarantee of the order in which these arrive, but we need both
 // before we are ready to proceed.  We currently keep a list of threads which

lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h

public:
     GetProcess() { return *m_process; }
 
     /// Returns a file descriptor to the controlling terminal of the inferior
     /// process.
     ///
     /// 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.
     int
     GetTerminalFD() const { return m_terminal_fd; }
 
     /// Reads @p size bytes from address @vm_adder in the inferior process
     /// address space.

lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp

 
 void
 ProcessMonitor::StopMonitor()
 {
     StopMonitoringChildProcess();
     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
 ProcessMonitor::StopOpThread()
 {
     if (!m_operation_thread.IsJoinable())
         return;
 
     m_operation_thread.Cancel();
     m_operation_thread.Join(nullptr);
 }

lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp

m_monitor = new ProcessMonitor (this,
                                     launch_info,
                                     error);
 
     m_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;
 }
 
 void
 ProcessPOSIX::DidLaunch()
 {