Introduce Connection::ReadAll and fix AdbClient

[labath]

Summary

AdbClient was attempting to handle the case where the socket input arrived in pieces, but it was
failing to handle the case where the connection was closed before that happened. In this case, it
would just spin in an infinite loop calling Connection::Read. (This was also the cause of the
spurious timeouts on the darwin->android buildbot. The exact cause of the premature EOF remains
to be investigated, but is likely a server bug.)

Since this wait-for-a-certain-number-of-bytes seems like a useful functionality to have, I am
moving it (with the infinite loop fixed) to the Connection class, and adding an
appropriate test for it.

[clayborg]

The "timeout_usec" parameter to Connection::ReadAll() doesn't seem like it is doing what is intended. It uses this timeout for every read call which means the Connection::ReadAll() function could take much longer. If the timeout was say 1 second and every 0.9 seconds we get 1 byte, this function could take a very long time to complete. So that should be fixed. It also seems like ReadAll() could be done by adding an extra parameter to the Read() function like "bool keep_reading_until_timeout".

[labath]

Added proper handling of the timeout after retries, and moved the functionality into
Connection::Read. I have called the parameter read_full_buffer, as I think that better reflects
its intended use. I did not want to add a default value for it, since default values on virtual
functions can produce unexpected results. I've also had to increase the timeout in AdbClient,
since now it actually had a meaning (it seems that ReadAll was spinning in a loop also in case
the server was sending no data at all). The patch has grown bigger, but this is mostly due to
me refactoring unit tests to promote code reuse.

[ovyalov]

LGTM

source/Plugins/Platform/Android/AdbClient.cpp

37 ↗

(On Diff #55397)

It might be useful to expose this timeout as a settings property to allow its customization for slow ADB connections.

[labath]

source/Plugins/Platform/Android/AdbClient.cpp

37 ↗

(On Diff #55397)

We can do it later if a need appears, but I don't see how making that a setting -- the value needs to be big enough to work under all circumstances, and in case of normal operation, it will not matter, as the function will return as soon as the data is ready.

[labath]

This is going to be messier than I expected. I would need to introduce the same repeat-until-the-buffer-is-full loop into ConnectionGenericFileWindows, which I don't think is good for code reuse. I see two possibilities:

• go back to the version with a separate function for this behavior (with the timeout problem fixed)
• keep the fix completely within the AdbClient class.

Which one do you prefer?

[clayborg]

I would prefer a fix in the ADB client only if I had to pick. I don't think anyone else will use this functionality and prefer to the keep the API as simple as possible for Connection subclasses.

[labath]

Ok, I've put it into AdbClient for now. We can revisit it if we see a second user somewhere...

lldb/trunk/include/lldb/Core/Connection.h

public:
     ///
     /// @param[in] dst_len
     ///     The number of bytes to attempt to read, and also the max
     ///     number of bytes that can be placed into \a dst.
     ///
     /// @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.
     ///
     /// @param[out] error_ptr
     ///     A pointer to an error object that should be given an
     ///     appropriate error value if this method returns zero. This
     ///     value can be NULL if the error value should be ignored.
     ///
     /// @return
     ///     The number of bytes actually read.
     ///
     /// @see size_t Communication::Read (void *, size_t, uint32_t);
     //------------------------------------------------------------------
     virtual size_t
-    Read (void *dst, 
-          size_t dst_len, 
-          uint32_t timeout_usec,
-          lldb::ConnectionStatus &status, 
-          Error *error_ptr) = 0;
+    Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
 
     //------------------------------------------------------------------
     /// The actual write function that attempts to write to the
     /// communications protocol.
     ///
     /// Subclasses must override this function.
     ///
     /// @param[in] dst

lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h

public:
 
     lldb::ConnectionStatus
     Connect (const char *s, Error *error_ptr) override;
 
     lldb::ConnectionStatus
     Disconnect (Error *error_ptr) override;
 
     size_t
-    Read (void *dst, 
-          size_t dst_len, 
-          uint32_t timeout_usec,
-          lldb::ConnectionStatus &status, 
-          Error *error_ptr) override;
+    Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
 
     size_t
     Write (const void *src, size_t src_len, lldb::ConnectionStatus &status, Error *error_ptr) override;
 
     std::string
     GetURI() override;
 
     lldb::ConnectionStatus

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

public:
     ~ConnectionFileDescriptor() override;
 
     bool IsConnected() const override;
 
     lldb::ConnectionStatus Connect(const char *s, Error *error_ptr) override;
 
     lldb::ConnectionStatus Disconnect(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
+    Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer, lldb::ConnectionStatus &status,
+         Error *error_ptr) override;
 
     size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status, Error *error_ptr) override;
 
     std::string GetURI() override;
 
     lldb::ConnectionStatus BytesAvailable(uint32_t timeout_usec, Error *error_ptr);
 
     bool InterruptRead() override;

lldb/trunk/source/Core/Communication.cpp

if (m_read_thread_enabled)
         return 0;
     }
 
     // We aren't using a read thread, just read the data synchronously in this
     // thread.
     lldb::ConnectionSP connection_sp (m_connection_sp);
     if (connection_sp)
     {
-        return connection_sp->Read (dst, dst_len, timeout_usec, status, error_ptr);
+        const bool read_full_buffer = false;
+        return connection_sp->Read(dst, dst_len, timeout_usec, read_full_buffer, status, error_ptr);
     }
 
     if (error_ptr)
         error_ptr->SetErrorString("Invalid connection.");
     status = eConnectionStatusNoConnection;
     return 0;
 }
 

 size_t
 Communication::ReadFromConnection (void *dst, 
                                    size_t dst_len, 
                                    uint32_t timeout_usec,
                                    ConnectionStatus &status, 
                                    Error *error_ptr)
 {
     lldb::ConnectionSP connection_sp(m_connection_sp);
-    return (connection_sp ? connection_sp->Read(dst, dst_len, timeout_usec, status, error_ptr) : 0);
+    const bool read_full_buffer = false;
+    return (connection_sp ? connection_sp->Read(dst, dst_len, read_full_buffer, timeout_usec, status, error_ptr) : 0);
 }
 
 bool
 Communication::ReadThreadIsRunning ()
 {
     return m_read_thread_enabled;
 }
 

