Create ScopedTimeout class for GDBRemoteCommunication

[tberghammer]

Summary

Create ScopedTimeout class for GDBRemoteCommunication

This new class makes it easier to change the timeout of a GDBRemoteCommunication instance for a short time and then restore it to its original value.

Requested by @ovyalov in D7788

[clayborg]

Looks good.

[ovyalov]

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

934 ↗

(On Diff #20501)

You may put this statement in initializer list and make m_saved_timeout const;

[tberghammer]

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

934 ↗

(On Diff #20501)

I would prefer to leave it here because I think it is a bit cleaner (don't like complex expressions in the initializer list) and making it const don't make that big effect in a such simple class.

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

PlatformRemoteGDBServer::LaunchProcess (ProcessLaunchInfo &launch_info)
     
     ArchSpec arch_spec = launch_info.GetArchitecture();
     const char *arch_triple = arch_spec.GetTriple().str().c_str();
     
     m_gdb_client.SendLaunchArchPacket(arch_triple);
     if (log)
         log->Printf ("PlatformRemoteGDBServer::%s() set launch architecture triple to '%s'", __FUNCTION__, arch_triple ? arch_triple : "<NULL>");
 
-    const uint32_t old_packet_timeout = m_gdb_client.SetPacketTimeout (5);
-    int arg_packet_err = m_gdb_client.SendArgumentsPacket (launch_info);
-    m_gdb_client.SetPacketTimeout (old_packet_timeout);
+    int arg_packet_err;
+    {
+        // Scope for the scoped timeout object
+        GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_client, 5);
+        arg_packet_err = m_gdb_client.SendArgumentsPacket (launch_info);
+    }
+
     if (arg_packet_err == 0)
     {
         std::string error_str;
         if (m_gdb_client.GetLaunchSuccess (error_str))
         {
             const auto pid = m_gdb_client.GetCurrentProcessID (false);
             if (pid != LLDB_INVALID_PROCESS_ID)
             {

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

enum class PacketResult
         ErrorSendAck,       // Didn't get an ack back after sending a packet
         ErrorReplyFailed,   // Error getting the reply
         ErrorReplyTimeout,  // Timed out waiting for reply
         ErrorReplyInvalid,  // Got a reply but it wasn't valid for the packet that was sent
         ErrorReplyAck,      // Sending reply ack failed
         ErrorDisconnected,  // We were disconnected
         ErrorNoSequenceLock // We couldn't get the sequence lock for a multi-packet request
     };
+
+    // Class to change the timeout for a given scope and restore it to the original value when the
+    // created ScopedTimeout object got out of scope
+    class ScopedTimeout
+    {
+    public:
+        ScopedTimeout (GDBRemoteCommunication& gdb_comm, uint32_t timeout);
+        ~ScopedTimeout ();
+
+    private:
+        GDBRemoteCommunication& m_gdb_comm;
+        uint32_t m_saved_timeout;
+    };
+
     //------------------------------------------------------------------
     // Constructors and Destructors
     //------------------------------------------------------------------
     GDBRemoteCommunication(const char *comm_name, 
                            const char *listener_name);
 
     virtual
     ~GDBRemoteCommunication();

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

#endif
     return error;
 }
 
 void
 GDBRemoteCommunication::DumpHistory(Stream &strm)
 {
     m_history.Dump (strm);
 }
+
+GDBRemoteCommunication::ScopedTimeout::ScopedTimeout (GDBRemoteCommunication& gdb_comm,
+                                                      uint32_t timeout) :
+    m_gdb_comm (gdb_comm)
+{
+    m_saved_timeout = m_gdb_comm.SetPacketTimeout (timeout);
+}
+
+GDBRemoteCommunication::ScopedTimeout::~ScopedTimeout ()
+{
+    m_gdb_comm.SetPacketTimeout (m_saved_timeout);
+}

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

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.
 
         const uint32_t minimum_timeout = 6;
         uint32_t old_timeout = GetPacketTimeoutInMicroSeconds() / lldb_private::TimeValue::MicroSecPerSec;
-        SetPacketTimeout (std::max (old_timeout, minimum_timeout));
+        GDBRemoteCommunication::ScopedTimeout timeout (*this, std::max (old_timeout, minimum_timeout));
 
         StringExtractorGDBRemote response;
-        PacketResult packet_send_result = SendPacketAndWaitForResponse("QStartNoAckMode", response, false);
-        SetPacketTimeout (old_timeout);
-
-        if (packet_send_result == PacketResult::Success)
+        if (SendPacketAndWaitForResponse("QStartNoAckMode", response, false) == PacketResult::Success)
         {
             if (response.IsOKResponse())
             {
                 m_send_acks = false;
                 m_supports_not_sending_acks = eLazyBoolYes;
             }
             return true;
         }
else
             // Make the GDB server we launch accept connections from any host since we can't figure out the hostname
             stream.Printf("host:*;");
         }
     }
     const char *packet = stream.GetData();
     int packet_len = stream.GetSize();
 
     // give the process a few seconds to startup
-    const uint32_t old_packet_timeout = SetPacketTimeout (10);
-    auto result = SendPacketAndWaitForResponse(packet, packet_len, response, false);
-    SetPacketTimeout (old_packet_timeout);
-    if (result == PacketResult::Success)
+    GDBRemoteCommunication::ScopedTimeout timeout (*this, 10);
+    
+    if (SendPacketAndWaitForResponse(packet, packet_len, response, false) == PacketResult::Success)
     {
         std::string name;
         std::string value;
         uint16_t port = 0;
         while (response.GetNameColonValue(name, value))
         {
             if (name.compare("port") == 0)
                 port = StringConvert::ToUInt32(value.c_str(), 0, 0);

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

if (object_file)
                 for (size_t i=0; i<num_environment_entries; ++i)
                 {
                     const char *env_entry = environment.GetArgumentAtIndex(i);
                     if (env_entry == NULL || m_gdb_comm.SendEnvironmentPacket(env_entry) != 0)
                         break;
                 }
             }
 
-            const uint32_t old_packet_timeout = m_gdb_comm.SetPacketTimeout (10);
+            {
+                // Scope for the scoped timeout object
+                GDBRemoteCommunication::ScopedTimeout timeout (m_gdb_comm, 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());
                     }
                 }
                 else
                 {
                     error.SetErrorStringWithFormat("'A' packet returned an error: %i", arg_packet_err);
                 }
-            
-            m_gdb_comm.SetPacketTimeout (old_packet_timeout);
-                
+            }
             if (GetID() == LLDB_INVALID_PROCESS_ID)
             {
                 if (log)
                     log->Printf("failed to connect to debugserver: %s", error.AsCString());
                 KillDebugserverProcess ();
                 return error;
             }
 
