From: Michał Górny <mgorny@moritz.systems>
Date: Wed, 3 Aug 2022 17:09:35 +0000 (+0200)
Subject: Revert "[lldb] [gdb-remote] Send interrupt packets from async thread"
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3426fc7318fe555ee37db14525e9bf024760fde2;p=platform%2Fupstream%2Fllvm.git

Revert "[lldb] [gdb-remote] Send interrupt packets from async thread"

This reverts commit 446b61cff4ea0cb7e7fcc5e0fec7bc749d379b08.  Of course
it does not work on Windows.
---

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index f4bf2d1..e3a3cfc 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -11,7 +11,6 @@
 #include "llvm/ADT/StringExtras.h"
 
 #include "lldb/Target/UnixSignals.h"
-#include "lldb/Utility/Connection.h"
 #include "lldb/Utility/LLDBAssert.h"
 
 #include "ProcessGDBRemoteLog.h"
@@ -53,15 +52,13 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
   if (!cont_lock)
     return eStateInvalid;
   OnRunPacketSent(true);
-  // The main ReadPacket loop wakes up at computed_timeout intervals, just to
+  // The main ReadPacket loop wakes up at computed_timeout intervals, just to 
   // check that the connection hasn't dropped.  When we wake up we also check
   // whether there is an interrupt request that has reached its endpoint.
-  // If we want a shorter interrupt timeout that kWakeupInterval, we need to
+  // If we want a shorter interrupt timeout that kWakeupInterval, we need to 
   // choose the shorter interval for the wake up as well.
-  std::chrono::seconds computed_timeout =
-      std::min(interrupt_timeout, kWakeupInterval);
-  std::chrono::time_point<std::chrono::steady_clock> interrupt_endpoint;
-  bool interrupt_sent = false;
+  std::chrono::seconds computed_timeout = std::min(interrupt_timeout, 
+                                                   kWakeupInterval);
   for (;;) {
     PacketResult read_result = ReadPacket(response, computed_timeout, false);
     // Reset the computed_timeout to the default value in case we are going
@@ -73,35 +70,16 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
       if (m_async_count == 0) {
         continue;
       }
-      if (!interrupt_sent) {
-        const char ctrl_c = '\x03';
-        ConnectionStatus status = eConnectionStatusSuccess;
-        size_t bytes_written = Write(&ctrl_c, 1, status, nullptr);
-        if (bytes_written == 0) {
-          LLDB_LOG(log, "failed to send interrupt packet");
-          return eStateInvalid;
-        }
-        interrupt_endpoint = steady_clock::now() + interrupt_timeout;
-        if (log)
-          log->PutCString(
-              "GDBRemoteClientBase::SendContinuePacketAndWaitForResponse sent "
-              "packet: \\x03");
-
-        interrupt_sent = true;
-        continue;
-      }
-
       auto cur_time = steady_clock::now();
-      if (cur_time >= interrupt_endpoint)
+      if (cur_time >= m_interrupt_endpoint)
         return eStateInvalid;
       else {
         // We woke up and found an interrupt is in flight, but we haven't
         // exceeded the interrupt wait time.  So reset the wait time to the
         // time left till the interrupt timeout.  But don't wait longer
         // than our wakeup timeout.
-        auto new_wait = interrupt_endpoint - cur_time;
-        computed_timeout = std::min(
-            kWakeupInterval,
+        auto new_wait = m_interrupt_endpoint - cur_time;
+        computed_timeout = std::min(kWakeupInterval,
             std::chrono::duration_cast<std::chrono::seconds>(new_wait));
         continue;
       }
@@ -369,6 +347,7 @@ GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm,
 }
 
 void GDBRemoteClientBase::Lock::SyncWithContinueThread() {
+  Log *log = GetLog(GDBRLog::Process|GDBRLog::Packets);
   std::unique_lock<std::mutex> lock(m_comm.m_mutex);
   if (m_comm.m_is_running && m_interrupt_timeout == std::chrono::seconds(0))
     return; // We were asked to avoid interrupting the sender. Lock is not
@@ -376,11 +355,22 @@ void GDBRemoteClientBase::Lock::SyncWithContinueThread() {
 
   ++m_comm.m_async_count;
   if (m_comm.m_is_running) {
-    // SendContinuePacketAndWaitForResponse() takes care of sending
-    // the actual interrupt packet since we've increased m_async_count.
-    // Interrupt the ReadPacket() call to avoid having to wait for
-    // the interrupt timeout.
-    m_comm.GetConnection()->InterruptRead();
+    if (m_comm.m_async_count == 1) {
+      // The sender has sent the continue packet and we are the first async
+      // packet. Let's interrupt it.
+      const char ctrl_c = '\x03';
+      ConnectionStatus status = eConnectionStatusSuccess;
+      size_t bytes_written = m_comm.Write(&ctrl_c, 1, status, nullptr);
+      if (bytes_written == 0) {
+        --m_comm.m_async_count;
+        LLDB_LOGF(log, "GDBRemoteClientBase::Lock::Lock failed to send "
+                       "interrupt packet");
+        return;
+      }
+      m_comm.m_interrupt_endpoint = steady_clock::now() + m_interrupt_timeout;
+      if (log)
+        log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03");
+    }
     m_comm.m_cv.wait(lock, [this] { return !m_comm.m_is_running; });
     m_did_interrupt = true;
   }
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index abd93fd..43a5313 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -121,6 +121,10 @@ private:
   /// an async thread e.g. to inject a signal.
   std::string m_continue_packet;
 
+  /// When was the interrupt packet sent. Used to make sure we time out if the
+  /// stub does not respond to interrupt requests.
+  std::chrono::time_point<std::chrono::steady_clock> m_interrupt_endpoint;
+
   /// Number of threads interested in sending.
   uint32_t m_async_count;