[Lldb-commits] [PATCH] Fix TestProcesslaunch regression caused by D7372

[Greg Clayton]

lgtm

> On Feb 6, 2015, at 3:34 AM, Pavel Labath <labath at google.com> wrote:
> 
> REPOSITORY
>  rL LLVM
> 
> http://reviews.llvm.org/D7440
> 
> Files:
>  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
>  lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
>  lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp
>  lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h
>  lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> 
> Index: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp
> ===================================================================
> --- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp
> +++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp
> @@ -2345,11 +2345,10 @@
>     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
> Index: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h
> ===================================================================
> --- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h
> +++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h
> @@ -84,7 +84,8 @@
>     /// 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.
> Index: lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> ===================================================================
> --- lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> +++ lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> @@ -252,7 +252,16 @@
>     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;
> Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
> ===================================================================
> --- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
> +++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
> @@ -1587,11 +1587,10 @@
>     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,
> Index: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
> ===================================================================
> --- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
> +++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
> @@ -79,7 +79,8 @@
>     /// 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.
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D7440.19472.patch>