[lldb] [gdb-remote] Isolate ClientBase from Communication

[mgorny]

Summary

Introduce an isolation layer between GDBRemoteClientBase
and GDBRemoteCommunication. This makes it possible to replace
the public packet reading/writing API with wrappers that will make
threaded access possible in the future.

Sponsored by: The FreeBSD Foundation

[labath]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

116–118

Is the plan to make this private/protected at some point, or something like that? Otherwise, I'm not really sure what's the benefit of this over the regular inheritance.

I like the idea of using composition instead of inheritance (I think we could do something similar with GDBRemoteCommunication and Communication), but right now this seems fairly messy, and the benefit is unclear.

[mgorny]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

116–118

Ideally, yes. However, I don't think I'm going to pursue it that far, i.e. someone else will have to take up the effort. And yes, I honestly doubt anybody will.

The main goal is make ground for D135031, i.e. communication via separate thread. What I've been aiming at is leaving GetCommunication() for stuff that's unlikely to break when invoked cross-thread (or unlikely to be invoked cross-thread), while abstracting away the part of the API that needs to be reimplemented to communicate via the comm thread.

[labath]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

116–118

Ok, maybe that's fine. Let's figure out what to do with the other patch first.

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

if (!parsed_url)
     return Status("Invalid URL: %s", url);
 
   // We're going to reuse the hostname when we connect to the debugserver.
   m_platform_scheme = parsed_url->scheme.str();
   m_platform_hostname = parsed_url->hostname.str();
 
   auto client_up =
       std::make_unique<process_gdb_remote::GDBRemoteCommunicationClient>();
