diff options
author | Pavel Labath <labath@google.com> | 2015-02-06 11:32:52 +0000 |
---|---|---|
committer | Pavel Labath <labath@google.com> | 2015-02-06 11:32:52 +0000 |
commit | 3a2da9eb0deb330d7cd85bfc40f3761875e68b4e (patch) | |
tree | bf159f741e8aae5f8a396e54f8e38f5f9f52b379 | |
parent | 77f62d45ef61fb923636ceb3ecee42e490a56e1c (diff) | |
download | bcm5719-llvm-3a2da9eb0deb330d7cd85bfc40f3761875e68b4e.tar.gz bcm5719-llvm-3a2da9eb0deb330d7cd85bfc40f3761875e68b4e.zip |
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
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 d78d7509035..b33f8330397 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 6cc7d63ff81..4ae963c89a2 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 81eb4d634a7..b2754bcc1a3 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 27beaef9606..0543e930d40 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 0e5ab5a8d8b..882fac75c9a 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; |