[lldb-dev] [PATCH] Avoid leaking file descriptors.

Piotr Rak piotr.rak at gmail.com
Sat Mar 22 08:43:16 PDT 2014


Hi,

I've noticed that we leak file descriptors, since we don't use O_CLOEXEC
when opening files.
I've changed lldb_utility::PseudoTerminal, debugserver version of it and
lldb_private::File to set it while opening by default.

This can be disabled by defining:
LLDB_DISABLE_O_CLOEXEC and LLDB_DISABLE_DUPFD_CLOEXEC, currently not
integrated with any filesystem.
It also should compile if O_CLOEXEC and F_DUPFD_CLOEXEC aren't defined, and
give compilation warning.

Also added new File open option:
eOpenOptionInheritOnExecPosixFD that makes it fallback to old behavior,
however in whole code base it was not required, and could be removed...

I went pretty carefully through fork()/exec() and posix_spawn() codepaths
to ensure that everything is OK, and it looked it is, but might have missed
something.
None of them required changes, from what I have noticed, since use dup2 or
posix_spawnattr_add*.

If any of your out of tree code needs to be adjusted, PseudoTerminal::Fork
looks like good example how to pass fd without leaking.

I've tested linux amd64 and haven't noticed any regressions comparing to
trunk, but some tests fail in non-deterministic way, so could mask it...

Most notably:
TestStepNoDebug.py
TestCommandRegex.py

Managed to make them fail with and without the patch running in separate.

I've  possibly broke debugserver when WITH_SPRINGBOARD is defined for
launch code path.
I don't know SBSLaunchApplicationForDebugging semantics, and have no idea
how to fix it, beside disabling it for PseudoTerminal if this option is
defined.
Those changes are not strictly required for debugserver yet I have made
them for completeness.

Both raw diff and git patch are attached.

Please test attached, and report back if any problems are noticed.

Cheers,
/Piotr

PS.
@Greg, sorry that it took longer than I thought it will.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140322/98f18d46/attachment.html>
-------------- next part --------------
diff --git a/include/lldb/Host/File.h b/include/lldb/Host/File.h
index 814d960..4bcf494 100644
--- a/include/lldb/Host/File.h
+++ b/include/lldb/Host/File.h
@@ -17,6 +17,10 @@
 
 #include "lldb/lldb-private.h"
 
+namespace lldb_utility {
+class PseudoTerminal;
+}
+
 namespace lldb_private {
 
 //----------------------------------------------------------------------
@@ -41,7 +45,11 @@ public:
         eOpenOptionNonBlocking          = (1u << 4),    // File reads
         eOpenOptionCanCreate            = (1u << 5),    // Create file if doesn't already exist
         eOpenOptionCanCreateNewOnly     = (1u << 6),    // Can create file only if it doesn't already exist
-        eOpenoptionDontFollowSymlinks   = (1u << 7)
+        eOpenoptionDontFollowSymlinks   = (1u << 7),
+        eOpenOptionInheritOnExecPosixFD = (1u << 8)     // Enable inheritance of file descriptor on exec(),
+                                                        // ie. as O_CLOEXEC open flag was _NOT_ specified for POSIX
+                                                        // systems. NOTE: Use of this STRONGLY DISCOURAGED
+                                                        // see SetDescriptorInheritableAfterExec() for more details
     };
     
     static mode_t
@@ -69,8 +77,16 @@ public:
     {
     }
 
+    //------------------------------------------------------------------
+    // NOTE: Resulting File descriptor of this file won't be inheritable
+    // to child process after exec().
+    //------------------------------------------------------------------
     File (const File &rhs);
     
+    //------------------------------------------------------------------
+    // NOTE: Resulting File descriptor of this file won't be inheritable
+    // to child process after exec().
+    //------------------------------------------------------------------
     File &
     operator= (const File &rhs);
     //------------------------------------------------------------------
@@ -90,6 +106,15 @@ public:
     ///     Options to use when opening (see File::Permissions)
     ///
     /// @see File::Open (const char *path, uint32_t options, uint32_t permissions)
