Enhance the Pipe interface for better portability

[zturner]

Summary

This patch makes a number of improvements to the Pipe interface.

1. An interface (PipeBase) is provided which exposes pure virtual methods for any implementation of Pipe to override. While not strictly necessary, this helps catch errors where the interfaces are out of sync. In fact, this would have caught a subtle bug on Windows where the signature of the Open() method had a compatible but different signature to the Posix version due to the use of default arguments. This guarantees that the interfaces remain in sync.

2. All methods return lldb_private::Error instead of bools, or in some cases nothing. This allows richer error information to be propagated up to LLDB.

3. A new ReadWithTimeout() method is exposed in the base class and implemented on Windows.

4. Support for both named and anonymous pipes is exposed through the base interface and implemented on Windows. For creating a new pipe, both named and anonymous pipes are supported, and for opening an existing pipe, only named pipes are supported.

The new interface will need to be implemented in PipePosix before this can go live. I was hoping Oleksiy could do that at the same time he puts the logic back for reading the port from debugserver's pipe with timeout. This new interface should provide sufficient flexibility that this will be portable on all platforms.

[zturner]

I went ahead and updated the Posix side to conform to the base interface. I compiled on MacOSX and there were no errors. There should be no functionality change here, so this patch is actually probably ok to go in as-is. However, a few methods are still unimplemented. These are all new methods that didn't exist before, so having them unimplemented shouldn't cause anything to break that was previously working. They are in PipePosix.cpp, and should be obvious by inspecting the patch.

Hopefully Oleksiy can fill out these stubs in a followup patch, but I think this patch is ready to go in after review.

[clayborg]

Looks fine.

[ovyalov]

Please see my comments.

include/lldb/Host/PipeBase.h

27 ↗

