From c4063eee0d9ad122bb346a6c9a8e9546e24c1291 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 25 Nov 2016 11:58:44 +0000 Subject: [PATCH] Introduce chrono to the Communication class This replaces the raw integer timeout parameters in the class with their chrono-based equivalents. To achieve this, I have moved the Timeout class to a more generic place and added a quick unit test for it. llvm-svn: 287920 --- lldb/include/lldb/Core/Communication.h | 11 +++-- lldb/include/lldb/Utility/Timeout.h | 56 ++++++++++++++++++++++ lldb/source/API/SBCommunication.cpp | 5 +- lldb/source/Core/Communication.cpp | 44 ++++++++--------- .../Process/gdb-remote/GDBRemoteCommunication.cpp | 4 +- .../Process/gdb-remote/GDBRemoteCommunication.h | 26 ---------- .../GDBRemoteCommunicationServerLLGS.cpp | 4 +- lldb/unittests/Utility/CMakeLists.txt | 1 + lldb/unittests/Utility/TimeoutTest.cpp | 22 +++++++++ 9 files changed, 113 insertions(+), 60 deletions(-) create mode 100644 lldb/include/lldb/Utility/Timeout.h create mode 100644 lldb/unittests/Utility/TimeoutTest.cpp diff --git a/lldb/include/lldb/Core/Communication.h b/lldb/include/lldb/Core/Communication.h index 4441b6f..8a875dd 100644 --- a/lldb/include/lldb/Core/Communication.h +++ b/lldb/include/lldb/Core/Communication.h @@ -21,7 +21,7 @@ #include "lldb/Core/Broadcaster.h" #include "lldb/Core/Error.h" #include "lldb/Host/HostThread.h" -#include "lldb/lldb-private.h" +#include "lldb/Utility/Timeout.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -196,15 +196,15 @@ public: /// The number of bytes to attempt to read, and also the max /// number of bytes that can be placed into \a dst. /// - /// @param[in] timeout_usec - /// A timeout value in micro-seconds. + /// @param[in] timeout + /// A timeout value or llvm::None for no timeout. /// /// @return /// The number of bytes actually read. /// /// @see size_t Connection::Read (void *, size_t); //------------------------------------------------------------------ - size_t Read(void *dst, size_t dst_len, uint32_t timeout_usec, + size_t Read(void *dst, size_t dst_len, const Timeout &timeout, lldb::ConnectionStatus &status, Error *error_ptr); //------------------------------------------------------------------ @@ -347,7 +347,8 @@ protected: void *m_callback_baton; bool m_close_on_eof; - size_t ReadFromConnection(void *dst, size_t dst_len, uint32_t timeout_usec, + size_t ReadFromConnection(void *dst, size_t dst_len, + const Timeout &timeout, lldb::ConnectionStatus &status, Error *error_ptr); //------------------------------------------------------------------ diff --git a/lldb/include/lldb/Utility/Timeout.h b/lldb/include/lldb/Utility/Timeout.h new file mode 100644 index 0000000..e5e975e --- /dev/null +++ b/lldb/include/lldb/Utility/Timeout.h @@ -0,0 +1,56 @@ +//===-- Timeout.h -----------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef liblldb_Timeout_h_ +#define liblldb_Timeout_h_ + +#include + +#include + +namespace lldb_private { + +// A general purpose class for representing timeouts for various APIs. It's +// basically an llvm::Optional>, but we +// customize it a bit to enable the standard chrono implicit conversions (e.g. +// from Timeout to Timeout. +// +// The intended meaning of the values is: +// - llvm::None - no timeout, the call should wait forever +// - 0 - poll, only complete the call if it will not block +// - >0 - wait for a given number of units for the result +template +class Timeout : 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: + Timeout(llvm::NoneType none) : Base(none) {} + Timeout(const Timeout &other) = default; + + template ::type> + Timeout(const Timeout &other) + : Base(other ? Base(Dur(*other)) : llvm::None) {} + + template ::type> + Timeout(const std::chrono::duration &other) + : Base(Dur(other)) {} +}; + +} // namespace lldb_private + +#endif // liblldb_Timeout_h_ diff --git a/lldb/source/API/SBCommunication.cpp b/lldb/source/API/SBCommunication.cpp index f80b114..9519c02 100644 --- a/lldb/source/API/SBCommunication.cpp +++ b/lldb/source/API/SBCommunication.cpp @@ -119,8 +119,11 @@ size_t SBCommunication::Read(void *dst, size_t dst_len, uint32_t timeout_usec, static_cast(m_opaque), static_cast(dst), static_cast(dst_len), timeout_usec); size_t bytes_read = 0; + Timeout timeout = timeout_usec == UINT32_MAX + ? Timeout(llvm::None) + : std::chrono::microseconds(timeout_usec); if (m_opaque) - bytes_read = m_opaque->Read(dst, dst_len, timeout_usec, status, NULL); + bytes_read = m_opaque->Read(dst, dst_len, timeout, status, NULL); else status = eConnectionStatusNoConnection; diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp index 78c2deb..47616cc 100644 --- a/lldb/source/Core/Communication.cpp +++ b/lldb/source/Core/Communication.cpp @@ -112,18 +112,20 @@ bool Communication::HasConnection() const { return m_connection_sp.get() != nullptr; } -size_t Communication::Read(void *dst, size_t dst_len, uint32_t timeout_usec, +size_t Communication::Read(void *dst, size_t dst_len, + const Timeout &timeout, ConnectionStatus &status, Error *error_ptr) { lldb_private::LogIfAnyCategoriesSet( LIBLLDB_LOG_COMMUNICATION, "%p Communication::Read (dst = %p, dst_len = %" PRIu64 ", timeout = %u usec) connection = %p", - this, dst, (uint64_t)dst_len, timeout_usec, m_connection_sp.get()); + this, dst, (uint64_t)dst_len, timeout ? timeout->count() : -1, + m_connection_sp.get()); if (m_read_thread_enabled) { // We have a dedicated read thread that is getting data for us size_t cached_bytes = GetCachedBytes(dst, dst_len); - if (cached_bytes > 0 || timeout_usec == 0) { + if (cached_bytes > 0 || (timeout && timeout->count() == 0)) { status = eConnectionStatusSuccess; return cached_bytes; } @@ -139,10 +141,9 @@ size_t Communication::Read(void *dst, size_t dst_len, uint32_t timeout_usec, listener_sp->StartListeningForEvents( this, eBroadcastBitReadThreadGotBytes | eBroadcastBitReadThreadDidExit); EventSP event_sp; - std::chrono::microseconds timeout = std::chrono::microseconds(0); - if (timeout_usec != UINT32_MAX) - timeout = std::chrono::microseconds(timeout_usec); - while (listener_sp->WaitForEvent(timeout, event_sp)) { + std::chrono::microseconds listener_timeout = + timeout ? *timeout : std::chrono::microseconds(0); + while (listener_sp->WaitForEvent(listener_timeout, event_sp)) { const uint32_t event_type = event_sp->GetType(); if (event_type & eBroadcastBitReadThreadGotBytes) { return GetCachedBytes(dst, dst_len); @@ -159,15 +160,7 @@ size_t Communication::Read(void *dst, size_t dst_len, uint32_t timeout_usec, // We aren't using a read thread, just read the data synchronously in this // thread. - lldb::ConnectionSP connection_sp(m_connection_sp); - if (connection_sp) { - return connection_sp->Read(dst, dst_len, timeout_usec, status, error_ptr); - } - - if (error_ptr) - error_ptr->SetErrorString("Invalid connection."); - status = eConnectionStatusNoConnection; - return 0; + return ReadFromConnection(dst, dst_len, timeout, status, error_ptr); } size_t Communication::Write(const void *src, size_t src_len, @@ -279,14 +272,20 @@ void Communication::AppendBytesToCache(const uint8_t *bytes, size_t len, } size_t Communication::ReadFromConnection(void *dst, size_t dst_len, - uint32_t timeout_usec, + const Timeout &timeout, ConnectionStatus &status, Error *error_ptr) { lldb::ConnectionSP connection_sp(m_connection_sp); - return ( - connection_sp - ? connection_sp->Read(dst, dst_len, timeout_usec, status, error_ptr) - : 0); + if (connection_sp) { + return connection_sp->Read(dst, dst_len, + timeout ? timeout->count() : UINT32_MAX, status, + error_ptr); + } + + if (error_ptr) + error_ptr->SetErrorString("Invalid connection."); + status = eConnectionStatusNoConnection; + return 0; } bool Communication::ReadThreadIsRunning() { return m_read_thread_enabled; } @@ -305,9 +304,8 @@ lldb::thread_result_t Communication::ReadThread(lldb::thread_arg_t p) { ConnectionStatus status = eConnectionStatusSuccess; bool done = false; while (!done && comm->m_read_thread_enabled) { - const int timeout_us = 5000000; size_t bytes_read = comm->ReadFromConnection( - buf, sizeof(buf), timeout_us, status, &error); + buf, sizeof(buf), std::chrono::seconds(5), status, &error); if (bytes_read > 0) comm->AppendBytesToCache(buf, bytes_read, true, status); else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index c263454..bd87521 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -332,9 +332,7 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet, bool disconnected = false; while (IsConnected() && !timed_out) { lldb::ConnectionStatus status = eConnectionStatusNoConnection; - size_t bytes_read = - Read(buffer, sizeof(buffer), timeout ? timeout->count() : UINT32_MAX, - status, &error); + size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error); if (log) log->Printf("%s: Read (buffer, (sizeof(buffer), timeout = %ld us, " diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index a73ba4e..1f3fa17 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -53,32 +53,6 @@ enum class CompressionType { class ProcessGDBRemote; -template -class Timeout : 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: - Timeout(llvm::NoneType none) : Base(none) {} - Timeout(const Timeout &other) = default; - - template ::type> - Timeout(const Timeout &other) - : Base(other ? Base(Dur(*other)) : llvm::None) {} - - template ::type> - Timeout(const std::chrono::duration &other) - : Base(Dur(other)) {} -}; - class GDBRemoteCommunication : public Communication { public: enum { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index fe439cd..bf72673 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -1021,8 +1021,8 @@ void GDBRemoteCommunicationServerLLGS::SendProcessOutput() { ConnectionStatus status; Error error; while (true) { - size_t bytes_read = - m_stdio_communication.Read(buffer, sizeof buffer, 0, status, &error); + size_t bytes_read = m_stdio_communication.Read( + buffer, sizeof buffer, std::chrono::microseconds(0), status, &error); switch (status) { case eConnectionStatusSuccess: SendONotification(buffer, bytes_read); diff --git a/lldb/unittests/Utility/CMakeLists.txt b/lldb/unittests/Utility/CMakeLists.txt index 99677a4..15a2982 100644 --- a/lldb/unittests/Utility/CMakeLists.txt +++ b/lldb/unittests/Utility/CMakeLists.txt @@ -2,6 +2,7 @@ add_lldb_unittest(UtilityTests ModuleCacheTest.cpp StringExtractorTest.cpp TaskPoolTest.cpp + TimeoutTest.cpp UriParserTest.cpp ) diff --git a/lldb/unittests/Utility/TimeoutTest.cpp b/lldb/unittests/Utility/TimeoutTest.cpp new file mode 100644 index 0000000..a30c616 --- /dev/null +++ b/lldb/unittests/Utility/TimeoutTest.cpp @@ -0,0 +1,22 @@ +//===-- TimeoutTest.cpp -----------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/Timeout.h" +#include "gtest/gtest.h" + +using namespace lldb_private; +using namespace std::chrono; + +TEST(TimeoutTest, Construction) { + ASSERT_FALSE(Timeout(llvm::None)); + ASSERT_TRUE(bool(Timeout(seconds(0)))); + ASSERT_EQ(seconds(0), *Timeout(seconds(0))); + ASSERT_EQ(seconds(3), *Timeout(seconds(3))); + ASSERT_TRUE(bool(Timeout(Timeout(seconds(0))))); +} -- 2.7.4