+    ///
+    /// NOTE: On POSIX systems by default file will be opened as O_CLOEXEC
+    /// open flag was specified, meaning it will be not inherited by child process
+    /// after calling exec().
+    /// This behavior is exactly oposite to default POSIX behavior and
+    /// to override it you can specify eOpenOptionInheritOnExecPosixFD
+    /// or call SetInheritableAfterExec(true) afterwards.
+    /// If this operation cannot be done in atomic manner it shall be emulated
+    /// by setting FD_CLOEXEC flag after open().
     //------------------------------------------------------------------
     File (const char *path,
           uint32_t options,
@@ -102,6 +127,7 @@ public:
     /// path. If \a path is not NULL or empty, this function will call
     /// File::Open (const char *path, uint32_t options, uint32_t permissions).
     ///
+    ///
     /// @param[in] path
     ///     The FileSpec for this file.
     ///
@@ -216,9 +242,39 @@ public:
     Error
     Close ();
     
+    //------------------------------------------------------------------
+    // NOTE: Resulting file descriptor of this file won't be inheritable
+    // by child process after exec().
+    //------------------------------------------------------------------
     Error
     Duplicate (const File &rhs);
 
+    //------------------------------------------------------------------
+    /// Like Duplicate() but sets file descriptor as inheritable after
+    /// child process exec().
+    ///
+    /// NOTE: Use of this method is STRONGLY DISCOURAGED.
+    /// @sa SetDescriptorInheritableAfterExec()
+    //------------------------------------------------------------------
+    Error
+    DuplicateInheritableAfterExec (const File &rhs);
+
+    //------------------------------------------------------------------
+    /// Makes file descriptor inheritable after process for child process
+    /// after exec call on POSIX systems.
+    ///
+    /// NOTE: Its use with inheritable is STRONGLY DISCOURAGED, unless
+    /// ensured that all other threads are stopped, and all of the
+    /// signals are blocked.
+    /// This is generaly impossibe to achive beside case when child
+    /// process after fork() is just about to call exec().
+    ///
+    ///
+    /// @sa eOpenOptionInheritOnExecPosixFD
+    //------------------------------------------------------------------
+    Error
+    SetDescriptorInheritableAfterExec (bool inheritable);
+
     int
     GetDescriptor() const;
 
@@ -516,6 +572,15 @@ public:
     {
         m_options = options;
     }
+private:
+    static int
+    OpenAndSetCloseOnExec (const char*, int oflags);
+
+    static int
+    OpenAndSetCloseOnExec (const char*, int oflags, mode_t mode);
+
+    friend class lldb_utility::PseudoTerminal; // for OpenAndSetCloseOnExec()
+
 protected:
     
     
diff --git a/source/Host/common/File.cpp b/source/Host/common/File.cpp
index 500b03c..b40e154 100644
--- a/source/Host/common/File.cpp
+++ b/source/Host/common/File.cpp
@@ -27,6 +27,29 @@
 #include "lldb/Host/Config.h"
 #include "lldb/Host/FileSpec.h"
 
+#if !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+#define LLDB_DISABLE_O_CLOEXEC 1
+
+#if !defined(_WIN32)
+// NOTE: For now we issue compile-time warning in that case, though, it will spam
+// future non-posix platforms builds and will require blacklisting them here.
+#warning "O_CLOEXEC not supported! This operation won't be atomic for open() in your build!"
+
+#endif // !defined(_WIN32)
+#endif // !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+
+
+#if !defined(LLDB_DISABLE_DUPFD_CLOEXEC) && !defined(F_DUPFD_CLOEXEC)
+#define LLDB_DISABLE_DUPFD_CLOEXEC
+
+#if !defined(_WIN32)
+// NOTE: For now we issue compile-time warning in that case, though, it will spam
+// future non-posix platforms builds and will require blacklisting them here.
+#warning "F_DUPFD_CLOEXEC not supported! This operation won't be atomic for open() in your build!"
+
+#endif // !defined(_WIN32)
+#endif // !defined(LLDB_DISABLE_DUPFD_CLOEXEC) && !defined(F_DUPFD_CLOEXEC)
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -73,6 +96,93 @@ GetStreamOpenModeFromOptions (uint32_t options)
     return NULL;
 }
 
