Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -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::milliseconds timeout(100); + 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 @@ -53,6 +53,30 @@ class ProcessGDBRemote; +template +class OptionalDur : public llvm::Optional> { +private: + template + using Dur = std::chrono::duration; + template + using EnableIf = std::enable_if< + std::is_convertible, + std::chrono::duration>::value>; + + using Base = llvm::Optional>; + +public: + OptionalDur(llvm::NoneType none) : Base(none) {} + OptionalDur(const OptionalDur &other) = default; + + template ::type> + OptionalDur(const OptionalDur &other) + : Base(other ? Base(Dur(*other)) : llvm::None) {} + + template ::type> + OptionalDur(const Dur &other) : Base(Dur(other)) {} +}; + class GDBRemoteCommunication : public Communication { public: enum { @@ -115,13 +139,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::seconds + SetPacketTimeout(std::chrono::seconds 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::seconds GetPacketTimeout() const { + return m_packet_timeout; + } //------------------------------------------------------------------ // Start a debugserver instance on the current host using the @@ -226,16 +253,18 @@ PacketResult SendPacketNoLock(llvm::StringRef payload); PacketResult ReadPacket(StringExtractorGDBRemote &response, - uint32_t timeout_usec, bool sync_on_timeout); + OptionalDur 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, + OptionalDur timeout); PacketResult - WaitForPacketWithTimeoutMicroSecondsNoLock(StringExtractorGDBRemote &response, - uint32_t timeout_usec, - bool sync_on_timeout); + WaitForPacketNoLock(StringExtractorGDBRemote &response, + OptionalDur 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 @@ -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,15 +274,13 @@ return result; } -GDBRemoteCommunication::PacketResult -GDBRemoteCommunication::ReadPacket(StringExtractorGDBRemote &response, - uint32_t timeout_usec, - bool sync_on_timeout) { +GDBRemoteCommunication::PacketResult GDBRemoteCommunication::ReadPacket( + StringExtractorGDBRemote &response, + OptionalDur 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. @@ -295,50 +289,34 @@ // 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 + OptionalDur 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, + OptionalDur 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())) { 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::milliseconds timeout(10); 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), 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(OptionalDur 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( + OptionalDur 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;