Revert "Add a read_full_buffer argument to ConnectionFileDescriptor::Read"

This reverts commit r268380 as it breaks windows build (I forgot to make neccesary adjustments to
ConnectionGenericFileWindows).

llvm-svn: 268384
This commit is contained in:
Pavel Labath 2016-05-03 14:07:41 +00:00
parent c1e6aa7e26
commit ef984e7dc0
12 changed files with 129 additions and 274 deletions

View File

@ -114,14 +114,6 @@ public:
/// @param[in] timeout_usec
/// The number of microseconds to wait for the data.
///
/// @param[in] read_full_buffer
/// If true, continues reading until the specified number of bytes is
/// read or some exceptional event occurs, which would prevent the
/// buffer from being filled (timeout, end of file, I/O error, etc.).
/// If false, the function returns as soon as at least some part of
/// the data is available (traditional behavior of the read system
/// call).
///
/// @param[out] status
/// On return, indicates whether the call was successful or terminated
/// due to some error condition.
@ -137,8 +129,11 @@ public:
/// @see size_t Communication::Read (void *, size_t, uint32_t);
//------------------------------------------------------------------
virtual size_t
Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
Error *error_ptr) = 0;
Read (void *dst,
size_t dst_len,
uint32_t timeout_usec,
lldb::ConnectionStatus &status,
Error *error_ptr) = 0;
//------------------------------------------------------------------
/// The actual write function that attempts to write to the

View File

@ -43,8 +43,11 @@ public:
Disconnect (Error *error_ptr) override;
size_t
Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
Error *error_ptr) override;
Read (void *dst,
size_t dst_len,
uint32_t timeout_usec,
lldb::ConnectionStatus &status,
Error *error_ptr) override;
size_t
Write (const void *src, size_t src_len, lldb::ConnectionStatus &status, Error *error_ptr) override;

View File

@ -59,9 +59,7 @@ class ConnectionFileDescriptor : public Connection
lldb::ConnectionStatus Disconnect(Error *error_ptr) override;
size_t
Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
Error *error_ptr) override;
size_t Read(void *dst, size_t dst_len, uint32_t timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr) override;
size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status, Error *error_ptr) override;

View File

@ -191,8 +191,7 @@ Communication::Read (void *dst, size_t dst_len, uint32_t timeout_usec, Connectio
lldb::ConnectionSP connection_sp (m_connection_sp);
if (connection_sp)
{
const bool read_full_buffer = false;
return connection_sp->Read(dst, dst_len, timeout_usec, read_full_buffer, status, error_ptr);
return connection_sp->Read (dst, dst_len, timeout_usec, status, error_ptr);
}
if (error_ptr)
@ -327,8 +326,7 @@ Communication::ReadFromConnection (void *dst,
Error *error_ptr)
{
lldb::ConnectionSP connection_sp(m_connection_sp);
const bool read_full_buffer = false;
return (connection_sp ? connection_sp->Read(dst, dst_len, read_full_buffer, timeout_usec, status, error_ptr) : 0);
return (connection_sp ? connection_sp->Read(dst, dst_len, timeout_usec, status, error_ptr) : 0);
}
bool

View File

@ -94,8 +94,11 @@ ConnectionSharedMemory::Disconnect (Error *error_ptr)
}
size_t
ConnectionSharedMemory::Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer,
ConnectionStatus &status, Error *error_ptr)
ConnectionSharedMemory::Read (void *dst,
size_t dst_len,
uint32_t timeout_usec,
ConnectionStatus &status,
Error *error_ptr)
{
status = eConnectionStatusSuccess;
return 0;

View File

@ -580,7 +580,6 @@ Editline::GetCharacter (EditLineCharType * c)
// Read an actual character
while (true)
{
const bool read_full_buffer = false; // Doesn't really matter, we're reading one byte only.
lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
char ch = 0;
@ -589,12 +588,12 @@ Editline::GetCharacter (EditLineCharType * c)
// for someone to interrupt us. After Read returns, immediately lock the mutex again and
// check if we were interrupted.
m_output_mutex.Unlock();
int read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, read_full_buffer, status, NULL);
int read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, status, NULL);
m_output_mutex.Lock();
if (m_editor_status == EditorStatus::Interrupted)
{
while (read_count > 0 && status == lldb::eConnectionStatusSuccess)
read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, read_full_buffer, status, NULL);
read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, status, NULL);
lldbassert(status == lldb::eConnectionStatusInterrupted);
return 0;
}

