Index: lldb/include/lldb/Core/Communication.h =================================================================== --- lldb/include/lldb/Core/Communication.h +++ lldb/include/lldb/Core/Communication.h @@ -324,6 +324,9 @@ m_bytes; ///< A buffer to cache bytes read in the ReadThread function. std::recursive_mutex m_bytes_mutex; ///< A mutex to protect multi-threaded ///access to the cached bytes. + lldb::ConnectionStatus m_pass_status; ///< Connection status passthrough + ///from read thread. + Status m_pass_error; ///< Error passthrough from read thread. std::mutex m_write_mutex; ///< Don't let multiple threads write at the same time... std::mutex m_synchronize_mutex; Index: lldb/source/Core/Communication.cpp =================================================================== --- lldb/source/Core/Communication.cpp +++ lldb/source/Core/Communication.cpp @@ -162,14 +162,10 @@ 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; + // hit an end-of-file condition or an error. + status = m_pass_status; + if (error_ptr) + *error_ptr = std::move(m_pass_error); if (GetCloseOnEOF()) Disconnect(nullptr); @@ -384,7 +380,8 @@ break; } } - log = GetLog(LLDBLog::Communication); + m_pass_status = status; + m_pass_error = std::move(error); LLDB_LOG(log, "Communication({0}) thread exiting...", this); // Handle threads wishing to synchronize with us. Index: lldb/unittests/Core/CommunicationTest.cpp =================================================================== --- lldb/unittests/Core/CommunicationTest.cpp +++ lldb/unittests/Core/CommunicationTest.cpp @@ -13,6 +13,9 @@ #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" +#include +#include +#include #include #if LLDB_ENABLE_POSIX @@ -75,9 +78,36 @@ EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile); EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded()); - // JoinReadThread() should just return immediately since there was no read + // JoinReadThread() should just return immediately if there was no read // thread started. EXPECT_TRUE(comm.JoinReadThread()); + + // Test using Communication without a connection. + comm.SetConnection(nullptr); + if (use_read_thread) + ASSERT_TRUE(comm.StartReadThread()); + error.Clear(); + EXPECT_EQ( + comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U); + EXPECT_EQ(status, lldb::eConnectionStatusNoConnection); + EXPECT_THAT_ERROR(error.ToError(), llvm::Failed()); + EXPECT_TRUE(comm.JoinReadThread()); + + // Test using Communication that is disconnected. + ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(), + llvm::Succeeded()); + comm.SetConnection(std::make_unique( + pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true)); + comm.SetCloseOnEOF(true); + ASSERT_EQ(comm.Disconnect(), lldb::eConnectionStatusSuccess); + if (use_read_thread) + ASSERT_TRUE(comm.StartReadThread()); + error.Clear(); + EXPECT_EQ( + comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U); + EXPECT_EQ(status, lldb::eConnectionStatusLostConnection); + EXPECT_THAT_ERROR(error.ToError(), llvm::Failed()); + EXPECT_TRUE(comm.JoinReadThread()); } TEST(CommunicationTest, Read) { @@ -88,6 +118,31 @@ CommunicationReadTest(/*use_thread=*/true); } +TEST(CommunicationTest, StopReadThread) { + std::condition_variable finished; + std::mutex finished_mutex; + + std::thread test_thread{[&finished]() { + Pipe pipe; + ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(), + llvm::Succeeded()); + + Communication comm("test"); + comm.SetConnection(std::make_unique( + pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true)); + comm.SetCloseOnEOF(true); + + ASSERT_TRUE(comm.StartReadThread()); + ASSERT_TRUE(comm.StopReadThread()); + finished.notify_all(); + }}; + + // StopReadThread() can hang, so force an external timeout. + std::unique_lock lock{finished_mutex}; + ASSERT_EQ(finished.wait_for(lock, std::chrono::seconds(3)), std::cv_status::no_timeout); + test_thread.join(); +} + TEST(CommunicationTest, SynchronizeWhileClosing) { // Set up a communication object reading from a pipe. Pipe pipe;