ProcessGDBRemote::DoDestroy ()
     // Interrupt if our inferior is running...
     int exit_status = SIGABRT;
     std::string exit_string;
 
     if (m_gdb_comm.IsConnected())
     {
         if (m_public_state.GetValue() != eStateAttaching)
         {
-
             StringExtractorGDBRemote response;
             bool send_async = true;
-            const uint32_t old_packet_timeout = m_gdb_comm.SetPacketTimeout (3);
+            GDBRemoteCommunication::ScopedTimeout (m_gdb_comm, 3);
 
             if (m_gdb_comm.SendPacketAndWaitForResponse("k", 1, response, send_async) == GDBRemoteCommunication::PacketResult::Success)
             {
                 char packet_cmd = response.GetChar(0);
 
                 if (packet_cmd == 'W' || packet_cmd == 'X')
                 {
 #if defined(__APPLE__)
#endif
                 }
             }
             else
             {
                 if (log)
                     log->Printf ("ProcessGDBRemote::DoDestroy - failed to send k packet");
                 exit_string.assign("failed to send the k packet");
             }
-
-            m_gdb_comm.SetPacketTimeout(old_packet_timeout);
         }
         else
         {
             if (log)
                 log->Printf ("ProcessGDBRemote::DoDestroy - killed or interrupted while attaching");
             exit_string.assign ("killed or interrupted while attaching.");
         }
     }