From 3426fc7318fe555ee37db14525e9bf024760fde2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20G=C3=B3rny?= Date: Wed, 3 Aug 2022 19:09:35 +0200 Subject: [PATCH] Revert "[lldb] [gdb-remote] Send interrupt packets from async thread" This reverts commit 446b61cff4ea0cb7e7fcc5e0fec7bc749d379b08. Of course it does not work on Windows. --- .../Process/gdb-remote/GDBRemoteClientBase.cpp | 58 +++++++++------------- .../Process/gdb-remote/GDBRemoteClientBase.h | 4 ++ 2 files changed, 28 insertions(+), 34 deletions(-) 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 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(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 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 m_interrupt_endpoint; + /// Number of threads interested in sending. uint32_t m_async_count; -- 2.7.4