[Lldb-commits] [lldb] r228391 - Fix TestProcesslaunch regression caused by D7372
Pavel Labath
labath at google.com
Fri Feb 6 03:32:53 PST 2015
Author: labath
Date: Fri Feb 6 05:32:52 2015
New Revision: 228391
URL: http://llvm.org/viewvc/llvm-project?rev=228391&view=rev
Log:
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
Modified:
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
Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp?rev=228391&r1=228390&r2=228391&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp Fri Feb 6 05:32:52 2015
@@ -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,
Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h?rev=228391&r1=228390&r2=228391&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h Fri Feb 6 05:32:52 2015
@@ -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.
Modified: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp?rev=228391&r1=228390&r2=228391&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp Fri Feb 6 05:32:52 2015
@@ -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
Modified: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h?rev=228391&r1=228390&r2=228391&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.h Fri Feb 6 05:32:52 2015
@@ -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.
Modified: lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp?rev=228391&r1=228390&r2=228391&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Process/POSIX/ProcessPOSIX.cpp Fri Feb 6 05:32:52 2015
@@ -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;
More information about the lldb-commits
mailing list