(On Diff #17358)

Minor thing - you may have name passed as a first parameter so a signature will be in accord with open sys call syntax - open(name, flags)

source/Host/posix/PipePosix.cpp

66 ↗

(On Diff #17358)

Please add error.SetErrorToErrno();

76 ↗

(On Diff #17358)

Could you call error.SetErrorToErrno(); before Close() to make sure that Close doesn't overwrite errno?

source/Host/windows/PipeWindows.cpp

116 ↗

(On Diff #17358)

Just out of curiosity - why are you using std::string::data here, not c_str?

[zturner]

include/lldb/Host/PipeBase.h

27 ↗

(On Diff #17358)

Sounds reasonable.

source/Host/posix/PipePosix.cpp

76 ↗

(On Diff #17358)

Yea, good point. I'll fix this.

source/Host/windows/PipeWindows.cpp

116 ↗

(On Diff #17358)

This isn't std::string::data, it's llvm::StringRef::data. llvm::StringRef is a wrapper around a char *, but it's immutable and doesn't own the string data. It just provides convenience methods like length(), equals_lower(), etc.

However, it doesn't provide a c_str() method. The reason for that is because StringRef can be used to refer to non-null terminated string data. So it stores the length internally. In theory they could provide a c_str() method that's the same as the data() method, but they argue that people will be confused and assume that the result of c_str() is null terminated. If you create the StringRef yourself, and you know it refers to a null terminated string, then it's safe to treat data() as a null terminated string.

[zturner]

Updated with changes suggested by ovyalov

[zturner]

In D6686#102208, @zturner wrote:

Updated with changes suggested by ovyalov

I missed one of your requested changes (adding the call to error.SetErrorToErrno()). I fixed that after I uploaded the last patch, but it's probably not worth uploading a new patch just for that one line. Let me know if there's anything else though.

[ovyalov]

Looks good - I ran your patch on Ubuntu, no regression.

Just minor things.

include/lldb/Host/posix/PipePosix.h

34 ↗

(On Diff #17371)

You may follow the same style here as you did in PipeWindows - no need to have virtual prefix.

55 ↗

(On Diff #17371)

It seems an indentation issue - could you fix it?

source/Host/posix/ConnectionFileDescriptorPosix.cpp

297 ↗

(On Diff #17371)

return (result.Success() && bytes_written == 1) ?

337 ↗

(On Diff #17371)

Do we need to log result variable instead of bytes_written?

[zturner]

source/Host/posix/ConnectionFileDescriptorPosix.cpp

297 ↗

(On Diff #17371)

It seems redundant to check for bytes_written == 1. A postcondition of Pipe::Write should be that result.Success() implies bytes_written == bytes_requested.

337 ↗

(On Diff #17371)

Seems reasonable. After fixing these issues i'll go ahead and commit.

[ovyalov]

source/Host/posix/ConnectionFileDescriptorPosix.cpp

297 ↗

(On Diff #17371)

Quote from PipePosix::Write:

int result = write(fd, buf, num_bytes);
        if (result >= 0)
            bytes_written = result;

So, if write returns 0 Write still will be successful.

[zturner]

source/Host/posix/ConnectionFileDescriptorPosix.cpp

297 ↗

(On Diff #17371)

Is it possible to re-write it in such a way that Read() blocks until either success, failure, or EOF, and Write() blocks until either success or failure?

I'm not familiar with all the posix intricacies, but a quick look at the man page suggests that the only case this might happen is when the pipe's output buffer is full. But shouldn't we require that Pipe::Write() blocks until the data is written or an error occurs? Of course, the function might need to be modified in order to guarantee that (similarly for read).

If we can agree that the functions should behave that way (even if they currently aren't), then maybe you could do that at the same time you implement the fifo and ReadWithTimeout logic? It might be easier for someone who is more familiar with linux than I am.

[ovyalov]

source/Host/posix/ConnectionFileDescriptorPosix.cpp

297 ↗

(On Diff #17371)

Yes, I think it makes to implement Read/Write in the way that you suggested and such change shouldn't affect the code that potentially anticipates possible partial read/writes.
It's definitely should be done in another CL - I can take on this later.

lldb/trunk/include/lldb/Host/Pipe.h

 //===-- Pipe.h --------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Host_Pipe_h_
 #define liblldb_Host_Pipe_h_
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/PipeWindows.h"
+namespace lldb_private
+{
+typedef PipeWindows Pipe;
+}
 #else
 #include "lldb/Host/posix/PipePosix.h"
+namespace lldb_private
+{
+typedef PipePosix Pipe;
+}
 #endif
 
 #endif // liblldb_Host_Pipe_h_

lldb/trunk/include/lldb/Host/PipeBase.h

+//===-- PipeBase.h -----------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_PipeBase_h_
+#define liblldb_Host_PipeBase_h_
+
+#include <chrono>
+#include <string>
+
+#include "lldb/Core/Error.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private
+{
+class PipeBase
+{
+  public:
+    virtual ~PipeBase() {}
+
+    virtual Error CreateNew(bool child_process_inherit) = 0;
+    virtual Error CreateNew(llvm::StringRef name, bool child_process_inherit) = 0;
+    virtual Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) = 0;
+    virtual Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) = 0;
+
+    virtual bool CanRead() const = 0;
+    virtual bool CanWrite() const = 0;
+
+    virtual int GetReadFileDescriptor() const = 0;
+    virtual int GetWriteFileDescriptor() const = 0;
+    virtual int ReleaseReadFileDescriptor() = 0;
+    virtual int ReleaseWriteFileDescriptor() = 0;
+
+    // Close both descriptors
+    virtual void Close() = 0;
+
+    virtual Error Write(const void *buf, size_t size, size_t &bytes_written) = 0;
+    virtual Error Read(void *buf, size_t size, size_t &bytes_read) = 0;
+    virtual Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) = 0;
+};
+}
+
+#endif

lldb/trunk/include/lldb/Host/posix/PipePosix.h

 //===-- PipePosix.h ---------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Host_posix_PipePosix_h_
 #define liblldb_Host_posix_PipePosix_h_
 #if defined(__cplusplus)
 
-#include <stdarg.h>
-#include <stdio.h>
-#include <sys/types.h>
-
-#include "lldb/lldb-private.h"
+#include "lldb/Host/PipeBase.h"
 
 namespace lldb_private {
 
 //----------------------------------------------------------------------
-/// @class Pipe Pipe.h "lldb/Host/posix/PipePosix.h"
+/// @class PipePosix PipePosix  .h "lldb/Host/posix/PipePosix.h"
 /// @brief A posix-based implementation of Pipe, a class that abtracts
 ///        unix style pipes.
 ///
 /// A class that abstracts the LLDB core from host pipe functionality.
 //----------------------------------------------------------------------
-class Pipe
+class PipePosix : public PipeBase
 {
 public:
     static int kInvalidDescriptor;
-    
-    Pipe();
-    
-    ~Pipe();
-    
-    bool
-    Open(bool child_processes_inherit = false);
-
-    bool
-    IsValid() const;
-    
-    bool
-    ReadDescriptorIsValid() const;
-
-    bool
-    WriteDescriptorIsValid() const;
+    PipePosix();
 
-    int
-    GetReadFileDescriptor() const;
-    
-    int
-    GetWriteFileDescriptor() const;
-    
+    ~PipePosix() override;
+    Error CreateNew(bool child_process_inherit) override;
+    Error CreateNew(llvm::StringRef name, bool child_process_inherit) override;
+    Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) override;
+    Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) override;
+
+    bool CanRead() const override;
+    bool CanWrite() const override;
+
+    int GetReadFileDescriptor() const override;
+    int GetWriteFileDescriptor() const override;
+    int ReleaseReadFileDescriptor() override;
+    int ReleaseWriteFileDescriptor() override;
     // Close both descriptors
-    void
-    Close();
+    void Close() override;
 
-    bool
-    CloseReadFileDescriptor();
-    
-    bool
-    CloseWriteFileDescriptor();
-
-    int
-    ReleaseReadFileDescriptor();
-    
-    int
-    ReleaseWriteFileDescriptor();
-
-    size_t
-    Read (void *buf, size_t size);
-
-    size_t
-    Write (const void *buf, size_t size);
+    Error Write(const void *buf, size_t size, size_t &bytes_written) override;
+    Error Read(void *buf, size_t size, size_t &bytes_read) override;
+    Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) override;
 private:
+  void CloseReadFileDescriptor();
+  void CloseWriteFileDescriptor();
+
     int m_fds[2];
 };
 
 } // namespace lldb_private
 
 #endif  // #if defined(__cplusplus)
 #endif  // liblldb_Host_posix_PipePosix_h_

lldb/trunk/include/lldb/Host/windows/PipeWindows.h

 //===-- PipePosix.h ---------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Host_windows_PipeWindows_h_
 #define liblldb_Host_windows_PipeWindows_h_
 
+#include "lldb/Host/PipeBase.h"
 #include "lldb/Host/windows/windows.h"
 
 namespace lldb_private
 {
 
 //----------------------------------------------------------------------
 /// @class Pipe PipeWindows.h "lldb/Host/windows/PipeWindows.h"
 /// @brief A windows-based implementation of Pipe, a class that abtracts
 ///        unix style pipes.
 ///
 /// A class that abstracts the LLDB core from host pipe functionality.
 //----------------------------------------------------------------------
-class Pipe
+class PipeWindows : public PipeBase
 {
   public:
-    Pipe();
+    PipeWindows();
+    ~PipeWindows() override;
 
-    ~Pipe();
+    Error CreateNew(bool child_process_inherit) override;
+    Error CreateNew(llvm::StringRef name, bool child_process_inherit) override;
+    Error OpenAsReader(llvm::StringRef name, bool child_process_inherit) override;
+    Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit) override;
+
+    bool CanRead() const override;
+    bool CanWrite() const override;
+
+    int GetReadFileDescriptor() const override;
+    int GetWriteFileDescriptor() const override;
+    int ReleaseReadFileDescriptor() override;
+    int ReleaseWriteFileDescriptor() override;
+
+    void Close() override;
+
+    Error Write(const void *buf, size_t size, size_t &bytes_written) override;
+    Error Read(void *buf, size_t size, size_t &bytes_read) override;
+    Error ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &timeout, size_t &bytes_read) override;
+
+    // PipeWindows specific methods.  These allow access to the underlying OS handle.
+    HANDLE GetReadNativeHandle();
+    HANDLE GetWriteNativeHandle();
 
-    bool Open(bool read_overlapped = false, bool write_overlapped = false);
-
-    bool IsValid() const;
-
-    bool ReadDescriptorIsValid() const;
-
-    bool WriteDescriptorIsValid() const;
-
-    int GetReadFileDescriptor() const;
-
-    int GetWriteFileDescriptor() const;
-
-    // Close both descriptors
-    void Close();
-
-    bool CloseReadFileDescriptor();
-
-    bool CloseWriteFileDescriptor();
-
-    int ReleaseReadFileDescriptor();
-
-    int ReleaseWriteFileDescriptor();
-
-    HANDLE
-    GetReadNativeHandle();
-
-    HANDLE
-    GetWriteNativeHandle();
-
-    size_t Read(void *buf, size_t size);
+  private:
+    Error OpenNamedPipe(llvm::StringRef name, bool child_process_inherit, bool is_read);
 
-    size_t Write(const void *buf, size_t size);
+    void CloseReadEndpoint();
+    void CloseWriteEndpoint();
 
-  private:
     HANDLE m_read;
     HANDLE m_write;
 
     int m_read_fd;
     int m_write_fd;
 
-    OVERLAPPED *m_read_overlapped;
-    OVERLAPPED *m_write_overlapped;
+    OVERLAPPED m_read_overlapped;
+    OVERLAPPED m_write_overlapped;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_Host_posix_PipePosix_h_

lldb/trunk/lldb.xcodeproj/project.pbxproj

/* Begin PBXFileReference section */
     26FA4315130103F400E71120 /* FileSpec.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = FileSpec.h; path = include/lldb/Host/FileSpec.h; sourceTree = "<group>"; };
     26FA43171301048600E71120 /* FileSpec.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FileSpec.cpp; sourceTree = "<group>"; };
     26FFC19314FC072100087D58 /* AuxVector.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AuxVector.cpp; sourceTree = "<group>"; };
     26FFC19414FC072100087D58 /* AuxVector.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AuxVector.h; sourceTree = "<group>"; };
     26FFC19514FC072100087D58 /* DYLDRendezvous.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DYLDRendezvous.cpp; sourceTree = "<group>"; };
     26FFC19614FC072100087D58 /* DYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DYLDRendezvous.h; sourceTree = "<group>"; };
     26FFC19714FC072100087D58 /* DynamicLoaderPOSIXDYLD.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DynamicLoaderPOSIXDYLD.cpp; sourceTree = "<group>"; };
     26FFC19814FC072100087D58 /* DynamicLoaderPOSIXDYLD.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DynamicLoaderPOSIXDYLD.h; sourceTree = "<group>"; };
+    3F5E8AF31A40D4A500A73232 /* PipeBase.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = PipeBase.h; path = include/lldb/Host/PipeBase.h; sourceTree = "<group>"; };
     3FDFD6C3199C396E009756A7 /* FileAction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = FileAction.h; path = include/lldb/Target/FileAction.h; sourceTree = "<group>"; };
     3FDFDDBC199C3A06009756A7 /* FileAction.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = FileAction.cpp; path = source/Target/FileAction.cpp; sourceTree = "<group>"; };
     3FDFDDBE199D345E009756A7 /* FileCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = FileCache.cpp; path = source/Host/common/FileCache.cpp; sourceTree = "<group>"; };
     3FDFDDC0199D34E2009756A7 /* FileCache.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = FileCache.h; path = include/lldb/Host/FileCache.h; sourceTree = "<group>"; };
     3FDFDDC1199D34E2009756A7 /* FileSystem.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = FileSystem.h; path = include/lldb/Host/FileSystem.h; sourceTree = "<group>"; };
     3FDFDDC5199D37ED009756A7 /* FileSystem.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FileSystem.cpp; sourceTree = "<group>"; };
     3FDFE52B19A2917A009756A7 /* HostInfoMacOSX.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = HostInfoMacOSX.mm; path = source/Host/macosx/HostInfoMacOSX.mm; sourceTree = "<group>"; };
     3FDFE52D19A291AF009756A7 /* HostInfoMacOSX.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = HostInfoMacOSX.h; path = include/lldb/Host/macosx/HostInfoMacOSX.h; sourceTree = "<group>"; };
26BC7DD010F1B7C100F91463 /* Host */ = {
         232CB60D191E00CC00EF39FC /* NativeBreakpointList.cpp */,
         232CB60E191E00CC00EF39FC /* NativeBreakpointList.h */,
         232CB60F191E00CC00EF39FC /* NativeProcessProtocol.cpp */,
         232CB610191E00CC00EF39FC /* NativeProcessProtocol.h */,
         232CB611191E00CC00EF39FC /* NativeThreadProtocol.cpp */,
         232CB612191E00CC00EF39FC /* NativeThreadProtocol.h */,
         A36FF33D17D8E98800244D40 /* OptionParser.h */,
         260A39A519647A3A004B4130 /* Pipe.h */,
+        3F5E8AF31A40D4A500A73232 /* PipeBase.h */,
         26BC7DD610F1B7D500F91463 /* Predicate.h */,
         3FDFED2219BA6D55009756A7 /* ProcessRunLock.h */,
         236124A71986B50E004EFC37 /* Socket.h */,
         26D7E45B13D5E2F9007FD12B /* SocketAddress.h */,
         26D7E45C13D5E30A007FD12B /* SocketAddress.cpp */,
         232CB613191E00CC00EF39FC /* SoftwareBreakpoint.cpp */,
         232CB614191E00CD00EF39FC /* SoftwareBreakpoint.h */,
         2689B0A4113EE3CD00A4AEDB /* Symbols.h */,

lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp

 
 void
 ConnectionFileDescriptor::OpenCommandPipe()
 {
     CloseCommandPipe();
 
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
     // Make the command file descriptor here:
-    if (!m_pipe.Open(m_child_processes_inherit))
+    Error result = m_pipe.CreateNew(m_child_processes_inherit);
+    if (!result.Success())
     {
         if (log)
             log->Printf("%p ConnectionFileDescriptor::OpenCommandPipe () - could not make pipe: %s", static_cast<void *>(this),
-                        strerror(errno));
+                        result.AsCString());
     }
     else
     {
         if (log)
             log->Printf("%p ConnectionFileDescriptor::OpenCommandPipe() - success readfd=%d writefd=%d", static_cast<void *>(this),
                         m_pipe.GetReadFileDescriptor(), m_pipe.GetWriteFileDescriptor());
     }
 }
#endif
     if (error_ptr)
         error_ptr->SetErrorString("invalid connect arguments");
     return eConnectionStatusError;
 }
 
 bool
 ConnectionFileDescriptor::InterruptRead()
 {
-    return m_pipe.Write("i", 1) == 1;
+    size_t bytes_written = 0;
+    Error result = m_pipe.Write("i", 1, bytes_written);
+    return result.Success();
 }
 
 ConnectionStatus
 ConnectionFileDescriptor::Disconnect(Error *error_ptr)
 {
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
     if (log)
         log->Printf("%p ConnectionFileDescriptor::Disconnect ()", static_cast<void *>(this));
ConnectionFileDescriptor::Disconnect(Error *error_ptr)
 
     m_shutting_down = true;
 
     Mutex::Locker locker;
     bool got_lock = locker.TryLock(m_mutex);
 
     if (!got_lock)
     {
-        if (m_pipe.WriteDescriptorIsValid())
+        if (m_pipe.CanWrite())
         {
-            int result;
-            result = m_pipe.Write("q", 1) == 1;
+            size_t bytes_written = 0;
+            Error result = m_pipe.Write("q", 1, bytes_written);
             if (log)
-                log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, sent 'q' to %d, result = %d.",
-                            static_cast<void *>(this), m_pipe.GetWriteFileDescriptor(), result);
+                log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, sent 'q' to %d, error = '%s'.",
+                            static_cast<void *>(this), m_pipe.GetWriteFileDescriptor(), result.AsCString());
         }
         else if (log)
         {
             log->Printf("%p ConnectionFileDescriptor::Disconnect(): Couldn't get the lock, but no command pipe is available.",
                         static_cast<void *>(this));
         }
         locker.Lock(m_mutex);
     }

lldb/trunk/source/Host/posix/PipePosix.cpp

 //===-- PipePosix.cpp -------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/posix/PipePosix.h"
 
-#include <unistd.h>
+#include <errno.h>
 #include <fcntl.h>
+#include <unistd.h>
+#include <sys/types.h>
 
+using namespace lldb;
 using namespace lldb_private;
 
-int Pipe::kInvalidDescriptor = -1;
+int PipePosix::kInvalidDescriptor = -1;
 
 enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
 
 // pipe2 is supported by Linux, FreeBSD v10 and higher.
 // TODO: Add more platforms that support pipe2.
-#define PIPE2_SUPPORTED defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD__ >= 10)
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD__ >= 10)
+#define PIPE2_SUPPORTED 1
+#else
+#define PIPE2_SUPPORTED 0
+#endif
 
 namespace
 {
 
 #if defined(FD_CLOEXEC) && !PIPE2_SUPPORTED
 bool SetCloexecFlag(int fd)
 {
     int flags = ::fcntl(fd, F_GETFD);
     if (flags == -1)
         return false;
     return (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == 0);
 }
 #endif
 
 }
 
-Pipe::Pipe()
+PipePosix::PipePosix()
 {
-    m_fds[READ] = Pipe::kInvalidDescriptor;
-    m_fds[WRITE] = Pipe::kInvalidDescriptor;
+    m_fds[READ] = PipePosix::kInvalidDescriptor;
+    m_fds[WRITE] = PipePosix::kInvalidDescriptor;
 }
 
-Pipe::~Pipe()
+PipePosix::~PipePosix()
 {
     Close();
 }
 
-bool
-Pipe::Open(bool child_processes_inherit)
+Error
+PipePosix::CreateNew(bool child_processes_inherit)
+{
+    Error error;
+    if (CanRead() || CanWrite())
     {
-    if (IsValid())
-        return true;
+        error.SetError(EINVAL, eErrorTypePOSIX);
+        return error;
+    }
 
 #if PIPE2_SUPPORTED
     if (::pipe2(m_fds, (child_processes_inherit) ? 0 : O_CLOEXEC) == 0)
-        return true;
+    {
+        error.SetErrorToErrno();
+        return error;
+    }
 #else
     if (::pipe(m_fds) == 0)
     {
 #ifdef FD_CLOEXEC
         if (!child_processes_inherit)
         {
             if (!SetCloexecFlag(m_fds[0]) || !SetCloexecFlag(m_fds[1]))
             {
+                error.SetErrorToErrno();
                 Close();
-                return false;
+                return error;
             }
         }
 #endif
-        return true;
+        return error;
     }
 #endif
 
-    m_fds[READ] = Pipe::kInvalidDescriptor;
-    m_fds[WRITE] = Pipe::kInvalidDescriptor;
-    return false;
+    m_fds[READ] = PipePosix::kInvalidDescriptor;
+    m_fds[WRITE] = PipePosix::kInvalidDescriptor;
+    error.SetErrorToErrno();
+    return error;
+}
+
+Error
+PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit)
+{
+    Error error;
+    if (CanRead() || CanWrite())
+        error.SetErrorString("Pipe is already opened");
+    else if (name.empty())
+        error.SetErrorString("Cannot create named pipe with empty name.");
+    else
+        error.SetErrorString("Not implemented");
+    return error;
+}
+
+Error
+PipePosix::OpenAsReader(llvm::StringRef name, bool child_process_inherit)
+{
+    Error error;
+    if (CanRead() || CanWrite())
+        error.SetErrorString("Pipe is already opened");
+    else if (name.empty())
+        error.SetErrorString("Cannot open named pipe with empty name.");
+    else
+        error.SetErrorString("Not implemented");
+    return error;
+}
+
+Error
+PipePosix::OpenAsWriter(llvm::StringRef name, bool child_process_inherit)
+{
+    Error error;
+    if (CanRead() || CanWrite())
+        error.SetErrorString("Pipe is already opened");
+    else if (name.empty())
+        error.SetErrorString("Cannot create named pipe with empty name.");
+    else
+        error.SetErrorString("Not implemented");
+    return error;
 }
 
 int
-Pipe::GetReadFileDescriptor() const
+PipePosix::GetReadFileDescriptor() const
 {
     return m_fds[READ];
 }
 
 int
-Pipe::GetWriteFileDescriptor() const
+PipePosix::GetWriteFileDescriptor() const
 {
     return m_fds[WRITE];
 }
 
 int
-Pipe::ReleaseReadFileDescriptor()
+PipePosix::ReleaseReadFileDescriptor()
 {
     const int fd = m_fds[READ];
-    m_fds[READ] = Pipe::kInvalidDescriptor;
+    m_fds[READ] = PipePosix::kInvalidDescriptor;
     return fd;
 }
 
 int
-Pipe::ReleaseWriteFileDescriptor()
+PipePosix::ReleaseWriteFileDescriptor()
 {
     const int fd = m_fds[WRITE];
-    m_fds[WRITE] = Pipe::kInvalidDescriptor;
+    m_fds[WRITE] = PipePosix::kInvalidDescriptor;
     return fd;
 }
 
 void
-Pipe::Close()
+PipePosix::Close()
 {
     CloseReadFileDescriptor();
     CloseWriteFileDescriptor();
 }
 
 bool
-Pipe::ReadDescriptorIsValid() const
+PipePosix::CanRead() const
 {
-    return m_fds[READ] != Pipe::kInvalidDescriptor;
+    return m_fds[READ] != PipePosix::kInvalidDescriptor;
 }
 
 bool
-Pipe::WriteDescriptorIsValid() const
+PipePosix::CanWrite() const
 {
-    return m_fds[WRITE] != Pipe::kInvalidDescriptor;
+    return m_fds[WRITE] != PipePosix::kInvalidDescriptor;
 }
 
-bool
-Pipe::IsValid() const
-{
-    return ReadDescriptorIsValid() && WriteDescriptorIsValid();
-}
-
-bool
-Pipe::CloseReadFileDescriptor()
+void
+PipePosix::CloseReadFileDescriptor()
 {
-    if (ReadDescriptorIsValid())
+    if (CanRead())
     {
         int err;
         err = close(m_fds[READ]);
-        m_fds[READ] = Pipe::kInvalidDescriptor;
-        return err == 0;
+        m_fds[READ] = PipePosix::kInvalidDescriptor;
     }
-    return true;
 }
 
-bool
-Pipe::CloseWriteFileDescriptor()
+void
+PipePosix::CloseWriteFileDescriptor()
 {
-    if (WriteDescriptorIsValid())
+    if (CanWrite())
     {
         int err;
         err = close(m_fds[WRITE]);
-        m_fds[WRITE] = Pipe::kInvalidDescriptor;
-        return err == 0;
+        m_fds[WRITE] = PipePosix::kInvalidDescriptor;
     }
-    return true;
 }
 
-
-size_t
-Pipe::Read (void *buf, size_t num_bytes)
+Error
+PipePosix::Read(void *buf, size_t num_bytes, size_t &bytes_read)
 {
-    if (ReadDescriptorIsValid())
+    bytes_read = 0;
+    Error error;
+
+    if (CanRead())
     {
         const int fd = GetReadFileDescriptor();
-        return read (fd, buf, num_bytes);
+        int result = read(fd, buf, num_bytes);
+        if (result >= 0)
+            bytes_read = result;
+        else
+            error.SetErrorToErrno();
     }
-    return 0; // Return 0 since errno won't be set if we didn't call read
+    else
+        error.SetError(EINVAL, eErrorTypePOSIX);
+
+    return error;
+}
+
+Error
+PipePosix::ReadWithTimeout(void *buf, size_t num_bytes, const std::chrono::milliseconds &duration, size_t &bytes_read)
+{
+    bytes_read = 0;
+    Error error;
+    error.SetErrorString("Not implemented");
+    return error;
 }
 
-size_t
-Pipe::Write (const void *buf, size_t num_bytes)
+Error
+PipePosix::Write(const void *buf, size_t num_bytes, size_t &bytes_written)
 {
-    if (WriteDescriptorIsValid())
+    bytes_written = 0;
+    Error error;
+
+    if (CanWrite())
     {
         const int fd = GetWriteFileDescriptor();
-        return write (fd, buf, num_bytes);
+        int result = write(fd, buf, num_bytes);
+        if (result >= 0)
+            bytes_written = result;
+        else
+            error.SetErrorToErrno();
     }
-    return 0; // Return 0 since errno won't be set if we didn't call write
+    else
+        error.SetError(EINVAL, eErrorTypePOSIX);
+
+    return error;
 }

lldb/trunk/source/Host/windows/PipeWindows.cpp

 #include "llvm/Support/raw_ostream.h"
 
 #include <fcntl.h>
 #include <io.h>
 
 #include <atomic>
 #include <string>
 
+using namespace lldb;
 using namespace lldb_private;
 
 namespace
 {
 std::atomic<uint32_t> g_pipe_serial(0);
 }
 
-Pipe::Pipe()
+PipeWindows::PipeWindows()
 {
     m_read = INVALID_HANDLE_VALUE;
     m_write = INVALID_HANDLE_VALUE;
 
     m_read_fd = -1;
     m_write_fd = -1;
-
-    m_read_overlapped = nullptr;
-    m_write_overlapped = nullptr;
+    ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+    ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
 }
 
-Pipe::~Pipe()
+PipeWindows::~PipeWindows()
 {
     Close();
 }
 
-bool
-Pipe::Open(bool read_overlapped, bool write_overlapped)
+Error
+PipeWindows::CreateNew(bool child_process_inherit)
 {
-    if (IsValid())
-        return true;
-
+    // Even for anonymous pipes, we open a named pipe.  This is because you cannot get
+    // overlapped i/o on Windows without using a named pipe.  So we synthesize a unique
+    // name.
     uint32_t serial = g_pipe_serial.fetch_add(1);
     std::string pipe_name;
     llvm::raw_string_ostream pipe_name_stream(pipe_name);
-    pipe_name_stream << "\\\\.\\Pipe\\lldb.pipe." << ::GetCurrentProcessId() << "." << serial;
+    pipe_name_stream << "lldb.pipe." << ::GetCurrentProcessId() << "." << serial;
     pipe_name_stream.flush();
 
-    DWORD read_mode = 0;
-    DWORD write_mode = 0;
-    if (read_overlapped)
-        read_mode |= FILE_FLAG_OVERLAPPED;
-    if (write_overlapped)
-        write_mode |= FILE_FLAG_OVERLAPPED;
+    return CreateNew(pipe_name.c_str(), child_process_inherit);
+}
+
+Error
+PipeWindows::CreateNew(llvm::StringRef name, bool child_process_inherit)
+{
+    if (name.empty())
+        return Error(ERROR_INVALID_PARAMETER, eErrorTypeWin32);
+
+    if (CanRead() || CanWrite())
+        return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
+
+    std::string pipe_path = "\\\\.\\Pipe\\";
+    pipe_path.append(name);
+
+    // Always open for overlapped i/o.  We implement blocking manually in Read and Write.
+    DWORD read_mode = FILE_FLAG_OVERLAPPED;
     m_read =
-        ::CreateNamedPipe(pipe_name.c_str(), PIPE_ACCESS_INBOUND | read_mode, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL);
+        ::CreateNamedPipe(pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL);
     if (INVALID_HANDLE_VALUE == m_read)
-        return false;
-    m_write = ::CreateFile(pipe_name.c_str(), GENERIC_WRITE, 0, NULL, OPEN_EXISTING, write_mode, NULL);
-    if (INVALID_HANDLE_VALUE == m_write)
+        return Error(::GetLastError(), eErrorTypeWin32);
+    m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
+    ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+    m_read_overlapped.hEvent = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
+
+    // Open the write end of the pipe.
+    Error result = OpenNamedPipe(name, child_process_inherit, false);
+    if (!result.Success())
     {
-        ::CloseHandle(m_read);
-        m_read = INVALID_HANDLE_VALUE;
-        return false;
+        CloseReadEndpoint();
+        return result;
     }
 
-    m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
-    m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
+    return result;
+}
 
-    if (read_overlapped)
-    {
-        m_read_overlapped = new OVERLAPPED;
-        ZeroMemory(m_read_overlapped, sizeof(OVERLAPPED));
-    }
-    if (write_overlapped)
-    {
-        m_write_overlapped = new OVERLAPPED;
-        ZeroMemory(m_write_overlapped, sizeof(OVERLAPPED));
-    }
-    return true;
+Error
+PipeWindows::OpenAsReader(llvm::StringRef name, bool child_process_inherit)
+    if (CanRead() || CanWrite())
+        return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
+
+    return OpenNamedPipe(name, child_process_inherit, true);
+
+Error
+PipeWindows::OpenAsWriter(llvm::StringRef name, bool child_process_inherit)
+    if (CanRead() || CanWrite())
+        return Error(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
+
+    return OpenNamedPipe(name, child_process_inherit, false);
+
+Error
+PipeWindows::OpenNamedPipe(llvm::StringRef name, bool child_process_inherit, bool is_read)
+{
+    if (name.empty())
+        return Error(ERROR_INVALID_PARAMETER, eErrorTypeWin32);
+
+    assert(is_read ? !CanRead() : !CanWrite());
+
+    SECURITY_ATTRIBUTES attributes = {0};
+    attributes.bInheritHandle = child_process_inherit;
+
+    std::string pipe_path = "\\\\.\\Pipe\\";
+    pipe_path.append(name);
+
+    if (is_read)
+    {
+        m_read = ::CreateFile(pipe_path.c_str(), GENERIC_READ, 0, &attributes, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
+        if (INVALID_HANDLE_VALUE == m_read)
+            return Error(::GetLastError(), eErrorTypeWin32);
+
+        m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
+
+        ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+        m_read_overlapped.hEvent = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
+    }
+    else
+    {
+        m_write = ::CreateFile(pipe_path.c_str(), GENERIC_WRITE, 0, &attributes, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
+        if (INVALID_HANDLE_VALUE == m_write)
+            return Error(::GetLastError(), eErrorTypeWin32);
+
+        m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
+
+        ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+    }
+
+    return Error();
 }
 
 int
-Pipe::GetReadFileDescriptor() const
+PipeWindows::GetReadFileDescriptor() const
 {
     return m_read_fd;
 }
 
 int
-Pipe::GetWriteFileDescriptor() const
+PipeWindows::GetWriteFileDescriptor() const
 {
     return m_write_fd;
 }
 
 int
-Pipe::ReleaseReadFileDescriptor()
+PipeWindows::ReleaseReadFileDescriptor()
 {
+    if (!CanRead())
+        return -1;
     int result = m_read_fd;
     m_read_fd = -1;
+    if (m_read_overlapped.hEvent)
+        ::CloseHandle(m_read_overlapped.hEvent);
     m_read = INVALID_HANDLE_VALUE;
-    if (m_read_overlapped)
-    {
-        delete m_read_overlapped;
-        m_read_overlapped = nullptr;
-    }
+    ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
     return result;
 }
 
 int
-Pipe::ReleaseWriteFileDescriptor()
+PipeWindows::ReleaseWriteFileDescriptor()
 {
+    if (!CanWrite())
+        return -1;
     int result = m_write_fd;
     m_write_fd = -1;
     m_write = INVALID_HANDLE_VALUE;
-    if (m_write_overlapped)
-    {
-        delete m_write_overlapped;
-        m_write_overlapped = nullptr;
-    }
+    ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
     return result;
 }
 
 void
-Pipe::Close()
+PipeWindows::CloseReadEndpoint()
 {
-    CloseReadFileDescriptor();
-    CloseWriteFileDescriptor();
-}
+    if (!CanRead())
+        return;
 
-bool
-Pipe::ReadDescriptorIsValid() const
-{
-    return m_read_fd != -1;
+    if (m_read_overlapped.hEvent)
+        ::CloseHandle(m_read_overlapped.hEvent);
+    _close(m_read_fd);
+    m_read = INVALID_HANDLE_VALUE;
+    m_read_fd = -1;
+    ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
 }
 
-bool
-Pipe::WriteDescriptorIsValid() const
+void
+PipeWindows::CloseWriteEndpoint()
 {
-    return m_write_fd != -1;
+    if (!CanWrite())
+        return;
+
+    _close(m_write_fd);
+    m_write = INVALID_HANDLE_VALUE;
+    m_write_fd = -1;
+    ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
 }
 
-bool
-Pipe::IsValid() const
+void
+PipeWindows::Close()
 {
-    return ReadDescriptorIsValid() && WriteDescriptorIsValid();
+    CloseReadEndpoint();
+    CloseWriteEndpoint();
 }
 
 bool
-Pipe::CloseReadFileDescriptor()
+PipeWindows::CanRead() const
 {
-    if (ReadDescriptorIsValid())
-    {
-        int err;
-        err = _close(m_read_fd);
-        m_read_fd = -1;
-        m_read = INVALID_HANDLE_VALUE;
-        if (m_read_overlapped)
-        {
-            delete m_read_overlapped;
-            m_read_overlapped = nullptr;
-        }
-        return err == 0;
-    }
-    return true;
+    return (m_read != INVALID_HANDLE_VALUE);
 }
 
 bool
-Pipe::CloseWriteFileDescriptor()
-{
-    if (WriteDescriptorIsValid())
-    {
-        int err;
-        err = _close(m_write_fd);
-        m_write_fd = -1;
-        m_write = INVALID_HANDLE_VALUE;
-        if (m_write_overlapped)
-        {
-            delete m_write_overlapped;
-            m_write_overlapped = nullptr;
-        }
-        return err == 0;
-    }
-    return true;
+PipeWindows::CanWrite() const
+    return (m_write != INVALID_HANDLE_VALUE);
 }
 
 HANDLE
-Pipe::GetReadNativeHandle()
+PipeWindows::GetReadNativeHandle()
 {
     return m_read;
 }
 
 HANDLE
-Pipe::GetWriteNativeHandle()
+PipeWindows::GetWriteNativeHandle()
 {
     return m_write;
 }
 
-size_t
-Pipe::Read(void *buf, size_t num_bytes)
-{
-    if (ReadDescriptorIsValid())
-    {
-        DWORD bytes_read = 0;
-        ::ReadFile(m_read, buf, num_bytes, &bytes_read, m_read_overlapped);
-        if (m_read_overlapped)
-            GetOverlappedResult(m_read, m_read_overlapped, &bytes_read, TRUE);
-        return bytes_read;
-    }
-    return 0; // Return 0 since errno won't be set if we didn't call read
+Error
+PipeWindows::Read(void *buf, size_t size, size_t &bytes_read)
+    return ReadWithTimeout(buf, size, std::chrono::milliseconds::zero(), bytes_read);
 }
 
-size_t
-Pipe::Write(const void *buf, size_t num_bytes)
+Error
+PipeWindows::ReadWithTimeout(void *buf, size_t size, const std::chrono::milliseconds &duration, size_t &bytes_read)
+{
+    if (!CanRead())
+        return Error(ERROR_INVALID_HANDLE, eErrorTypeWin32);
+
+    bytes_read = 0;
+    DWORD sys_bytes_read = size;
+    BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read, &m_read_overlapped);
+    if (!result && GetLastError() != ERROR_IO_PENDING)
+        return Error(::GetLastError(), eErrorTypeWin32);
+
+    DWORD timeout = (duration == std::chrono::milliseconds::zero()) ? INFINITE : duration.count();
+    DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
+    if (wait_result != WAIT_OBJECT_0)
     {
-    if (WriteDescriptorIsValid())
+        // The operation probably failed.  However, if it timed out, we need to cancel the I/O.
+        // Between the time we returned from WaitForSingleObject and the time we call CancelIoEx,
+        // the operation may complete.  If that hapens, CancelIoEx will fail and return ERROR_NOT_FOUND.
+        // If that happens, the original operation should be considered to have been successful.
+        bool failed = true;
+        DWORD failure_error = ::GetLastError();
+        if (wait_result == WAIT_TIMEOUT)
         {
-        DWORD bytes_written = 0;
-        ::WriteFile(m_write, buf, num_bytes, &bytes_written, m_read_overlapped);
-        if (m_write_overlapped)
-            GetOverlappedResult(m_write, m_write_overlapped, &bytes_written, TRUE);
-        return bytes_written;
-    }
-    return 0; // Return 0 since errno won't be set if we didn't call write
+            BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
+            if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
+                failed = false;
+        }
+        if (failed)
+            return Error(failure_error, eErrorTypeWin32);
+    }
+
+    // Now we call GetOverlappedResult setting bWait to false, since we've already waited
+    // as long as we're willing to.
+    if (!GetOverlappedResult(m_read, &m_read_overlapped, &sys_bytes_read, FALSE))
+        return Error(::GetLastError(), eErrorTypeWin32);
+
+    bytes_read = sys_bytes_read;
+    return Error();
+
+Error
+PipeWindows::Write(const void *buf, size_t num_bytes, size_t &bytes_written)
+{
+    if (!CanWrite())
+        return Error(ERROR_INVALID_HANDLE, eErrorTypeWin32);
+
+    DWORD sys_bytes_written = 0;
+    BOOL write_result = ::WriteFile(m_write, buf, num_bytes, &sys_bytes_written, &m_write_overlapped);
+    if (!write_result && GetLastError() != ERROR_IO_PENDING)
+        return Error(::GetLastError(), eErrorTypeWin32);
+
+    BOOL result = GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_written, TRUE);
+    if (!result)
+        return Error(::GetLastError(), eErrorTypeWin32);
+    return Error();
 }

lldb/trunk/source/Interpreter/ScriptInterpreterPython.cpp

if (command && command[0])
         if (options.GetEnableIO())
         {
             if (result)
             {
                 input_file_sp = debugger.GetInputFile();
                 // Set output to a temporary file so we can forward the results on to the result object
                 
                 Pipe pipe;
-#if defined(_WIN32)
-                // By default Windows does not create a pipe object that can be used for a non-blocking read.
-                // We must explicitly request it.  Furthermore, we can't use an fd for non-blocking read
-                // operations, and must use the native os HANDLE.
-                if (pipe.Open(true, false))
+                Error pipe_result = pipe.CreateNew(false);
+                if (pipe_result.Success())
                 {
+#if defined(_WIN32)
                     lldb::file_t read_file = pipe.GetReadNativeHandle();
                     pipe.ReleaseReadFileDescriptor();
                     std::unique_ptr<ConnectionGenericFile> conn_ap(new ConnectionGenericFile(read_file, true));
 #else
-                if (pipe.Open())
-                {
                     std::unique_ptr<ConnectionFileDescriptor> conn_ap(new ConnectionFileDescriptor(pipe.ReleaseReadFileDescriptor(), true));
 #endif
                     if (conn_ap->IsConnected())
                     {
                         output_comm.SetConnection(conn_ap.release());
                         output_comm.SetReadThreadBytesReceivedCallback(ReadThreadBytesReceived, &result->GetOutputStream());
                         output_comm.StartReadThread();
                         join_read_thread = true;

lldb/trunk/source/Target/Process.cpp

public:
     ~IOHandlerProcessSTDIO ()
     {
         
     }
     
     bool
     OpenPipes ()
     {
-        if (m_pipe.IsValid())
+        if (m_pipe.CanRead() && m_pipe.CanWrite())
             return true;
-        return m_pipe.Open();
+        Error result = m_pipe.CreateNew(false);
+        return result.Success();
     }
 
     void
     ClosePipes()
     {
         m_pipe.Close();
     }
     
#ifndef _WIN32
                                 if (m_write_file.Write(&ch, n).Fail() || n != 1)
                                     SetIsDone(true);
                             }
                             else
                                 SetIsDone(true);
                         }
                         if (FD_ISSET (pipe_read_fd, &read_fdset))
                         {
+                            size_t bytes_read;
                             // Consume the interrupt byte
-                            if (m_pipe.Read (&ch, 1) == 1)
+                            Error error = m_pipe.Read(&ch, 1, bytes_read);
+                            if (error.Success())
                             {
                                 switch (ch)
                                 {
                                     case 'q':
                                         SetIsDone(true);
                                         break;
                                     case 'i':
                                         if (StateIsRunningState(m_process->GetState()))
#endif
     {
         
     }
     
     virtual void
     Cancel ()
     {
         char ch = 'q';  // Send 'q' for quit
-        m_pipe.Write (&ch, 1);
+        size_t bytes_written = 0;
+        m_pipe.Write(&ch, 1, bytes_written);
     }
 
     virtual bool
     Interrupt ()
     {
         // Do only things that are safe to do in an interrupt context (like in
         // a SIGINT handler), like write 1 byte to a file descriptor. This will
         // interrupt the IOHandlerProcessSTDIO::Run() and we can look at the byte
         // that was written to the pipe and then call m_process->Halt() from a
         // much safer location in code.
         if (m_active)
         {
             char ch = 'i'; // Send 'i' for interrupt
-            return m_pipe.Write (&ch, 1) == 1;
+            size_t bytes_written = 0;
+            Error result = m_pipe.Write(&ch, 1, bytes_written);
+            return result.Success();
         }
         else
         {
             // This IOHandler might be pushed on the stack, but not being run currently
             // so do the right thing if we aren't actively watching for STDIN by sending
             // the interrupt to the process. Otherwise the write to the pipe above would
             // do nothing. This can happen when the command interpreter is running and
             // gets a "expression ...". It will be on the IOHandler thread and sending