+#if !defined(_WIN32)
+
+static int
+DoDuplicateSetFD (int file_descriptor)
+{
+    int result = 0;
+    int duplicated = ::fcntl(file_descriptor, F_DUPFD);
+
+    if (duplicated != -1)
+        result = ::fcntl(duplicated, F_SETFD, FD_CLOEXEC);
+
+    if (result == -1)
+    {
+        // Stash errno so it is not changed even if close fails...
+        int saved_errno = errno;
+        ::close(duplicated);
+        duplicated = -1;
+        errno = saved_errno;
+    }
+
+    return duplicated;
+}
+#endif // !defined(_WIN32)
+
+static int
+DoDuplicateFDCloseOnExec (int file_descriptor)
+{
+#if defined(_WIN32)
+    int duplicated = ::_dup(file_descriptor);
+
+#elif !defined(LLDB_DISABLE_DUPFD_CLOEXEC)
+    int duplicated = ::fcntl (file_descriptor, F_DUPFD_CLOEXEC);
+
+    if (duplicated == -1 && errno == EINVAL)
+    {
+        // If F_DUPFD_CLOEXEC is not supported by running system fallback
+        // to non-atomic F_SETFD
+        duplicated = DoDuplicateSetFD (file_descriptor);
+    }
+
+#else // !defined(_WIN32) && defined(LLDB_DISABLE_DUPFD_CLOEXEC)
+    int duplicated = DoDuplicateSetFD(file_descriptor);
+#endif // end !defined(_WIN32) && !defined(F_DUPFD_CLOEXEC)
+
+    return duplicated;
+}
+
+static int DoOpenSetFD (const char* path, int oflags, mode_t mode)
+{
+    int fd = ::open (path, oflags, mode);
+
+    if (fd >= 0)
+    {
+#ifndef _WIN32
+        if (::fcntl (fd, F_SETFD, FD_CLOEXEC) == -1)
+        {
+            int saved_errno = errno;
+            ::close(fd);
+            errno = saved_errno;
+            fd = -1;
+        }
+#endif
+    }
+
+    return fd;
+}
+
+static int DoOpenSetFD (const char* path, int oflags)
+{
+    int fd = ::open (path, oflags);
+
+    if (fd >= 0)
+    {
+#ifndef _WIN32
+        if (::fcntl (fd, F_SETFD, FD_CLOEXEC) == -1)
+        {
+            int saved_errno = errno;
+            ::close(fd);
+            errno = saved_errno;
+            fd = -1;
+        }
+#endif
+    }
+
+    return fd;
+}
+
 int File::kInvalidDescriptor = -1;
 FILE * File::kInvalidStream = NULL;
 
@@ -172,11 +282,7 @@ File::GetStream ()
                 {
                     // We must duplicate the file descriptor if we don't own it because
                     // when you call fdopen, the stream will own the fd
-#ifdef _WIN32
-                    m_descriptor = ::_dup(GetDescriptor());
-#else
-                    m_descriptor = ::fcntl(GetDescriptor(), F_DUPFD);
-#endif
+                    m_descriptor = DoDuplicateFDCloseOnExec(GetDescriptor());
                     m_own_descriptor = true;
                 }
 
