Fix deadlock in Python one-line execution.

Python one-line execution was using ConnectionFileDescriptor to do
a non-blocking read against a pipe.  This won't work on Windows,
as CFD is implemented using select(), and select() only works with
sockets on Windows.

The solution is to use ConnectionGenericFile on Windows, which uses
the native API to do overlapped I/O on the pipe.  This in turn
requires re-implementing Host::Pipe on Windows using native OS
handles instead of the more portable _pipe CRT api.

Reviewed by: Greg Clayton
Differential Revision: http://reviews.llvm.org/D5679

llvm-svn: 219339
This commit is contained in:
Zachary Turner 2014-10-08 20:38:41 +00:00
parent 1947f863ef
commit b2df30d652
8 changed files with 431 additions and 105 deletions

View File

@ -7,77 +7,13 @@
//
//===----------------------------------------------------------------------===//
#ifndef liblldb_Pipe_h_
#define liblldb_Pipe_h_
#if defined(__cplusplus)
#ifndef liblldb_Host_Pipe_h_
#define liblldb_Host_Pipe_h_
#include <stdarg.h>
#include <stdio.h>
#include <sys/types.h>
#if defined(_WIN32)
#include "lldb/Host/windows/PipeWindows.h"
#else
#include "lldb/Host/windows/PipePosix.h"
#endif
#include "lldb/lldb-private.h"
namespace lldb_private {
//----------------------------------------------------------------------
/// @class Pipe Pipe.h "lldb/Host/Pipe.h"
/// @brief A class that abtracts unix style pipes.
///
/// A class that abstracts the LLDB core from host pipe functionality.
//----------------------------------------------------------------------
class Pipe
{
public:
static int kInvalidDescriptor;
Pipe();
~Pipe();
bool
Open();
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();
size_t
Read (void *buf, size_t size);
size_t
Write (const void *buf, size_t size);
private:
int m_fds[2];
};
} // namespace lldb_private
#endif // #if defined(__cplusplus)
#endif // liblldb_Pipe_h_
#endif // liblldb_Host_Pipe_h_

View File

@ -0,0 +1,84 @@
//===-- 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"
namespace lldb_private {
//----------------------------------------------------------------------
/// @class Pipe Pipe.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
{
public:
static int kInvalidDescriptor;
Pipe();
~Pipe();
bool
Open();
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();
size_t
Read (void *buf, size_t size);
size_t
Write (const void *buf, size_t size);
private:
int m_fds[2];
};
} // namespace lldb_private
#endif // #if defined(__cplusplus)
#endif // liblldb_Host_posix_PipePosix_h_

View File

@ -0,0 +1,78 @@
//===-- 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/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
{
public:
Pipe();
~Pipe();
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);
size_t Write(const void *buf, size_t size);
private:
HANDLE m_read;
HANDLE m_write;
int m_read_fd;
int m_write_fd;
OVERLAPPED *m_read_overlapped;
OVERLAPPED *m_write_overlapped;
};
} // namespace lldb_private
#endif // liblldb_Host_posix_PipePosix_h_

View File

@ -21,7 +21,6 @@ add_host_subdirectory(common
common/NativeProcessProtocol.cpp
common/NativeThreadProtocol.cpp
common/OptionParser.cpp
common/Pipe.cpp
common/ProcessRunLock.cpp
common/Socket.cpp
common/SocketAddress.cpp
@ -48,6 +47,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Windows")
windows/HostProcessWindows.cpp
windows/HostThreadWindows.cpp
windows/Mutex.cpp
windows/PipeWindows.cpp
windows/ProcessRunLock.cpp
windows/ThisThread.cpp
windows/Windows.cpp
@ -58,6 +58,7 @@ else()
posix/HostInfoPosix.cpp
posix/HostProcessPosix.cpp
posix/HostThreadPosix.cpp
posix/PipePosix.cpp
)
if (CMAKE_SYSTEM_NAME MATCHES "Darwin")

View File

