Skip to content

Commit

Permalink
Enhance the Pipe interface for better portability.
Browse files Browse the repository at this point in the history
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.

2) All methods return lldb_private::Error instead of returning bool
   or void.  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.

New methods described in points #3 and #4 are stubbed out on posix,
but fully implemented on Windows.  These should be implemented by
someone on the linux / mac / bsd side.

Reviewed by: Greg Clayton, Oleksiy Vyalov
Differential Revision: http://reviews.llvm.org/D6686

llvm-svn: 224442
  • Loading branch information
Zachary Turner committed Dec 17, 2014
1 parent 06b2c54 commit 0b9d3ee
Show file tree
Hide file tree
Showing 10 changed files with 452 additions and 266 deletions.
8 changes: 8 additions & 0 deletions lldb/include/lldb/Host/Pipe.h
Expand Up @@ -12,8 +12,16 @@

#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_
48 changes: 48 additions & 0 deletions lldb/include/lldb/Host/PipeBase.h
@@ -0,0 +1,48 @@
//===-- 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
68 changes: 24 additions & 44 deletions lldb/include/lldb/Host/posix/PipePosix.h
Expand Up @@ -11,70 +11,50 @@
#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;
PipePosix();

bool
WriteDescriptorIsValid() const;
~PipePosix() override;

int
GetReadFileDescriptor() const;

int
GetWriteFileDescriptor() const;

// Close both descriptors
void
Close();
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
CloseReadFileDescriptor();

bool
CloseWriteFileDescriptor();
bool CanRead() const override;
bool CanWrite() const override;

int
ReleaseReadFileDescriptor();

int
ReleaseWriteFileDescriptor();
int GetReadFileDescriptor() const override;
int GetWriteFileDescriptor() const override;
int ReleaseReadFileDescriptor() override;
int ReleaseWriteFileDescriptor() override;

size_t
Read (void *buf, size_t size);
// Close both descriptors
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;

size_t
Write (const void *buf, size_t size);
private:
void CloseReadFileDescriptor();
void CloseWriteFileDescriptor();

int m_fds[2];
};

Expand Down
59 changes: 27 additions & 32 deletions lldb/include/lldb/Host/windows/PipeWindows.h
Expand Up @@ -10,6 +10,7 @@
#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
Expand All @@ -22,55 +23,49 @@ namespace lldb_private
///
/// 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 Open(bool read_overlapped = false, bool write_overlapped = false);
bool CanRead() const override;
bool CanWrite() const override;

bool IsValid() const;
int GetReadFileDescriptor() const override;
int GetWriteFileDescriptor() const override;
int ReleaseReadFileDescriptor() override;
int ReleaseWriteFileDescriptor() override;

bool ReadDescriptorIsValid() const;
void Close() override;

bool WriteDescriptorIsValid() const;
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;

int GetReadFileDescriptor() const;
// PipeWindows specific methods. These allow access to the underlying OS handle.
HANDLE GetReadNativeHandle();
HANDLE GetWriteNativeHandle();

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
Expand Down
2 changes: 2 additions & 0 deletions lldb/lldb.xcodeproj/project.pbxproj
Expand Up @@ -1794,6 +1794,7 @@
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>"; };
Expand Down Expand Up @@ -3700,6 +3701,7 @@
232CB612191E00CC00EF39FC /* NativeThreadProtocol.h */,
A36FF33D17D8E98800244D40 /* OptionParser.h */,
260A39A519647A3A004B4130 /* Pipe.h */,
3F5E8AF31A40D4A500A73232 /* PipeBase.h */,
26BC7DD610F1B7D500F91463 /* Predicate.h */,
3FDFED2219BA6D55009756A7 /* ProcessRunLock.h */,
236124A71986B50E004EFC37 /* Socket.h */,
Expand Down
19 changes: 11 additions & 8 deletions lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
Expand Up @@ -96,11 +96,12 @@ ConnectionFileDescriptor::OpenCommandPipe()

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
{
Expand Down Expand Up @@ -291,7 +292,9 @@ ConnectionFileDescriptor::Connect(const char *s, Error *error_ptr)
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
Expand Down Expand Up @@ -325,13 +328,13 @@ ConnectionFileDescriptor::Disconnect(Error *error_ptr)

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)
{
Expand Down

0 comments on commit 0b9d3ee

Please sign in to comment.