View File

@ -411,8 +411,7 @@ ConnectionFileDescriptor::Disconnect(Error *error_ptr)
}
size_t
ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer,
ConnectionStatus &status, Error *error_ptr)
ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec, ConnectionStatus &status, Error *error_ptr)
{
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
@ -435,36 +434,26 @@ ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec,
return 0;
}
size_t total_bytes_read = 0;
char *dst_buf = static_cast<char *>(dst);
auto now = std::chrono::steady_clock::now();
const auto deadline = now + std::chrono::microseconds(timeout_usec);
Error error;
do
{
timeout_usec = std::chrono::duration_cast<std::chrono::microseconds>(deadline - now).count();
status = BytesAvailable(timeout_usec, error_ptr);
if (status != eConnectionStatusSuccess)
return 0;
status = BytesAvailable(timeout_usec, error_ptr);
if (status != eConnectionStatusSuccess)
return 0;
size_t bytes_read = dst_len - total_bytes_read;
error = m_read_sp->Read(dst_buf + total_bytes_read, bytes_read);
if (log)
{
log->Printf("%p ConnectionFileDescriptor::Read() fd = %" PRIu64 ", dst = %p, dst_len = %" PRIu64
") => %" PRIu64 ", error = %s",
this, static_cast<uint64_t>(m_read_sp->GetWaitableHandle()), dst,
static_cast<uint64_t>(dst_len), static_cast<uint64_t>(bytes_read), error.AsCString());
}
total_bytes_read += bytes_read;
if (bytes_read == 0)
{
// End-of-file. Do not automatically close; pass along for the end-of-file handlers.
error.Clear();
status = eConnectionStatusEndOfFile;
}
now = std::chrono::steady_clock::now();
} while (read_full_buffer && total_bytes_read < dst_len && status == eConnectionStatusSuccess && now < deadline);
Error error;
size_t bytes_read = dst_len;
error = m_read_sp->Read(dst, bytes_read);
if (log)
{
log->Printf("%p ConnectionFileDescriptor::Read() fd = %" PRIu64 ", dst = %p, dst_len = %" PRIu64 ") => %" PRIu64 ", error = %s",
static_cast<void *>(this), static_cast<uint64_t>(m_read_sp->GetWaitableHandle()), static_cast<void *>(dst),
static_cast<uint64_t>(dst_len), static_cast<uint64_t>(bytes_read), error.AsCString());
}
if (bytes_read == 0)
{
error.Clear(); // End-of-file. Do not automatically close; pass along for the end-of-file handlers.
status = eConnectionStatusEndOfFile;
}
if (error_ptr)
*error_ptr = error;
@ -520,7 +509,7 @@ ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec,
return 0;
}
return total_bytes_read;
return bytes_read;
}
size_t

View File

@ -34,7 +34,7 @@ using namespace lldb_private::platform_android;
namespace {
const uint32_t kReadTimeout = 4000000; // 4 seconds
const uint32_t kReadTimeout = 1000000; // 1 second
const char * kOKAY = "OKAY";
const char * kFAIL = "FAIL";
const char * kDATA = "DATA";
@ -251,9 +251,7 @@ AdbClient::ReadMessageStream (std::vector<char>& message, uint32_t timeout_ms)
if (elapsed_time >= timeout_ms)
return Error("Timed out");
const bool read_full_buffer = true;
size_t n =
m_conn.Read(buffer, sizeof(buffer), 1000 * (timeout_ms - elapsed_time), read_full_buffer, status, &error);
size_t n = m_conn.Read(buffer, sizeof(buffer), 1000 * (timeout_ms - elapsed_time), status, &error);
if (n > 0)
message.insert(message.end(), &buffer[0], &buffer[n]);
}
@ -492,15 +490,19 @@ AdbClient::ReadSyncHeader (std::string &response_id, uint32_t &data_len)
Error
AdbClient::ReadAllBytes (void *buffer, size_t size)
{
const bool read_full_buffer = true;
Error error;
ConnectionStatus status;
size_t read_bytes = m_conn.Read(buffer, size, kReadTimeout, read_full_buffer, status, &error);
if (error.Fail())
return error;
if (read_bytes < size)
return Error("Unable to read full buffer.");
return Error();
char *read_buffer = static_cast<char*>(buffer);
size_t tota_read_bytes = 0;
while (tota_read_bytes < size)
{
auto read_bytes = m_conn.Read (read_buffer + tota_read_bytes, size - tota_read_bytes, kReadTimeout, status, &error);
if (error.Fail ())
return error;
tota_read_bytes += read_bytes;
}
return error;
}
Error

