Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -21,7 +21,7 @@ using namespace lldb_private; using namespace lldb_private::process_gdb_remote; -static const std::chrono::seconds kInterruptTimeout(5); +static const std::chrono::microseconds kInterruptTimeout(5000000); // 5s ///////////////////////// // GDBRemoteClientBase // @@ -51,11 +51,7 @@ OnRunPacketSent(true); for (;;) { - PacketResult read_result = ReadPacket( - response, - std::chrono::duration_cast(kInterruptTimeout) - .count(), - false); + PacketResult read_result = ReadPacket(response, kInterruptTimeout, false); switch (read_result) { case PacketResult::ErrorReplyTimeout: { std::lock_guard lock(m_mutex); @@ -188,11 +184,7 @@ const size_t max_response_retries = 3; for (size_t i = 0; i < max_response_retries; ++i) { - packet_result = ReadPacket( - response, std::chrono::duration_cast( - GetPacketTimeout()) - .count(), - true); + packet_result = ReadPacket(response, GetPacketTimeout(), true); // Make sure we received a response if (packet_result != PacketResult::Success) return packet_result; @@ -232,7 +224,7 @@ OnRunPacketSent(true); // wait for the response to the vCont - if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success) { + if (ReadPacket(response, llvm::None, false) == PacketResult::Success) { if (response.IsOKResponse()) return true; } @@ -254,8 +246,8 @@ // additional packet to make sure the packet sequence does not get // skewed. StringExtractorGDBRemote extra_stop_reply_packet; - uint32_t timeout_usec = 100000; // 100ms - ReadPacket(extra_stop_reply_packet, timeout_usec, false); + std::chrono::microseconds timeout(100000); // 100ms + ReadPacket(extra_stop_reply_packet, timeout, false); // Interrupting is typically done using SIGSTOP or SIGINT, so if // the process stops with some other signal, we definitely want to @@ -268,11 +260,9 @@ // We probably only stopped to perform some async processing, so continue // after that is done. // TODO: This is not 100% correct, as the process may have been stopped with - // SIGINT or SIGSTOP - // that was not caused by us (e.g. raise(SIGINT)). This will normally cause a - // stop, but if it's - // done concurrently with a async interrupt, that stop will get eaten - // (llvm.org/pr20231). + // SIGINT or SIGSTOP that was not caused by us (e.g. raise(SIGINT)). This will + // normally cause a stop, but if it's done concurrently with a async + // interrupt, that stop will get eaten (llvm.org/pr20231). return false; } Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -83,12 +83,12 @@ class ScopedTimeout { public: ScopedTimeout(GDBRemoteCommunication &gdb_comm, - std::chrono::seconds timeout); + std::chrono::microseconds timeout); ~ScopedTimeout(); private: GDBRemoteCommunication &m_gdb_comm; - std::chrono::seconds m_saved_timeout; + std::chrono::microseconds m_saved_timeout; }; GDBRemoteCommunication(const char *comm_name, const char *listener_name); @@ -115,13 +115,16 @@ // packets and waiting for responses. For servers, this is used when waiting // for ACKs. //------------------------------------------------------------------ - std::chrono::seconds SetPacketTimeout(std::chrono::seconds packet_timeout) { + std::chrono::microseconds + SetPacketTimeout(std::chrono::microseconds packet_timeout) { const auto old_packet_timeout = m_packet_timeout; m_packet_timeout = packet_timeout; return old_packet_timeout; } - std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; } + std::chrono::microseconds GetPacketTimeout() const { + return m_packet_timeout; + } //------------------------------------------------------------------ // Start a debugserver instance on the current host using the @@ -212,7 +215,7 @@ mutable bool m_dumped_to_log; }; - std::chrono::seconds m_packet_timeout; + std::chrono::microseconds m_packet_timeout; uint32_t m_echo_number; LazyBool m_supports_qEcho; History m_history; @@ -226,16 +229,18 @@ PacketResult SendPacketNoLock(llvm::StringRef payload); PacketResult ReadPacket(StringExtractorGDBRemote &response, - uint32_t timeout_usec, bool sync_on_timeout); + llvm::Optional timeout, + bool sync_on_timeout); // Pop a packet from the queue in a thread safe manner - PacketResult PopPacketFromQueue(StringExtractorGDBRemote &response, - uint32_t timeout_usec); + PacketResult + PopPacketFromQueue(StringExtractorGDBRemote &response, + llvm::Optional timeout); PacketResult - WaitForPacketWithTimeoutMicroSecondsNoLock(StringExtractorGDBRemote &response, - uint32_t timeout_usec, - bool sync_on_timeout); + WaitForPacketNoLock(StringExtractorGDBRemote &response, + llvm::Optional timeout, + bool sync_on_timeout); bool CompressionIsEnabled() { return m_compression_type != CompressionType::None; Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -133,9 +133,9 @@ const char *listener_name) : Communication(comm_name), #ifdef LLDB_CONFIGURATION_DEBUG - m_packet_timeout(1000), + m_packet_timeout(1000000000), #else - m_packet_timeout(1), + m_packet_timeout(1000000), #endif m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512), m_send_acks(true), m_compression_type(CompressionType::None), @@ -263,11 +263,7 @@ GDBRemoteCommunication::PacketResult GDBRemoteCommunication::GetAck() { StringExtractorGDBRemote packet; - PacketResult result = ReadPacket( - packet, - std::chrono::duration_cast(GetPacketTimeout()) - .count(), - false); + PacketResult result = ReadPacket(packet, GetPacketTimeout(), false); if (result == PacketResult::Success) { if (packet.GetResponseType() == StringExtractorGDBRemote::ResponseType::eAck) @@ -278,67 +274,49 @@ return result; } -GDBRemoteCommunication::PacketResult -GDBRemoteCommunication::ReadPacket(StringExtractorGDBRemote &response, - uint32_t timeout_usec, - bool sync_on_timeout) { +GDBRemoteCommunication::PacketResult GDBRemoteCommunication::ReadPacket( + StringExtractorGDBRemote &response, + llvm::Optional timeout, bool sync_on_timeout) { if (m_read_thread_enabled) - return PopPacketFromQueue(response, timeout_usec); + return PopPacketFromQueue(response, timeout); else - return WaitForPacketWithTimeoutMicroSecondsNoLock(response, timeout_usec, - sync_on_timeout); + return WaitForPacketNoLock(response, timeout, sync_on_timeout); } // This function is called when a packet is requested. // A whole packet is popped from the packet queue and returned to the caller. // Packets are placed into this queue from the communication read thread. // See GDBRemoteCommunication::AppendBytesToCache. -GDBRemoteCommunication::PacketResult -GDBRemoteCommunication::PopPacketFromQueue(StringExtractorGDBRemote &response, - uint32_t timeout_usec) { - auto until = std::chrono::system_clock::now() + - std::chrono::microseconds(timeout_usec); - - while (true) { - // scope for the mutex - { - // lock down the packet queue - std::unique_lock lock(m_packet_queue_mutex); - - // Wait on condition variable. - if (m_packet_queue.size() == 0) { - std::cv_status result = - m_condition_queue_not_empty.wait_until(lock, until); - if (result == std::cv_status::timeout) - break; - } - - if (m_packet_queue.size() > 0) { - // get the front element of the queue - response = m_packet_queue.front(); - - // remove the front element - m_packet_queue.pop(); - - // we got a packet - return PacketResult::Success; - } - } - - // Disconnected +GDBRemoteCommunication::PacketResult GDBRemoteCommunication::PopPacketFromQueue( + StringExtractorGDBRemote &response, + llvm::Optional timeout) { + auto pred = [&] { return !m_packet_queue.empty() && IsConnected(); }; + // lock down the packet queue + std::unique_lock lock(m_packet_queue_mutex); + + if (!timeout) + m_condition_queue_not_empty.wait(lock, pred); + else { + if (!m_condition_queue_not_empty.wait_for(lock, *timeout, pred)) + return PacketResult::ErrorReplyTimeout; if (!IsConnected()) return PacketResult::ErrorDisconnected; - - // Loop while not timed out } - return PacketResult::ErrorReplyTimeout; + // get the front element of the queue + response = m_packet_queue.front(); + + // remove the front element + m_packet_queue.pop(); + + // we got a packet + return PacketResult::Success; } GDBRemoteCommunication::PacketResult -GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock( - StringExtractorGDBRemote &packet, uint32_t timeout_usec, - bool sync_on_timeout) { +GDBRemoteCommunication::WaitForPacketNoLock( + StringExtractorGDBRemote &packet, + llvm::Optional timeout, bool sync_on_timeout) { uint8_t buffer[8192]; Error error; @@ -354,12 +332,13 @@ while (IsConnected() && !timed_out) { lldb::ConnectionStatus status = eConnectionStatusNoConnection; size_t bytes_read = - Read(buffer, sizeof(buffer), timeout_usec, status, &error); + Read(buffer, sizeof(buffer), timeout ? timeout->count() : UINT32_MAX, + status, &error); if (log) - log->Printf("%s: Read (buffer, (sizeof(buffer), timeout_usec = 0x%x, " + log->Printf("%s: Read (buffer, (sizeof(buffer), timeout = %ld us, " "status = %s, error = %s) => bytes_read = %" PRIu64, - LLVM_PRETTY_FUNCTION, timeout_usec, + LLVM_PRETTY_FUNCTION, long(timeout ? timeout->count() : -1), Communication::ConnectionStatusAsCString(status), error.AsCString(), (uint64_t)bytes_read); @@ -422,8 +401,8 @@ uint32_t successful_responses = 0; for (uint32_t i = 0; i < max_retries; ++i) { StringExtractorGDBRemote echo_response; - echo_packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock( - echo_response, timeout_usec, false); + echo_packet_result = + WaitForPacketNoLock(echo_response, timeout, false); if (echo_packet_result == PacketResult::Success) { ++successful_responses; if (response_regex.Execute(echo_response.GetStringRef())) { @@ -1324,7 +1303,7 @@ void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); } GDBRemoteCommunication::ScopedTimeout::ScopedTimeout( - GDBRemoteCommunication &gdb_comm, std::chrono::seconds timeout) + GDBRemoteCommunication &gdb_comm, std::chrono::microseconds timeout) : m_gdb_comm(gdb_comm) { m_saved_timeout = m_gdb_comm.SetPacketTimeout(timeout); } Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -122,9 +122,9 @@ // GDB server and flush them all StringExtractorGDBRemote response; PacketResult packet_result = PacketResult::Success; - const uint32_t timeout_usec = 10 * 1000; // Wait for 10 ms for a response + const std::chrono::microseconds timeout(10000); // 10 ms while (packet_result == PacketResult::Success) - packet_result = ReadPacket(response, timeout_usec, false); + packet_result = ReadPacket(response, timeout, false); // The return value from QueryNoAckModeSupported() is true if the packet // was sent and _any_ response (including UNIMPLEMENTED) was received), @@ -202,8 +202,8 @@ // longer than normal to receive a reply. Wait at least 6 seconds for a // reply to this packet. - ScopedTimeout timeout( - *this, std::max(GetPacketTimeout(), std::chrono::seconds(6))); + ScopedTimeout timeout(*this, std::max(GetPacketTimeout(), + std::chrono::microseconds(6000000))); StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse("QStartNoAckMode", response, false) == Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h @@ -43,8 +43,9 @@ RegisterPacketHandler(StringExtractorGDBRemote::ServerPacketType packet_type, PacketHandler handler); - PacketResult GetPacketAndSendResponse(uint32_t timeout_usec, Error &error, - bool &interrupt, bool &quit); + PacketResult + GetPacketAndSendResponse(llvm::Optional timeout, + Error &error, bool &interrupt, bool &quit); // After connecting, do a little handshake with the client to make sure // we are at least communicating Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp @@ -38,14 +38,12 @@ } GDBRemoteCommunication::PacketResult -GDBRemoteCommunicationServer::GetPacketAndSendResponse(uint32_t timeout_usec, - Error &error, - bool &interrupt, - bool &quit) { +GDBRemoteCommunicationServer::GetPacketAndSendResponse( + llvm::Optional timeout, Error &error, + bool &interrupt, bool &quit) { StringExtractorGDBRemote packet; - PacketResult packet_result = - WaitForPacketWithTimeoutMicroSecondsNoLock(packet, timeout_usec, false); + PacketResult packet_result = WaitForPacketNoLock(packet, timeout, false); if (packet_result == PacketResult::Success) { const StringExtractorGDBRemote::ServerPacketType packet_type = packet.GetServerPacketType(); Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -926,8 +926,8 @@ bool done = false; Error error; while (true) { - const PacketResult result = - GetPacketAndSendResponse(0, error, interrupt, done); + const PacketResult result = GetPacketAndSendResponse( + std::chrono::microseconds(0), error, interrupt, done); if (result == PacketResult::ErrorReplyTimeout) break; // No more packets in the queue Index: tools/lldb-server/lldb-platform.cpp =================================================================== --- tools/lldb-server/lldb-platform.cpp +++ tools/lldb-server/lldb-platform.cpp @@ -364,7 +364,7 @@ bool interrupt = false; bool done = false; while (!interrupt && !done) { - if (platform.GetPacketAndSendResponse(UINT32_MAX, error, interrupt, + if (platform.GetPacketAndSendResponse(llvm::None, error, interrupt, done) != GDBRemoteCommunication::PacketResult::Success) break;