lldb/trunk/source/Core/ConnectionSharedMemory.cpp

#else
         shm_unlink (m_name.c_str());
 #endif
         m_name.clear();
     }
     return eConnectionStatusSuccess;
 }
 
 size_t
-ConnectionSharedMemory::Read (void *dst, 
-                              size_t dst_len, 
-                              uint32_t timeout_usec,
-                              ConnectionStatus &status, 
-                              Error *error_ptr)
+ConnectionSharedMemory::Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer,
+                             ConnectionStatus &status, Error *error_ptr)
 {
     status = eConnectionStatusSuccess;
     return 0;
 }
 
 size_t
 ConnectionSharedMemory::Write (const void *src, size_t src_len, ConnectionStatus &status, Error *error_ptr)
 {

lldb/trunk/source/Host/common/Editline.cpp

if (m_multiline_enabled)
             MoveCursor (CursorLocation::BlockEnd, CursorLocation::EditingCursor);
         }
         m_current_line_rows = new_line_rows;
     }
     
     // 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;
 
         // This mutex is locked by our caller (GetLine). Unlock it while we read a character
         // (blocking operation), so we do not hold the mutex indefinitely. This gives a chance
         // 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, status, NULL);
+        int read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, read_full_buffer, 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, status, NULL);
+                read_count = m_input_connection.Read(&ch, 1, UINT32_MAX, read_full_buffer, status, NULL);
             lldbassert(status == lldb::eConnectionStatusInterrupted);
             return 0;
         }
 
         if (read_count)
         {
 #if LLDB_EDITLINE_USE_WCHAR
             // After the initial interruptible read, this is guaranteed not to block

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

ConnectionFileDescriptor::Disconnect(Error *error_ptr)
     m_pipe.Close();
 
     m_uri.clear();
     m_shutting_down = false;
     return status;
 }
 
 size_t
-ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec, ConnectionStatus &status, Error *error_ptr)
+ConnectionFileDescriptor::Read(void *dst, size_t dst_len, uint32_t timeout_usec, bool read_full_buffer,
+                               ConnectionStatus &status, Error *error_ptr)
 {
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
 
     Mutex::Locker locker;
     bool got_lock = locker.TryLock(m_mutex);
     if (!got_lock)
     {
         if (log)
             log->Printf("%p ConnectionFileDescriptor::Read () failed to get the connection lock.", static_cast<void *>(this));
         if (error_ptr)
             error_ptr->SetErrorString("failed to get the connection lock for read.");
 
         status = eConnectionStatusTimedOut;
         return 0;
     }
 
     if (m_shutting_down)
     {
         status = eConnectionStatusError;
         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;
 
-    Error error;
-    size_t bytes_read = dst_len;
-    error = m_read_sp->Read(dst, bytes_read);
-
+        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",
-                    static_cast<void *>(this), static_cast<uint64_t>(m_read_sp->GetWaitableHandle()), static_cast<void *>(dst),
+            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)
         {
-        error.Clear(); // End-of-file.  Do not automatically close; pass along for the end-of-file handlers.
+            // 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);
 
     if (error_ptr)
         *error_ptr = error;
 
     if (error.Fail())
     {
         uint32_t error_value = error.GetError();
         switch (error_value)
if (error.Fail())
                     log->Printf("%p ConnectionFileDescriptor::Read (), unexpected error: %s", static_cast<void *>(this),
                                 strerror(error_value));
                 status = eConnectionStatusError;
                 break; // Break to close....
         }
 
         return 0;
     }
-    return bytes_read;
+    return total_bytes_read;
 }
 
 size_t
 ConnectionFileDescriptor::Write(const void *src, size_t src_len, ConnectionStatus &status, Error *error_ptr)
 {
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
     if (log)
         log->Printf("%p ConnectionFileDescriptor::Write (src = %p, src_len = %" PRIu64 ")", static_cast<void *>(this),

lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp

 #include <sstream>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::platform_android;
 
 namespace {
 
-const uint32_t kReadTimeout = 1000000; // 1 second
+const uint32_t kReadTimeout = 4000000; // 4 seconds
 const char * kOKAY = "OKAY";
 const char * kFAIL = "FAIL";
 const char * kDATA = "DATA";
 const char * kDONE = "DONE";
 
 const char * kSEND = "SEND";
 const char * kRECV = "RECV";
 const char * kSTAT = "STAT";
AdbClient::ReadMessageStream (std::vector<char>& message, uint32_t timeout_ms)
     char buffer[1024];
     while (error.Success() && status == lldb::eConnectionStatusSuccess)
     {
         auto end = std::chrono::steady_clock::now();
         uint32_t elapsed_time = std::chrono::duration_cast<std::chrono::milliseconds>(end - start).count();
         if (elapsed_time >= timeout_ms)
             return Error("Timed out");
 
-        size_t n = m_conn.Read(buffer, sizeof(buffer), 1000 * (timeout_ms - elapsed_time), status, &error);
+        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);
         if (n > 0)
             message.insert(message.end(), &buffer[0], &buffer[n]);
     }
     return error;
 }
 
 Error
 AdbClient::ReadResponseStatus()
AdbClient::ReadSyncHeader (std::string &response_id, uint32_t &data_len)
     }
 
     return error;
 }
 
 Error
 AdbClient::ReadAllBytes (void *buffer, size_t size)
 {
+    const bool read_full_buffer = true;
     Error error;
     ConnectionStatus status;
-    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;
+    size_t read_bytes = m_conn.Read(buffer, size, kReadTimeout, read_full_buffer, status, &error);
+    if (error.Fail())
+    if (read_bytes < size)
+        return Error("Unable to read full buffer.");
+    return Error();
 }
 
 Error
 AdbClient::Stat (const FileSpec &remote_file, uint32_t &mode, uint32_t &size, uint32_t &mtime)
 {
     auto error = StartSync ();
     if (error.Fail ())
         return error;

lldb/trunk/unittests/Host/CMakeLists.txt

 add_lldb_unittest(HostTests
   FileSpecTest.cpp
+  ConnectionFileDescriptorPosixTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
   )

lldb/trunk/unittests/Host/ConnectionFileDescriptorPosixTest.cpp

+//===-- 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));
+}

lldb/trunk/unittests/Host/SocketTest.cpp

 #endif
 
 #include <cstdio>
 #include <functional>
 #include <thread>
 
 #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
 #include "lldb/Host/posix/DomainSocket.h"
 #endif
 
 using namespace lldb_private;
 
#endif
 
     void
     TearDown() override
     {
 #if defined(_MSC_VER)
         ::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)
 {
     std::string host_str;
     std::string port_str;
     int32_t port;
     Error error;

 #ifndef LLDB_DISABLE_POSIX
 TEST_F (SocketTest, DomainListenConnectAccept)
 {
     char* file_name_str = tempnam(nullptr, nullptr);
     EXPECT_NE (nullptr, file_name_str);
     const std::string file_name(file_name_str);
     free(file_name_str);
 
-    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);
+    CreateConnectedSockets<DomainSocket>(file_name.c_str(), [=](const DomainSocket &) { return file_name; });
 }
 #endif
 
 TEST_F (SocketTest, TCPListen0ConnectAccept)
 {
-    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);
+    CreateConnectedTCPSockets();
 }
 
 TEST_F (SocketTest, TCPGetAddress)
 {
     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);
+    std::tie(socket_a_up, socket_b_up) = CreateConnectedTCPSockets();
 
     EXPECT_EQ (socket_a_up->GetLocalPortNumber (), socket_b_up->GetRemotePortNumber ());
     EXPECT_EQ (socket_b_up->GetLocalPortNumber (), socket_a_up->GetRemotePortNumber ());
     EXPECT_NE (socket_a_up->GetLocalPortNumber (), socket_b_up->GetLocalPortNumber ());
     EXPECT_STREQ ("127.0.0.1", socket_a_up->GetRemoteIPAddress ().c_str ());
     EXPECT_STREQ ("127.0.0.1", socket_b_up->GetRemoteIPAddress ().c_str ());
 }
 

lldb/trunk/unittests/Host/SocketUtil.h

+//===-- 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 */