[Lldb-commits] [lldb] 446b61c - [lldb] [gdb-remote] Send interrupt packets from async thread
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 3 09:40:37 PDT 2022
Author: Michał Górny
Date: 2022-08-03T18:40:25+02:00
New Revision: 446b61cff4ea0cb7e7fcc5e0fec7bc749d379b08
URL: https://github.com/llvm/llvm-project/commit/446b61cff4ea0cb7e7fcc5e0fec7bc749d379b08
DIFF: https://github.com/llvm/llvm-project/commit/446b61cff4ea0cb7e7fcc5e0fec7bc749d379b08.diff
LOG: [lldb] [gdb-remote] Send interrupt packets from async thread
Refactor the mechanism for sending interrupt packets to send them
from async thread (i.e. the same thread that sends the continue packet
preceding them and that waits for the response), rather than from
the thread requesting the interrupt. This is going to become especially
important when using the vCtrlC packet as part of the non-stop protocol,
as -- unlike the simple ^c sent in the all-stop mode -- this packet
involves an explicit reply.
Suggested by Pavel Labath in D126614.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131075
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index e3a3cfc4f23ea..f4bf2d164449b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -11,6 +11,7 @@
#include "llvm/ADT/StringExtras.h"
#include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/Connection.h"
#include "lldb/Utility/LLDBAssert.h"
#include "ProcessGDBRemoteLog.h"
@@ -52,13 +53,15 @@ 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::seconds computed_timeout =
+ std::min(interrupt_timeout, kWakeupInterval);
+ std::chrono::time_point<std::chrono::steady_clock> interrupt_endpoint;
+ bool interrupt_sent = false;
for (;;) {
PacketResult read_result = ReadPacket(response, computed_timeout, false);
// Reset the computed_timeout to the default value in case we are going
@@ -70,16 +73,35 @@ 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 >= m_interrupt_endpoint)
+ if (cur_time >= 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 = m_interrupt_endpoint - cur_time;
- computed_timeout = std::min(kWakeupInterval,
+ auto new_wait = interrupt_endpoint - cur_time;
+ computed_timeout = std::min(
+ kWakeupInterval,
std::chrono::duration_cast<std::chrono::seconds>(new_wait));
continue;
}
@@ -347,7 +369,6 @@ 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
@@ -355,22 +376,11 @@ void GDBRemoteClientBase::Lock::SyncWithContinueThread() {
++m_comm.m_async_count;
if (m_comm.m_is_running) {
- 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");
- }
+ // 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();
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 43a5313eae6ad..abd93fd249476 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -121,10 +121,6 @@ class GDBRemoteClientBase : public GDBRemoteCommunication {
/// 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;
More information about the lldb-commits
mailing list