[Lldb-commits] [lldb] 39e0a87 - [lldb] [Core] Pass error/status from Communication::ReadThread()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-09-01T14:16:38+02:00
New Revision: 39e0a87c26e0d2b7498186a446dda4ec3d9b709d

URL: https://github.com/llvm/llvm-project/commit/39e0a87c26e0d2b7498186a446dda4ec3d9b709d
DIFF: https://github.com/llvm/llvm-project/commit/39e0a87c26e0d2b7498186a446dda4ec3d9b709d.diff

LOG: [lldb] [Core] Pass error/status from Communication::ReadThread()

Ensure that the ConnectionStatus and Status from
Communication::ReadThread() is correctly passed to ::Read() in order
to fix further discrepancies between ::Read() calls in non-threaded
and threaded modes.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D132577

Added: 
    

Modified: 
    lldb/include/lldb/Core/Communication.h
    lldb/source/Core/Communication.cpp
    lldb/unittests/Core/CommunicationTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/Communication.h b/lldb/include/lldb/Core/Communication.h
index 44b3a16a05269..c07b06a6c2509 100644
--- a/lldb/include/lldb/Core/Communication.h
+++ b/lldb/include/lldb/Core/Communication.h
@@ -324,6 +324,9 @@ class Communication : public Broadcaster {
       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;

diff  --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp
index 56f0990d8c7f7..3606d92a9d4e0 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -162,14 +162,10 @@ size_t Communication::Read(void *dst, size_t dst_len,
 
       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 @@ lldb::thread_result_t Communication::ReadThread() {
       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.

diff  --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp
index 5cb1d4a3bbbf0..a6da6302e5e14 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -15,6 +15,7 @@
 #include "TestingSupport/Host/SocketTestUtilities.h"
 #include "TestingSupport/SubsystemRAII.h"
 
+#include <chrono>
 #include <thread>
 
 #if LLDB_ENABLE_POSIX
@@ -78,9 +79,31 @@ static void CommunicationReadTest(bool use_read_thread) {
   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 that is disconnected.
+  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 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_F(CommunicationTest, Read) {