diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp --- a/lldb/source/Core/Communication.cpp +++ b/lldb/source/Core/Communication.cpp @@ -132,10 +132,16 @@ if (m_read_thread_enabled) { // We have a dedicated read thread that is getting data for us size_t cached_bytes = GetCachedBytes(dst, dst_len); - if (cached_bytes > 0 || (timeout && timeout->count() == 0)) { + if (cached_bytes > 0) { status = eConnectionStatusSuccess; return cached_bytes; } + if (timeout && timeout->count() == 0) { + if (error_ptr) + error_ptr->SetErrorString("Timed out."); + status = eConnectionStatusTimedOut; + return 0; + } if (!m_connection_sp) { if (error_ptr) @@ -155,11 +161,25 @@ } if (event_type & eBroadcastBitReadThreadDidExit) { + // If the thread exited of its own accord, it either means it + // hit an end-of-file condition or lost connection + // (we verified that we had an connection above). + if (!m_connection_sp) { + if (error_ptr) + error_ptr->SetErrorString("Lost connection."); + status = eConnectionStatusLostConnection; + } else + status = eConnectionStatusEndOfFile; + if (GetCloseOnEOF()) Disconnect(nullptr); - break; + return 0; } } + + if (error_ptr) + error_ptr->SetErrorString("Timed out."); + status = eConnectionStatusTimedOut; return 0; } diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp --- a/lldb/unittests/Core/CommunicationTest.cpp +++ b/lldb/unittests/Core/CommunicationTest.cpp @@ -21,6 +21,72 @@ using namespace lldb_private; +static void CommunicationReadTest(bool use_read_thread) { + Pipe pipe; + ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(), + llvm::Succeeded()); + + Status error; + size_t bytes_written; + ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(), + llvm::Succeeded()); + ASSERT_EQ(bytes_written, 4U); + + Communication comm("test"); + comm.SetConnection(std::make_unique( + pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true)); + comm.SetCloseOnEOF(true); + + if (use_read_thread) + ASSERT_TRUE(comm.StartReadThread()); + + // This read should wait for the data to become available and return it. + lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess; + char buf[16]; + error.Clear(); + EXPECT_EQ( + comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 4U); + EXPECT_EQ(status, lldb::eConnectionStatusSuccess); + EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded()); + buf[4] = 0; + EXPECT_STREQ(buf, "test"); + + // These reads should time out as there is no more data. + error.Clear(); + EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status, + &error), + 0U); + EXPECT_EQ(status, lldb::eConnectionStatusTimedOut); + EXPECT_THAT_ERROR(error.ToError(), llvm::Failed()); + + // 0 is special-cased, so we test it separately. + error.Clear(); + EXPECT_EQ( + comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 0U); + EXPECT_EQ(status, lldb::eConnectionStatusTimedOut); + EXPECT_THAT_ERROR(error.ToError(), llvm::Failed()); + + // This read should return EOF. + pipe.CloseWriteFileDescriptor(); + error.Clear(); + EXPECT_EQ( + comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U); + EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile); + EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded()); + + // JoinReadThread() should just return immediately since there was no read + // thread started. + EXPECT_TRUE(comm.JoinReadThread()); +} + +TEST(CommunicationTest, Read) { + CommunicationReadTest(/*use_thread=*/false); +} + +TEST(CommunicationTest, ReadThread) { + CommunicationReadTest(/*use_thread=*/true); +} + #ifndef _WIN32 TEST(CommunicationTest, SynchronizeWhileClosing) { // Set up a communication object reading from a pipe.