-  client_up->SetPacketTimeout(
+  client_up->GetCommunication().SetPacketTimeout(
       process_gdb_remote::ProcessGDBRemote::GetPacketTimeout());
-  client_up->SetConnection(std::make_unique<ConnectionFileDescriptor>());
-  client_up->Connect(url, &error);
+  client_up->GetCommunication().SetConnection(
+      std::make_unique<ConnectionFileDescriptor>());
+  client_up->GetCommunication().Connect(url, &error);
 
   if (error.Fail())
     return error;
 
   if (client_up->HandshakeWithServer(&error)) {
     m_gdb_client_up = std::move(client_up);
     m_gdb_client_up->GetHostInfo();
     // If a working directory was set prior to connecting, send it down
LLDB_LOGF(
       log,
       "PlatformRemoteGDBServer::%s() set launch architecture triple to '%s'",
       __FUNCTION__, arch_triple ? arch_triple : "<NULL>");
 
   int arg_packet_err;
   {
     // Scope for the scoped timeout object
     process_gdb_remote::GDBRemoteCommunication::ScopedTimeout timeout(
-        *m_gdb_client_up, std::chrono::seconds(5));
+        m_gdb_client_up->GetCommunication(), std::chrono::seconds(5));
     arg_packet_err = m_gdb_client_up->SendArgumentsPacket(launch_info);
   }
 
   if (arg_packet_err == 0) {
     std::string error_str;
     if (m_gdb_client_up->GetLaunchSuccess(error_str)) {
       const auto pid = m_gdb_client_up->GetCurrentProcessID(false);
       if (pid != LLDB_INVALID_PROCESS_ID) {

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

 
 #include "GDBRemoteCommunication.h"
 
 #include <condition_variable>
 
 namespace lldb_private {
 namespace process_gdb_remote {
 
-class GDBRemoteClientBase : public GDBRemoteCommunication, public Broadcaster {
+class GDBRemoteClientBase : public Broadcaster {
 public:
+  typedef GDBRemoteCommunication::PacketResult PacketResult;
+
   enum {
     eBroadcastBitRunPacketSent = (1u << 0),
   };
 
   struct ContinueDelegate {
     virtual ~ContinueDelegate();
     virtual void HandleAsyncStdout(llvm::StringRef out) = 0;
     virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
private:
     GDBRemoteClientBase &m_comm;
     std::chrono::seconds m_interrupt_timeout;
     bool m_acquired;
     bool m_did_interrupt;
 
     void SyncWithContinueThread();
   };
 
+  // Wrappers for GDBRemoteCommunication API.
+  PacketResult ReadPacket(StringExtractorGDBRemote &response,
+                          Timeout<std::micro> timeout, bool sync_on_timeout);
+
+  PacketResult SendPacketNoLock(llvm::StringRef payload);
+
+  size_t SendAck();
+
+  size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status,
+               Status *error_ptr);
+
+  bool IsConnected() const;
+
+  lldb::ConnectionStatus Disconnect(Status *error_ptr = nullptr);
+
+  GDBRemoteCommunication &GetCommunication() {
+    return m_comm;
+  }
+
 protected:
+  GDBRemoteCommunication m_comm;
+
   PacketResult
   SendPacketAndWaitForResponseNoLock(llvm::StringRef payload,
                                      StringExtractorGDBRemote &response);
 
   virtual void OnRunPacketSent(bool first);
 
 private:
   /// Variables handling synchronization between the Continue thread and any

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

 
 /////////////////////////
 // GDBRemoteClientBase //
 /////////////////////////
 
 GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default;
 
 GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name)
-    : GDBRemoteCommunication(), Broadcaster(nullptr, comm_name),
-      m_async_count(0), m_is_running(false), m_should_stop(false) {}
+    : Broadcaster(nullptr, comm_name), m_async_count(0), m_is_running(false),
+      m_should_stop(false) {}
 
 StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
     ContinueDelegate &delegate, const UnixSignals &signals,
     llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
     StringExtractorGDBRemote &response) {
   Log *log = GetLog(GDBRLog::Process);
   response.Clear();
 
if (Log *log = GetLog(GDBRLog::Process))
                 __FUNCTION__, int(payload.size()), payload.data());
     return PacketResult::ErrorSendFailed;
   }
 
   PacketResult packet_result = SendPacketNoLock(payload);
   if (packet_result != PacketResult::Success)
     return packet_result;
 
-  return ReadPacketWithOutputSupport(response, GetPacketTimeout(), true,
+  return ReadPacketWithOutputSupport(response, m_comm.GetPacketTimeout(), true,
                                      output_callback);
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
     llvm::StringRef payload, StringExtractorGDBRemote &response) {
   PacketResult packet_result = SendPacketNoLock(payload);
   if (packet_result != PacketResult::Success)
     return packet_result;
 
   const size_t max_response_retries = 3;
   for (size_t i = 0; i < max_response_retries; ++i) {
-    packet_result = ReadPacket(response, GetPacketTimeout(), true);
+    packet_result = ReadPacket(response, m_comm.GetPacketTimeout(), true);
     // Make sure we received a response
     if (packet_result != PacketResult::Success)
       return packet_result;
     // Make sure our response is valid for the payload that was sent
     if (response.ValidateResponse())
       return packet_result;
     // Response says it wasn't valid
     Log *log = GetLog(GDBRLog::Packets);
GDBRemoteClientBase::Lock::~Lock() {
   if (!m_acquired)
     return;
   {
     std::unique_lock<std::mutex> lock(m_comm.m_mutex);
     --m_comm.m_async_count;
   }
   m_comm.m_cv.notify_one();
 }
+
+GDBRemoteClientBase::PacketResult
+GDBRemoteClientBase::ReadPacket(StringExtractorGDBRemote &response,
+                                Timeout<std::micro> timeout,
+                                bool sync_on_timeout) {
+  return m_comm.ReadPacket(response, timeout, sync_on_timeout);
+}
+
+GDBRemoteClientBase::PacketResult
+GDBRemoteClientBase::SendPacketNoLock(llvm::StringRef payload) {
+  return m_comm.SendPacketNoLock(payload);
+}
+
+size_t GDBRemoteClientBase::SendAck() { return m_comm.SendAck(); }
+
+size_t GDBRemoteClientBase::Write(const void *src, size_t src_len,
+                                  ConnectionStatus &status, Status *error_ptr) {
+  return m_comm.Write(src, src_len, status, error_ptr);
+}
+
+bool GDBRemoteClientBase::IsConnected() const { return m_comm.IsConnected(); }
+
+ConnectionStatus GDBRemoteClientBase::Disconnect(Status *error_ptr) {
+  return m_comm.Disconnect(error_ptr);
+}

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

public:
   void SetPacketRecorder(repro::PacketRecorder *recorder);
 
   static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
                                     GDBRemoteCommunication &server);
 
   /// Expand GDB run-length encoding.
   static std::string ExpandRLE(std::string);
 
+  void EnableqEcho() { m_supports_qEcho = true; }
+
+  void DisableSendingAcks() { m_send_acks = false; }
+
+  void EnableCompression(CompressionType type) { m_compression_type = type; }
+
+  PacketResult ReadPacket(StringExtractorGDBRemote &response,
+                          Timeout<std::micro> timeout, bool sync_on_timeout);
+
+  PacketResult ReadPacketWithOutputSupport(
+      StringExtractorGDBRemote &response, Timeout<std::micro> timeout,
+      bool sync_on_timeout,
+      llvm::function_ref<void(llvm::StringRef)> output_callback);
+
+  PacketResult SendPacketNoLock(llvm::StringRef payload);
+
 protected:
   std::chrono::seconds m_packet_timeout;
   uint32_t m_echo_number;
-  LazyBool m_supports_qEcho;
+  bool m_supports_qEcho;
   GDBRemoteCommunicationHistory m_history;
   bool m_send_acks;
   bool m_is_platform; // Set to true if this class represents a platform,
                       // false if this class represents a debug session for
                       // a single process
 
   std::string m_bytes;
   std::recursive_mutex m_bytes_mutex;
   CompressionType m_compression_type;
 
-  PacketResult SendPacketNoLock(llvm::StringRef payload);
   PacketResult SendNotificationPacketNoLock(llvm::StringRef notify_type,
                                             std::deque<std::string>& queue,
                                             llvm::StringRef payload);
   PacketResult SendRawPacketNoLock(llvm::StringRef payload,
                                    bool skip_ack = false);
 
-  PacketResult ReadPacket(StringExtractorGDBRemote &response,
-                          Timeout<std::micro> timeout, bool sync_on_timeout);
-
   PacketResult WaitForPacketNoLock(StringExtractorGDBRemote &response,
                                    Timeout<std::micro> timeout,
                                    bool sync_on_timeout);
 
   bool CompressionIsEnabled() {
     return m_compression_type != CompressionType::None;
   }
 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

 // GDBRemoteCommunication constructor
 GDBRemoteCommunication::GDBRemoteCommunication()
     : Communication(),
 #ifdef LLDB_CONFIGURATION_DEBUG
       m_packet_timeout(1000),
 #else
       m_packet_timeout(1),
 #endif
-      m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
+      m_echo_number(0), m_supports_qEcho(false), m_history(512),
       m_send_acks(true), m_is_platform(false),
       m_compression_type(CompressionType::None), m_listen_url() {
 }
 
 // Destructor
 GDBRemoteCommunication::~GDBRemoteCommunication() {
   if (IsConnected()) {
     Disconnect();

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

protected:
   LazyBool m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolCalculate;
   LazyBool m_supports_jGetSharedCacheInfo = eLazyBoolCalculate;
   LazyBool m_supports_QPassSignals = eLazyBoolCalculate;
   LazyBool m_supports_error_string_reply = eLazyBoolCalculate;
   LazyBool m_supports_multiprocess = eLazyBoolCalculate;
   LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
   LazyBool m_supports_qSaveCore = eLazyBoolCalculate;
   LazyBool m_uses_native_signals = eLazyBoolCalculate;
+  LazyBool m_supports_qEcho = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
       m_supports_qThreadStopInfo : 1, m_supports_z0 : 1, m_supports_z1 : 1,
       m_supports_z2 : 1, m_supports_z3 : 1, m_supports_z4 : 1,
       m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
       m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
       m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
     GetRemoteQSupported();
   }
   return m_max_packet_size;
 }
 
 bool GDBRemoteCommunicationClient::QueryNoAckModeSupported() {
   if (m_supports_not_sending_acks == eLazyBoolCalculate) {
-    m_send_acks = true;
     m_supports_not_sending_acks = eLazyBoolNo;
 
     // This is the first real packet that we'll send in a debug session and it
     // may take a little longer than normal to receive a reply.  Wait at least
     // 6 seconds for a reply to this packet.
 
-    ScopedTimeout timeout(*this, std::max(GetPacketTimeout(), seconds(6)));
+    GDBRemoteCommunication::ScopedTimeout timeout(
+        m_comm, std::max(m_comm.GetPacketTimeout(), seconds(6)));
 
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse("QStartNoAckMode", response) ==
         PacketResult::Success) {
       if (response.IsOKResponse()) {
-        m_send_acks = false;
+        m_comm.DisableSendingAcks();
         m_supports_not_sending_acks = eLazyBoolYes;
       }
       return true;
     }
   }
   return false;
 }
 
for (llvm::StringRef x : llvm::split(response.GetStringRef(), ';')) {
       } else if (x == "qXfer:libraries:read+")
         m_supports_qXfer_libraries_read = eLazyBoolYes;
       else if (x == "qXfer:features:read+")
         m_supports_qXfer_features_read = eLazyBoolYes;
       else if (x == "qXfer:memory-map:read+")
         m_supports_qXfer_memory_map_read = eLazyBoolYes;
       else if (x == "qXfer:siginfo:read+")
         m_supports_qXfer_siginfo_read = eLazyBoolYes;
-      else if (x == "qEcho")
+      else if (x == "qEcho") {
         m_supports_qEcho = eLazyBoolYes;
-      else if (x == "QPassSignals+")
+        m_comm.EnableqEcho();
+      } else if (x == "QPassSignals+")
         m_supports_QPassSignals = eLazyBoolYes;
       else if (x == "multiprocess+")
         m_supports_multiprocess = eLazyBoolYes;
       else if (x == "memory-tagging+")
         m_supports_memory_tagging = eLazyBoolYes;
       else if (x == "qSaveCore+")
         m_supports_qSaveCore = eLazyBoolYes;
       else if (x == "native-signals+")

 #endif
 
   if (avail_type != CompressionType::None) {
     StringExtractorGDBRemote response;
     std::string packet = "QEnableCompression:type:" + avail_name.str() + ";";
     if (SendPacketAndWaitForResponse(packet, response) != PacketResult::Success)
       return;
 
-    if (response.IsOKResponse()) {
-      m_compression_type = avail_type;
-    }
+    if (response.IsOKResponse())
+      m_comm.EnableCompression(avail_type);
   }
 }
 
 const char *GDBRemoteCommunicationClient::GetGDBServerProgramName() {
   if (GetGDBServerVersion()) {
     if (!m_gdb_server_name.empty())
       return m_gdb_server_name.c_str();
   }

 }
 
 bool GDBRemoteCommunicationClient::GetHostInfo(bool force) {
   Log *log = GetLog(GDBRLog::Process);
 
   if (force || m_qHostInfo_is_valid == eLazyBoolCalculate) {
     // host info computation can require DNS traffic and shelling out to external processes.
     // Increase the timeout to account for that.
-    ScopedTimeout timeout(*this, seconds(10));
+    GDBRemoteCommunication::ScopedTimeout timeout(m_comm, seconds(10));
     m_qHostInfo_is_valid = eLazyBoolNo;
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse("qHostInfo", response) ==
         PacketResult::Success) {
       if (response.IsNormalResponse()) {
         llvm::StringRef name;
         llvm::StringRef value;
         uint32_t cpu = LLDB_INVALID_CPUTYPE;
if (SendPacketAndWaitForResponse("qHostInfo", response) ==
                     .Case("after", eLazyBoolYes)
                     .Default(eLazyBoolCalculate);
             if (m_watchpoints_trigger_after_instruction != eLazyBoolCalculate)
               ++num_keys_decoded;
           } else if (name.equals("default_packet_timeout")) {
             uint32_t timeout_seconds;
             if (!value.getAsInteger(0, timeout_seconds)) {
               m_default_packet_timeout = seconds(timeout_seconds);
-              SetPacketTimeout(m_default_packet_timeout);
+              m_comm.SetPacketTimeout(m_default_packet_timeout);
               ++num_keys_decoded;
             }
           } else if (name.equals("vm-page-size")) {
             int page_size;
             if (!value.getAsInteger(0, page_size)) {
               m_target_vm_page_size = page_size;
               ++num_keys_decoded;
             }
if (!match_info.MatchAllProcesses()) {
         packet.PutCString("triple:");
         packet.PutCString(triple.getTriple());
         packet.PutChar(';');
       }
     }
     StringExtractorGDBRemote response;
     // Increase timeout as the first qfProcessInfo packet takes a long time on
     // Android. The value of 1min was arrived at empirically.
-    ScopedTimeout timeout(*this, minutes(1));
+    GDBRemoteCommunication::ScopedTimeout timeout(m_comm, minutes(1));
     if (SendPacketAndWaitForResponse(packet.GetString(), response) ==
         PacketResult::Success) {
       do {
         ProcessInstanceInfo process_info;
         if (!DecodeProcessInfoResponse(response, process_info))
           break;
         process_infos.push_back(process_info);
         response = StringExtractorGDBRemote();
if (HostInfo::GetHostname(hostname)) {
       stream.Printf("host:%s;", hostname.c_str());
     } else {
       // Make the GDB server we launch accept connections from any host since
       // we can't figure out the hostname
       stream.Printf("host:*;");
     }
   }
   // give the process a few seconds to startup
-  ScopedTimeout timeout(*this, seconds(10));
+  GDBRemoteCommunication::ScopedTimeout timeout(m_comm, seconds(10));
 
   if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
       PacketResult::Success) {
     llvm::StringRef name;
     llvm::StringRef value;
     while (response.GetNameColonValue(name, value)) {
       if (name.equals("port"))
         value.getAsInteger(0, port);
GDBRemoteCommunicationClient::GetModulesInfo(
   unescaped_payload.PutCString("jModulesInfo:");
   unescaped_payload.AsRawOstream() << std::move(module_array);
 
   StreamGDBRemote payload;
   payload.PutEscapedBytes(unescaped_payload.GetString().data(),
                           unescaped_payload.GetSize());
 
   // Increase the timeout for jModulesInfo since this packet can take longer.
-  ScopedTimeout timeout(*this, std::chrono::seconds(10));
+  GDBRemoteCommunication::ScopedTimeout timeout(m_comm,
+                                                std::chrono::seconds(10));
 
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse(payload.GetString(), response) !=
           PacketResult::Success ||
       response.IsErrorResponse())
     return llvm::None;
 
   if (response.IsUnsupportedResponse()) {
bool GDBRemoteCommunicationClient::UsesNativeSignals() {
 
   // If the remote didn't indicate native-signal support explicitly,
   // check whether it is an old version of lldb-server.
   return GetThreadSuffixSupported();
 }
 
 llvm::Expected<int> GDBRemoteCommunicationClient::KillProcess(lldb::pid_t pid) {
   StringExtractorGDBRemote response;
-  GDBRemoteCommunication::ScopedTimeout(*this, seconds(3));
+  GDBRemoteCommunication::ScopedTimeout(m_comm, seconds(3));
 
-  if (SendPacketAndWaitForResponse("k", response, GetPacketTimeout()) !=
+  if (SendPacketAndWaitForResponse("k", response, m_comm.GetPacketTimeout()) !=
       PacketResult::Success)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "failed to send k packet");
 
   char packet_cmd = response.GetChar(0);
   if (packet_cmd == 'W' || packet_cmd == 'X')
     return response.GetHexU8();
 
   return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                  "unexpected response to k packet: %s",
                                  response.GetStringRef().str().c_str());
 }

lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

if (lock) {
 
         return success;
       }
     } else {
       Log *log(GetLog(GDBRLog::Thread | GDBRLog::Packets));
       if (log) {
         if (log->GetVerbose()) {
           StreamString strm;
-          gdb_comm.DumpHistory(strm);
+          gdb_comm.GetCommunication().DumpHistory(strm);
           LLDB_LOGF(log,
                     "error: failed to get packet sequence mutex, not sending "
                     "write register for \"%s\":\n%s",
                     reg_info->name, strm.GetData());
         } else
           LLDB_LOGF(log,
                     "error: failed to get packet sequence mutex, not sending "
                     "write register for \"%s\"",
data_sp = std::make_shared<DataBufferHeap>(
         m_reg_data.GetDataStart(), m_reg_info_sp->GetRegisterDataByteSize());
     return true;
   } else {
 
     Log *log(GetLog(GDBRLog::Thread | GDBRLog::Packets));
     if (log) {
       if (log->GetVerbose()) {
         StreamString strm;
-        gdb_comm.DumpHistory(strm);
+        gdb_comm.GetCommunication().DumpHistory(strm);
         LLDB_LOGF(log,
                   "error: failed to get packet sequence mutex, not sending "
                   "read all registers:\n%s",
                   strm.GetData());
       } else
         LLDB_LOGF(log,
                   "error: failed to get packet sequence mutex, not sending "
                   "read all registers");
if (use_g_packet) {
       }
       return num_restored > 0;
     }
   } else {
     Log *log(GetLog(GDBRLog::Thread | GDBRLog::Packets));
     if (log) {
       if (log->GetVerbose()) {
         StreamString strm;
-        gdb_comm.DumpHistory(strm);
+        gdb_comm.GetCommunication().DumpHistory(strm);
         LLDB_LOGF(log,
                   "error: failed to get packet sequence mutex, not sending "
                   "write all registers:\n%s",
                   strm.GetData());
       } else
         LLDB_LOGF(log,
                   "error: failed to get packet sequence mutex, not sending "
                   "write all registers");

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 void DumpProcessGDBRemotePacketHistory(void *p, const char *path) {
   auto file = FileSystem::Instance().Open(
       FileSpec(path), File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
   if (!file) {
     llvm::consumeError(file.takeError());
     return;
   }
   StreamFile stream(std::move(file.get()));
-  ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(stream);
+  ((ProcessGDBRemote *)p)
+      ->GetGDBRemote()
+      .GetCommunication()
+      .DumpHistory(stream);
 }
 } // namespace lldb
 
 namespace {
 
 #define LLDB_PROPERTIES_processgdbremote
 #include "ProcessGDBRemoteProperties.inc"
 
LLDB_LOGF(log,
               "ProcessGDBRemote::%s failed to listen for "
               "m_async_broadcaster events",
               __FUNCTION__);
   }
 
   const uint64_t timeout_seconds =
       GetGlobalPluginProperties().GetPacketTimeout();
   if (timeout_seconds > 0)
-    m_gdb_comm.SetPacketTimeout(std::chrono::seconds(timeout_seconds));
+    m_gdb_comm.GetCommunication().SetPacketTimeout(
+        std::chrono::seconds(timeout_seconds));
 
   m_use_g_packet_for_reading =
       GetGlobalPluginProperties().GetUseGPacketForReading();
 }
 
 // Destructor
 ProcessGDBRemote::~ProcessGDBRemote() {
   //  m_mach_process.UnregisterNotificationCallbacks (this);
if (working_dir) {
       m_gdb_comm.SetWorkingDir(working_dir);
     }
 
     // Send the environment and the program + arguments after we connect
     m_gdb_comm.SendEnvironment(launch_info.GetEnvironment());
 
     {
       // Scope for the scoped timeout object
-      GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
-                                                    std::chrono::seconds(10));
+      GDBRemoteCommunication::ScopedTimeout timeout(
+          m_gdb_comm.GetCommunication(), std::chrono::seconds(10));
 
       int arg_packet_err = m_gdb_comm.SendArgumentsPacket(launch_info);
       if (arg_packet_err == 0) {
         std::string error_str;
         if (m_gdb_comm.GetLaunchSuccess(error_str)) {
           SetID(m_gdb_comm.GetCurrentProcessID());
         } else {
           error.SetErrorString(error_str.c_str());
LLDB_LOGF(log, "ProcessGDBRemote::%s Connecting to %s", __FUNCTION__,
               connect_url.str().c_str());
     std::unique_ptr<ConnectionFileDescriptor> conn_up(
         new ConnectionFileDescriptor());
     if (conn_up) {
       const uint32_t max_retry_count = 50;
       uint32_t retry_count = 0;
       while (!m_gdb_comm.IsConnected()) {
         if (conn_up->Connect(connect_url, &error) == eConnectionStatusSuccess) {
-          m_gdb_comm.SetConnection(std::move(conn_up));
+          m_gdb_comm.GetCommunication().SetConnection(std::move(conn_up));
           break;
         }
 
         retry_count++;
 
         if (retry_count >= max_retry_count)
           break;
 
#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
     auto cleanup_our = llvm::make_scope_exit([&]() { close(our_socket); });
     auto cleanup_gdb = llvm::make_scope_exit([&]() { close(gdb_socket); });
 
     // Don't let any child processes inherit our communication socket
     SetCloexecFlag(our_socket);
     communication_fd = gdb_socket;
 #endif
 
-    error = m_gdb_comm.StartDebugserverProcess(
+    error = m_gdb_comm.GetCommunication().StartDebugserverProcess(
         nullptr, GetTarget().GetPlatform().get(), debugserver_launch_info,
         nullptr, nullptr, communication_fd);
 
     if (error.Success())
       m_debugserver_pid = debugserver_launch_info.GetProcessID();
     else
       m_debugserver_pid = LLDB_INVALID_PROCESS_ID;
 
     if (m_debugserver_pid != LLDB_INVALID_PROCESS_ID) {
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
       // Our process spawned correctly, we can now set our connection to use
       // our end of the socket pair
       cleanup_our.release();
-      m_gdb_comm.SetConnection(
+      m_gdb_comm.GetCommunication().SetConnection(
           std::make_unique<ConnectionFileDescriptor>(our_socket, true));
 #endif
       StartAsyncThread();
     }
 
     if (error.Fail()) {
       Log *log = GetLog(GDBRLog::Process);
 

 
 StructuredData::ObjectSP
 ProcessGDBRemote::GetLoadedDynamicLibrariesInfos_sender(
     StructuredData::ObjectSP args_dict) {
   StructuredData::ObjectSP object_sp;
 
   if (m_gdb_comm.GetLoadedDynamicLibrariesInfosSupported()) {
     // Scope for the scoped timeout object
-    GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
+    GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm.GetCommunication(),
                                                   std::chrono::seconds(10));
 
     StreamString packet;
     packet << "jGetLoadedDynamicLibrariesInfos:";
     args_dict->Dump(packet, false);
 
     // FIXME the final character of a JSON dictionary, '}', is the escape
     // character in gdb-remote binary mode.  lldb currently doesn't escape
CommandObjectProcessGDBRemotePacketHistory(CommandInterpreter &interpreter)
                             "Dumps the packet history buffer. ", nullptr) {}
 
   ~CommandObjectProcessGDBRemotePacketHistory() override = default;
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     ProcessGDBRemote *process =
         (ProcessGDBRemote *)m_interpreter.GetExecutionContext().GetProcessPtr();
     if (process) {
-      process->GetGDBRemote().DumpHistory(result.GetOutputStream());
+      process->GetGDBRemote().GetCommunication().DumpHistory(
+          result.GetOutputStream());
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
     result.SetStatus(eReturnStatusFailed);
     return false;
   }
 };
 

lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp

struct MockDelegate : public GDBRemoteClientBase::ContinueDelegate {
 
   void HandleAsyncStructuredDataPacket(llvm::StringRef data) override {
     structured_data_packets.push_back(std::string(data));
   }
 };
 
 struct TestClient : public GDBRemoteClientBase {
   TestClient() : GDBRemoteClientBase("test.client") {
-    m_send_acks = false;
+    GetCommunication().DisableSendingAcks();
   }
 };
 
 class GDBRemoteClientBaseTest : public GDBRemoteTest {
 public:
   void SetUp() override {
-    ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
+    ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(
+                          client.GetCommunication(), server),
                       llvm::Succeeded());
     ASSERT_EQ(TestClient::eBroadcastBitRunPacketSent,
               listener_sp->StartListeningForEvents(
                   &client, TestClient::eBroadcastBitRunPacketSent));
   }
 
 protected:
   // We don't have a process to get the interrupt timeout from, so make one up.

lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

 using namespace lldb;
 using namespace llvm;
 
 namespace {
 
 typedef GDBRemoteCommunication::PacketResult PacketResult;
 
 struct TestClient : public GDBRemoteCommunicationClient {
-  TestClient() { m_send_acks = false; }
+  TestClient() {
+    GetCommunication().DisableSendingAcks();
+  }
 };
 
 void Handle_QThreadSuffixSupported(MockServer &server, bool supported) {
   StringExtractorGDBRemote request;
   ASSERT_EQ(PacketResult::Success, server.GetPacket(request));
   ASSERT_EQ("QThreadSuffixSupported", request.GetStringRef());
   if (supported)
     ASSERT_EQ(PacketResult::Success, server.SendOKResponse());

 uint8_t one_register[] = {'A', 'B', 'C', 'D'};
 std::string one_register_hex = "41424344";
 
 } // end anonymous namespace
 
 class GDBRemoteCommunicationClientTest : public GDBRemoteTest {
 public:
   void SetUp() override {
-    ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
+    ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(
+                          client.GetCommunication(), server),
                       llvm::Succeeded());
   }
 
 protected:
   TestClient client;
   MockServer server;
 };
 

lldb/unittests/tools/lldb-server/tests/TestClient.cpp

 using namespace llvm;
 using namespace llgs_tests;
 
 #ifdef SendMessage
 #undef SendMessage
 #endif
 
 TestClient::TestClient(std::unique_ptr<Connection> Conn) {
-  SetConnection(std::move(Conn));
-  SetPacketTimeout(std::chrono::seconds(10));
+  GetCommunication().SetConnection(std::move(Conn));
+  GetCommunication().SetPacketTimeout(std::chrono::seconds(10));
 }
 
 TestClient::~TestClient() {
   if (!IsConnected())
     return;
 
   EXPECT_THAT_ERROR(SendMessage("k"), Succeeded());
 }
 
 Error TestClient::initializeConnection() {
   if (SendAck() == 0)
     return make_error<StringError>("Sending initial ACK failed.",
                                    inconvertibleErrorCode());
 
   if (Error E = SendMessage("QStartNoAckMode"))
     return E;
 
-  m_send_acks = false;
+  GetCommunication().DisableSendingAcks();
   return Error::success();
 }
 
 Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log) {
   return launch(Log, {});
 }
 
 Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef<StringRef> InferiorArgs) {
Error TestClient::Continue(StringRef message) {
   auto StopReplyOr = SendMessage<StopReply>(
       message, m_process_info->GetEndian(), m_register_infos);
   if (!StopReplyOr)
     return StopReplyOr.takeError();
 
   m_stop_reply = std::move(*StopReplyOr);
   if (!isa<StopReplyStop>(m_stop_reply)) {
     StringExtractorGDBRemote R;
-    PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
+    PacketResult result =
+        ReadPacket(R, GetCommunication().GetPacketTimeout(), false);
     if (result != PacketResult::ErrorDisconnected) {
       return make_error<StringError>(
           formatv("Expected connection close after sending {0}. Got {1}/{2} "
                   "instead.",
                   message, result, R.GetStringRef())
               .str(),
           inconvertibleErrorCode());
     }
   }
   return Error::success();
 }