@ -1,4 +1,4 @@
//===-- Pipe.cpp ------------------------------------------------*- C++ -*-===//
//===-- PipePosix.cpp -------------------------------------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
@ -7,14 +7,9 @@
//
//===----------------------------------------------------------------------===//
#include "lldb/Host/Pipe.h"
#include "lldb/Host/posix/PipePosix.h"
#if defined(_WIN32)
#include <io.h>
#include <fcntl.h>
#else
#include <unistd.h>
#endif
using namespace lldb_private;
@ -39,13 +34,9 @@ Pipe::Open()
if (IsValid())
return true;
#ifdef _WIN32
if (::_pipe(m_fds, 256, O_BINARY) == 0)
return true;
#else
if (::pipe(m_fds) == 0)
return true;
#endif
m_fds[READ] = Pipe::kInvalidDescriptor;
m_fds[WRITE] = Pipe::kInvalidDescriptor;
return false;
@ -110,11 +101,7 @@ Pipe::CloseReadFileDescriptor()
if (ReadDescriptorIsValid())
{
int err;
#ifdef _WIN32
err = _close(m_fds[READ]);
#else
err = close(m_fds[READ]);
#endif
m_fds[READ] = Pipe::kInvalidDescriptor;
return err == 0;
}
@ -127,11 +114,7 @@ Pipe::CloseWriteFileDescriptor()
if (WriteDescriptorIsValid())
{
int err;
#ifdef _WIN32
err = _close(m_fds[WRITE]);
#else
err = close(m_fds[WRITE]);
#endif
m_fds[WRITE] = Pipe::kInvalidDescriptor;
return err == 0;
}
@ -145,11 +128,7 @@ Pipe::Read (void *buf, size_t num_bytes)
if (ReadDescriptorIsValid())
{
const int fd = GetReadFileDescriptor();
#ifdef _WIN32
return _read (fd, (char *)buf, num_bytes);
#else
return read (fd, buf, num_bytes);
#endif
}
return 0; // Return 0 since errno won't be set if we didn't call read
}
@ -160,12 +139,7 @@ Pipe::Write (const void *buf, size_t num_bytes)
if (WriteDescriptorIsValid())
{
const int fd = GetWriteFileDescriptor();
#ifdef _WIN32
return _write (fd, (char *)buf, num_bytes);
#else
return write (fd, buf, num_bytes);
#endif
}
return 0; // Return 0 since errno won't be set if we didn't call write
}

View File

@ -241,9 +241,16 @@ ConnectionGenericFile::Read(void *dst, size_t dst_len, uint32_t timeout_usec, ll
goto finish;
}
// An unknown error occured. Fail out.
return_info.Set(0, eConnectionStatusError, ::GetLastError());
else if (::GetLastError() == ERROR_BROKEN_PIPE)
{
// The write end of a pipe was closed. This is equivalent to EOF.
return_info.Set(0, eConnectionStatusEndOfFile, 0);
}
else
{
// An unknown error occured. Fail out.
return_info.Set(0, eConnectionStatusError, ::GetLastError());
}
goto finish;
finish:

View File

@ -0,0 +1,231 @@
//===-- PipeWindows.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/windows/PipeWindows.h"
#include "llvm/Support/raw_ostream.h"
#include <fcntl.h>
#include <io.h>
#include <atomic>
#include <string>
using namespace lldb_private;
namespace
{
std::atomic<uint32_t> g_pipe_serial(0);
}
Pipe::Pipe()
{
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;
}
Pipe::~Pipe()
{
Close();
}
bool
Pipe::Open(bool read_overlapped, bool write_overlapped)
{
if (IsValid())
return true;
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.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;
m_read =
::CreateNamedPipe(pipe_name.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)
{
::CloseHandle(m_read);
m_read = INVALID_HANDLE_VALUE;
return false;
}
m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
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;
}
int
Pipe::GetReadFileDescriptor() const
{
return m_read_fd;
}
int
Pipe::GetWriteFileDescriptor() const
{
return m_write_fd;
}
int
Pipe::ReleaseReadFileDescriptor()
{
int result = 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 result;
}
int
Pipe::ReleaseWriteFileDescriptor()
{
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;
}
return result;
}
void
Pipe::Close()
{
CloseReadFileDescriptor();
CloseWriteFileDescriptor();
}
bool
Pipe::ReadDescriptorIsValid() const
{
return m_read_fd != -1;
}
bool
Pipe::WriteDescriptorIsValid() const
{
return m_write_fd != -1;
}
bool
Pipe::IsValid() const
{
return ReadDescriptorIsValid() && WriteDescriptorIsValid();
}
bool
Pipe::CloseReadFileDescriptor()
{
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;
}
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;
}
HANDLE
Pipe::GetReadNativeHandle()
{
return m_read;
}
HANDLE
Pipe::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
}
size_t
Pipe::Write(const void *buf, size_t num_bytes)
{
if (WriteDescriptorIsValid())
{
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
}

View File

@ -39,6 +39,10 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadPlan.h"
#if defined(_WIN32)
#include "lldb/Host/windows/ConnectionGenericFileWindows.h"
#endif
using namespace lldb;
using namespace lldb_private;
@ -598,9 +602,20 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
// 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))
{
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());