@@ -213,16 +319,52 @@ Error
 File::Duplicate (const File &rhs)
 {
     Error error;
+
+    // FIXME: This closes file even if later fails leaving File state somewhere
+    // inbetween, should this operation really be destructive in such manner???
     if (IsValid ())
         Close();
 
     if (rhs.DescriptorIsValid())
     {
-#ifdef _WIN32
+        m_descriptor = DoDuplicateFDCloseOnExec(rhs.GetDescriptor());
+
+        if (!DescriptorIsValid())
+            error.SetErrorToErrno();
+        else
+        {
+            m_options = rhs.m_options;
+            m_own_descriptor = true;
+        }
+    }
+    else
+    {
+        error.SetErrorString ("invalid file to duplicate");
+    }
+    return error;
+}
+
+Error
+File::DuplicateInheritableAfterExec(const File &rhs)
+{
+    Error error;
+
+    // FIXME: This closes file even if later fails leaving File state somewhere
+    // inbetween, should this operation really be destructive in such manner?
+    if (IsValid ())
+        Close();
+
+    if (rhs.DescriptorIsValid())
+    {
+#if defined(_WIN32)
+
         m_descriptor = ::_dup(rhs.GetDescriptor());
-#else
+
+#else // !defined(_WIN32)
+
         m_descriptor = ::fcntl(rhs.GetDescriptor(), F_DUPFD);
-#endif
+
+#endif // end !defined(_WIN32)
         if (!DescriptorIsValid())
             error.SetErrorToErrno();
         else
@@ -239,6 +381,28 @@ File::Duplicate (const File &rhs)
 }
 
 Error
+File::SetDescriptorInheritableAfterExec (bool inheritable)
+{
+    Error error;
+
+
+    if (DescriptorIsValid())
+    {
+#ifndef _WIN32
+        int set_fd = inheritable? 0 : FD_CLOEXEC;
+
+        if (::fcntl(GetDescriptor(), F_SETFD, set_fd) == -1)
+            error.SetErrorToErrno();
+#endif
+    }
+    else
+    {
+        error.SetErrorString("invalid file descriptor");
+    }
+    return error;
+}
+
+Error
 File::Open (const char *path, uint32_t options, uint32_t permissions)
 {
     Error error;
@@ -248,6 +412,8 @@ File::Open (const char *path, uint32_t options, uint32_t permissions)
     int oflag = 0;
     const bool read = options & eOpenOptionRead;
     const bool write = options & eOpenOptionWrite;
+    const bool inherit_on_exec = options & eOpenOptionInheritOnExecPosixFD;
+
     if (write)
     {
         if (read)
@@ -280,10 +446,19 @@ File::Open (const char *path, uint32_t options, uint32_t permissions)
 #ifndef _WIN32
     if (options & eOpenOptionNonBlocking)
         oflag |= O_NONBLOCK;
+
 #else
     oflag |= O_BINARY;
 #endif
 
+// FIXME: we should probably be able enable this one even if build host doesn't
+// support O_CLOEXEC. Unfortunely it is really hard as it's value will differ
+// through platforms or even might differ between system versions.
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+    if (inherit_on_exec)
+        oflag |= O_CLOEXEC;
+#endif // !defined(LLDB_DISABLE_O_CLOEXEC)
+
     mode_t mode = 0;
     if (oflag & O_CREAT)
     {
@@ -300,7 +475,22 @@ File::Open (const char *path, uint32_t options, uint32_t permissions)
 
     do
     {
+
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
         m_descriptor = ::open(path, oflag, mode);
+
+        // If system we are running doesn't support O_CLOEXEC, lets emulate it.
+        if (!inherit_on_exec &&
+            m_descriptor < 0 &&
+            errno == EINVAL)
+        {
+            oflag &= ~static_cast<int>(O_CLOEXEC);
+            m_descriptor = DoOpenSetFD (path, oflag, mode);
+        }
+#elif !defined(_WIN32)
+        m_descriptor = DoOpenSetFD(path, oflag, mode);
+#endif // !defined(LLDB_DISABLE_O_CLOEXEC)
+
     } while (m_descriptor < 0 && errno == EINTR);
 
     if (!DescriptorIsValid())
@@ -853,9 +1043,53 @@ File::PrintfVarArg (const char *format, va_list args)
     return result;
 }
 
+int
+File::OpenAndSetCloseOnExec(const char *path, int oflag)
+{
+    int fd = -1;
+    do
+    {
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+        int oflag_ocloexec = oflag | O_CLOEXEC;
+        fd = ::open(path, oflag_ocloexec);
+
+        if (fd < 0 && errno == EINVAL)
+            fd = DoOpenSetFD(path, oflag);
+#else
+        fd = DoOpenSetFD(path, oflag);
+#endif
+    }
+    while (fd < 0 && errno == EINTR);
+
+    return fd;
+}
+
+int
+File::OpenAndSetCloseOnExec(const char *path, int oflag, mode_t mode)
+{
+    int fd = -1;
+    do
+    {
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+        int oflag_ocloexec = oflag | O_CLOEXEC;
+        fd = ::open(path, oflag_ocloexec, mode);
+
+        if (fd < 0 && errno == EINVAL)
+            fd = DoOpenSetFD(path, oflag, mode);
+#else
+        fd = DoOpenSetFD(path, oflag, mode);
+#endif
+    }
+    while (fd < 0 && errno == EINTR);
+
+    return fd;
+}
+
 mode_t
 File::ConvertOpenOptionsForPOSIXOpen (uint32_t open_options)
 {
+    bool inherit_on_exec = open_options & eOpenOptionInheritOnExecPosixFD;
+
     mode_t mode = 0;
     if (open_options & eOpenOptionRead && open_options & eOpenOptionWrite)
         mode |= O_RDWR;
@@ -871,6 +1105,13 @@ File::ConvertOpenOptionsForPOSIXOpen (uint32_t open_options)
     if (open_options & eOpenOptionNonBlocking)
         mode |= O_NONBLOCK;
 
+    if (!inherit_on_exec)
+    {
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+        mode |= O_CLOEXEC;
+#endif
+    }
+
     if (open_options & eOpenOptionCanCreateNewOnly)
         mode |= O_CREAT | O_EXCL;
     else if (open_options & eOpenOptionCanCreate)
diff --git a/source/Utility/PseudoTerminal.cpp b/source/Utility/PseudoTerminal.cpp
index 98d581d..1df4b3c 100644
--- a/source/Utility/PseudoTerminal.cpp
+++ b/source/Utility/PseudoTerminal.cpp
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/PseudoTerminal.h"
+#include "lldb/Host/File.h"
 
 #include <errno.h>
 #include <stdlib.h>
@@ -17,11 +18,21 @@
 #include <sys/ioctl.h>
 #endif
 
+#if !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+#define LLDB_DISABLE_O_CLOEXEC 1
+
+#if !defined(_WIN32)
+// NOTE: For now we issue compile-time warning in that case, though, it will spam
+// future non-posix platforms builds and will require blacklisting them here.
+#warning "O_CLOEXEC not supported! This operation won't be atomic for open() in your build!"
+
+#endif // !defined(_WIN32)
+#endif // !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+
 #ifdef _WIN32
 #include "lldb/Host/windows/win32.h"
 // empty functions
-int posix_openpt(int flag) { return 0; }
-
+static int DoPosixOpenPTCloseOnExec() { return 0; }
 int strerror_r(int errnum, char *buf, size_t buflen) { return 0; }
 
 int unlockpt(int fd) { return 0; }
@@ -30,10 +41,58 @@ char *ptsname(int fd) { return 0; }
 
 pid_t fork(void) { return 0; }
 pid_t setsid(void) { return 0; }
+#else
+
+static int
+DoPosixOpenPTCloseOnExec(int oflag)
+{
+    int fd = -1;
+    do
+    {
+#ifdef LLDB_DISABLE_O_CLOEXEC
+        int oflag_cloexec = oflag | O_CLOEXEC;
+
+        fd = ::posix_openpt(oflag_cloexec);
+
+        // If O_CLOEXEC is not supported try to emulate it by /dev/ptmx
+        if (fd < 0 && errno == EINVAL)
+            fd = ::open("/dev/ptmx", oflag_cloexec);
+
+        if (fd < 0)
+        {
+            switch (errno) {
+            case EINTR:     continue;  // retry
+            case EINVAL:    break;     // try emulate
+            default:        return fd; // return
+            }
+        }
+#endif
+        // Emulate O_CLOEXEC by fcntl
+        if (fd < 0)
+        {
+            fd = ::posix_openpt(oflag);
+            if (fd >= 0)
+            {
+                int ret = ::fcntl(fd, F_SETFD, FD_CLOEXEC);
+                if (ret == -1)
+                {
+                    int saved_errno = errno;
+                    ::close(fd);
+                    errno = saved_errno;
+                    fd = -1;
+                }
+            }
+        }
+    }
+    while (fd < 0 && errno == EINTR);
+
+    return fd;
+}
 #endif
 
 using namespace lldb_utility;
 
+
 //----------------------------------------------------------------------
 // PseudoTerminal constructor
 //----------------------------------------------------------------------
@@ -104,7 +163,7 @@ PseudoTerminal::OpenFirstAvailableMaster (int oflag, char *error_str, size_t err
         error_str[0] = '\0';
 
     // Open the master side of a pseudo terminal
-    m_master_fd = ::posix_openpt (oflag);
+    m_master_fd = DoPosixOpenPTCloseOnExec (oflag);
     if (m_master_fd < 0)
     {
         if (error_str)
@@ -158,7 +217,8 @@ PseudoTerminal::OpenSlave (int oflag, char *error_str, size_t error_len)
     if (slave_name == NULL)
         return false;
 
-    m_slave_fd = ::open (slave_name, oflag);
+    using lldb_private::File;
+    m_slave_fd = File::OpenAndSetCloseOnExec (slave_name, oflag);
 
     if (m_slave_fd < 0)
     {
diff --git a/tools/debugserver/source/PseudoTerminal.cpp b/tools/debugserver/source/PseudoTerminal.cpp
index f1b505c..77a2d90 100644
--- a/tools/debugserver/source/PseudoTerminal.cpp
+++ b/tools/debugserver/source/PseudoTerminal.cpp
@@ -16,6 +16,107 @@
 #include <sys/ioctl.h>
 #include <unistd.h>
 
+#if !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+#define LLDB_DISABLE_O_CLOEXEC 1
+
+#if !defined(_WIN32)
+// NOTE: For now we issue compile-time warning in that case, though, it will spam
+// future non-posix platforms builds and will require blacklisting them here.
+#warning "O_CLOEXEC not supported! This operation won't be atomic for open() in your build!"
+
+#endif // !defined(_WIN32)
+#endif // !defined(LLDB_DISABLE_O_CLOEXEC) && !defined(O_CLOEXEC)
+
+// Copied from source/Utility/PseudoTerminal.cpp
+// Please try to keep in sync!
+static int
+DoPosixOpenPTCloseOnExec(int oflag)
+{
+    int fd = -1;
+    do
+    {
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+        int oflag_cloexec = oflag | O_CLOEXEC;
+
+        fd = ::posix_openpt(oflag_cloexec);
+
+        // If O_CLOEXEC is not supported try to emulate it by /dev/ptmx
+        if (fd < 0 && errno == EINVAL)
+            fd = ::open("/dev/ptmx", oflag_cloexec);
+
+        if (fd < 0)
+        {
+            switch (errno) {
+            case EINTR:     continue;  // retry
+            case EINVAL:    break;     // try emulate
+            default:        return fd; // return
+            }
+        }
+#endif
+        // Emulate O_CLOEXEC by fcntl
+        if (fd < 0)
+        {
+            fd = ::posix_openpt(oflag);
+            if (fd >= 0)
+            {
+                int ret = ::fcntl(fd, F_SETFD, FD_CLOEXEC);
+                if (ret == -1)
+                {
+                    int saved_errno = errno;
+                    ::close(fd);
+                    errno = saved_errno;
+                    fd = -1;
+                }
+            }
+        }
+    }
+    while (fd < 0 && errno == EINTR);
+
+    return fd;
+}
+
+// Copied from Host/File.cpp please try to keep in sync!
+static int DoOpenSetFD (const char* path, int oflags)
+{
+    int fd = ::open (path, oflags);
+
+    if (fd >= 0)
+    {
+#ifndef _WIN32
+        if (::fcntl (fd, F_SETFD, FD_CLOEXEC) == -1)
+        {
+            int saved_errno = errno;
+            ::close(fd);
+            errno = saved_errno;
+            fd = -1;
+        }
+#endif
+    }
+
+    return fd;
+}
+
+static int
+OpenAndSetCloseOnExec(const char *path, int oflag)
+{
+    int fd = -1;
+    do
+    {
+#if !defined(LLDB_DISABLE_O_CLOEXEC)
+        int oflag_ocloexec = oflag | O_CLOEXEC;
+        fd = ::open(path, oflag_ocloexec);
+
+        if (fd < 0 && errno == EINVAL)
+            fd = DoOpenSetFD(path, oflag);
+#else
+        fd = DoOpenSetFD(path, oflag);
+#endif
+    }
+    while (fd < 0 && errno == EINTR);
+
+    return fd;
+}
+
 //----------------------------------------------------------------------
 // PseudoTerminal constructor
 //----------------------------------------------------------------------
@@ -79,7 +180,7 @@ PseudoTerminal::Error
 PseudoTerminal::OpenFirstAvailableMaster(int oflag)
 {
     // Open the master side of a pseudo terminal
-    m_master_fd = ::posix_openpt (oflag);
+    m_master_fd = DoPosixOpenPTCloseOnExec (oflag);
     if (m_master_fd < 0)
     {
         return err_posix_openpt_failed;
@@ -123,7 +224,7 @@ PseudoTerminal::OpenSlave(int oflag)
     if (slave_name == NULL)
         return err_ptsname_failed;
 
-    m_slave_fd = ::open (slave_name, oflag);
+    m_slave_fd = OpenAndSetCloseOnExec (slave_name, oflag);
 
     if (m_slave_fd < 0)
         return err_open_slave_failed;
@@ -179,6 +280,7 @@ pid_t
 PseudoTerminal::Fork(PseudoTerminal::Error& error)
 {
     pid_t pid = invalid_pid;
+
     error = OpenFirstAvailableMaster (O_RDWR|O_NOCTTY);
 
     if (error == 0)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Avoid-leaking-file-descriptors-to-child-processes.patch
Type: text/x-patch
Size: 22913 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140322/98f18d46/attachment.bin>


More information about the lldb-dev mailing list