View File

@ -1,6 +1,5 @@
add_lldb_unittest(HostTests
FileSpecTest.cpp
ConnectionFileDescriptorPosixTest.cpp
SocketAddressTest.cpp
SocketTest.cpp
SymbolsTest.cpp

View File

@ -1,135 +0,0 @@
//===-- ConnectionFileDescriptorPosixTest.cpp -------------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#if defined(_MSC_VER) && (_HAS_EXCEPTIONS == 0)
// Workaround for MSVC standard library bug, which fails to include <thread> when
// exceptions are disabled.
#include <eh.h>
#endif
#include "gtest/gtest.h"
#include "SocketUtil.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
using namespace lldb_private;
using namespace lldb;
class ConnectionFileDescriptorPosixTest : public testing::Test
{
public:
void
SetUp() override
{
#if defined(_MSC_VER)
WSADATA data;
::WSAStartup(MAKEWORD(2, 2), &data);
#endif
}
void
TearDown() override
{
#if defined(_MSC_VER)
::WSACleanup();
#endif
}
};
TEST_F(ConnectionFileDescriptorPosixTest, ReadAll)
{
const bool read_full_buffer = true;
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
std::tie(socket_a_up, socket_b_up) = CreateConnectedTCPSockets();
ConnectionFileDescriptor connection_a(socket_a_up.release());
// First, make sure Read returns nothing.
const auto k_reasonable_timeout_us = 10 * 1000;
char buffer[100];
ConnectionStatus status;
Error error;
size_t bytes_read =
connection_a.Read(buffer, sizeof buffer, k_reasonable_timeout_us, read_full_buffer, status, &error);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(eConnectionStatusTimedOut, status);
ASSERT_EQ(0u, bytes_read);
// Write some data, and make sure it arrives.
const char data[] = {1, 2, 3, 4};
size_t bytes_written = sizeof data;
error = socket_b_up->Write(data, bytes_written);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(sizeof data, bytes_written);
bytes_read = connection_a.Read(buffer, sizeof data, k_reasonable_timeout_us, read_full_buffer, status, &error);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(eConnectionStatusSuccess, status);
ASSERT_EQ(sizeof data, bytes_read);
ASSERT_EQ(0, memcmp(buffer, data, sizeof data));
memset(buffer, 0, sizeof buffer);
// Write the data in two chunks. Make sure we read all of it.
std::future<Error> future_error = std::async(std::launch::async, [&socket_b_up, data]() {
size_t bytes_written = sizeof(data) / 2;
Error error = socket_b_up->Write(data, bytes_written);
if (error.Fail())
return error;
std::this_thread::sleep_for(std::chrono::microseconds(k_reasonable_timeout_us / 10));
bytes_written = sizeof(data) / 2;
return socket_b_up->Write(data + bytes_written, bytes_written);
});
bytes_read = connection_a.Read(buffer, sizeof data, k_reasonable_timeout_us, read_full_buffer, status, &error);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(eConnectionStatusSuccess, status);
ASSERT_EQ(sizeof data, bytes_read);
ASSERT_TRUE(future_error.get().Success()) << future_error.get().AsCString();
ASSERT_EQ(0, memcmp(buffer, data, sizeof data));
// Close the remote end, make sure Read result is reasonable.
socket_b_up.reset();
bytes_read = connection_a.Read(buffer, sizeof buffer, k_reasonable_timeout_us, read_full_buffer, status, &error);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(eConnectionStatusEndOfFile, status);
ASSERT_EQ(0u, bytes_read);
}
TEST_F(ConnectionFileDescriptorPosixTest, Read)
{
const bool read_full_buffer = false;
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
std::tie(socket_a_up, socket_b_up) = CreateConnectedTCPSockets();
ConnectionFileDescriptor connection_a(socket_a_up.release());
const uint32_t k_very_large_timeout_us = 10 * 1000 * 1000;
char buffer[100];
ConnectionStatus status;
Error error;
// Write some data (but not a full buffer). Make sure it arrives, and we do not wait too long.
const char data[] = {1, 2, 3, 4};
size_t bytes_written = sizeof data;
error = socket_b_up->Write(data, bytes_written);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(sizeof data, bytes_written);
const auto start = std::chrono::steady_clock::now();
size_t bytes_read =
connection_a.Read(buffer, sizeof buffer, k_very_large_timeout_us, read_full_buffer, status, &error);
ASSERT_TRUE(error.Success()) << error.AsCString();
ASSERT_EQ(eConnectionStatusSuccess, status);
ASSERT_EQ(sizeof data, bytes_read);
ASSERT_EQ(0, memcmp(buffer, data, sizeof data));
ASSERT_LT(std::chrono::steady_clock::now(), start + std::chrono::microseconds(k_very_large_timeout_us / 10));
}

View File

@ -19,9 +19,9 @@
#include "gtest/gtest.h"
#include "SocketUtil.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/Socket.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Host/common/UDPSocket.h"
#ifndef LLDB_DISABLE_POSIX
@ -49,6 +49,52 @@ class SocketTest : public testing::Test
::WSACleanup();
#endif
}
protected:
static void
AcceptThread(Socket *listen_socket, const char *listen_remote_address, bool child_processes_inherit,
Socket **accept_socket, Error *error)
{
*error = listen_socket->Accept(listen_remote_address, child_processes_inherit, *accept_socket);
}
template<typename SocketType>
void
CreateConnectedSockets(const char *listen_remote_address, const std::function<std::string(const SocketType&)> &get_connect_addr, std::unique_ptr<SocketType> *a_up, std::unique_ptr<SocketType> *b_up)
{
bool child_processes_inherit = false;
Error error;
std::unique_ptr<SocketType> listen_socket_up(new SocketType(child_processes_inherit, error));
EXPECT_FALSE(error.Fail());
error = listen_socket_up->Listen(listen_remote_address, 5);
EXPECT_FALSE(error.Fail());
EXPECT_TRUE(listen_socket_up->IsValid());
Error accept_error;
Socket *accept_socket;
std::thread accept_thread(AcceptThread, listen_socket_up.get(), listen_remote_address, child_processes_inherit,
&accept_socket, &accept_error);
std::string connect_remote_address = get_connect_addr(*listen_socket_up);
std::unique_ptr<SocketType> connect_socket_up(new SocketType(child_processes_inherit, error));
EXPECT_FALSE(error.Fail());
error = connect_socket_up->Connect(connect_remote_address.c_str());
EXPECT_FALSE(error.Fail());
EXPECT_TRUE(connect_socket_up->IsValid());
a_up->swap(connect_socket_up);
EXPECT_TRUE(error.Success());
EXPECT_NE(nullptr, a_up->get());
EXPECT_TRUE((*a_up)->IsValid());
accept_thread.join();
b_up->reset(static_cast<SocketType*>(accept_socket));
EXPECT_TRUE(accept_error.Success());
EXPECT_NE(nullptr, b_up->get());
EXPECT_TRUE((*b_up)->IsValid());
listen_socket_up.reset();
}
};
TEST_F (SocketTest, DecodeHostAndPort)
@ -102,20 +148,44 @@ TEST_F (SocketTest, DomainListenConnectAccept)
const std::string file_name(file_name_str);
free(file_name_str);
CreateConnectedSockets<DomainSocket>(file_name.c_str(), [=](const DomainSocket &) { return file_name; });
std::unique_ptr<DomainSocket> socket_a_up;
std::unique_ptr<DomainSocket> socket_b_up;
CreateConnectedSockets<DomainSocket>(file_name.c_str(),
[=](const DomainSocket &)
{
return file_name;
},
&socket_a_up, &socket_b_up);
}
#endif
TEST_F (SocketTest, TCPListen0ConnectAccept)
{
CreateConnectedTCPSockets();
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
CreateConnectedSockets<TCPSocket>("127.0.0.1:0",
[=](const TCPSocket &s)
{
char connect_remote_address[64];
snprintf(connect_remote_address, sizeof(connect_remote_address), "localhost:%u", s.GetLocalPortNumber());
return std::string(connect_remote_address);
},
&socket_a_up, &socket_b_up);
}
TEST_F (SocketTest, TCPGetAddress)
{
std::unique_ptr<TCPSocket> socket_a_up;
std::unique_ptr<TCPSocket> socket_b_up;
std::tie(socket_a_up, socket_b_up) = CreateConnectedTCPSockets();
CreateConnectedSockets<TCPSocket>("127.0.0.1:0",
[=](const TCPSocket &s)
{
char connect_remote_address[64];
snprintf(connect_remote_address, sizeof(connect_remote_address), "localhost:%u", s.GetLocalPortNumber());
return std::string(connect_remote_address);
},
&socket_a_up,
&socket_b_up);
EXPECT_EQ (socket_a_up->GetLocalPortNumber (), socket_b_up->GetRemotePortNumber ());
EXPECT_EQ (socket_b_up->GetLocalPortNumber (), socket_a_up->GetRemotePortNumber ());

View File

@ -1,66 +0,0 @@
//===-- SocketUtil.h --------------------------------------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#ifndef lldb_unittests_Host_SocketUtil_h
#define lldb_unittests_Host_SocketUtil_h
#include <future>
#include "gtest/gtest.h"
#include "lldb/Core/Error.h"
#include "lldb/Host/Socket.h"
#include "lldb/Host/common/TCPSocket.h"
template <typename SocketType>
std::pair<std::unique_ptr<SocketType>, std::unique_ptr<SocketType>>
CreateConnectedSockets(const char *listen_remote_address,
const std::function<std::string(const SocketType &)> &get_connect_addr)
{
using namespace lldb_private;
const bool child_processes_inherit = false;
Error error;
std::unique_ptr<SocketType> listen_socket_up(new SocketType(child_processes_inherit, error));
EXPECT_FALSE(error.Fail());
error = listen_socket_up->Listen(listen_remote_address, 5);
EXPECT_FALSE(error.Fail());
EXPECT_TRUE(listen_socket_up->IsValid());
Socket *accept_socket;
std::future<Error> accept_error = std::async(std::launch::async, [&]() {
return listen_socket_up->Accept(listen_remote_address, child_processes_inherit, accept_socket);
});
std::string connect_remote_address = get_connect_addr(*listen_socket_up);
std::unique_ptr<SocketType> connect_socket_up(new SocketType(child_processes_inherit, error));
EXPECT_FALSE(error.Fail());
error = connect_socket_up->Connect(connect_remote_address.c_str());
EXPECT_FALSE(error.Fail());
EXPECT_NE(nullptr, connect_socket_up);
EXPECT_TRUE(connect_socket_up->IsValid());
EXPECT_TRUE(accept_error.get().Success());
EXPECT_NE(nullptr, accept_socket);
EXPECT_TRUE(accept_socket->IsValid());
return {std::move(connect_socket_up), std::unique_ptr<SocketType>(static_cast<SocketType *>(accept_socket))};
}
inline std::pair<std::unique_ptr<lldb_private::TCPSocket>, std::unique_ptr<lldb_private::TCPSocket>>
CreateConnectedTCPSockets()
{
return CreateConnectedSockets<lldb_private::TCPSocket>("127.0.0.1:0", [=](const lldb_private::TCPSocket &s) {
char connect_remote_address[64];
snprintf(connect_remote_address, sizeof(connect_remote_address), "localhost:%u", s.GetLocalPortNumber());
return std::string(connect_remote_address);
});
}
#endif /* lldb_unittests_Host_SocketUtil_h */