Author: Michał Górny Date: 2021-10-26T13:53:08+02:00 New Revision: 4373f3595f8e37f6183d9880ee5b4eb59cba3852
URL: https://github.com/llvm/llvm-project/commit/4373f3595f8e37f6183d9880ee5b4eb59cba3852 DIFF: https://github.com/llvm/llvm-project/commit/4373f3595f8e37f6183d9880ee5b4eb59cba3852.diff LOG: [lldb] [Host] Move port predicate-related logic to gdb-remote Remove the port predicate from Socket and ConnectionFileDescriptor, and move it to gdb-remote. It is specifically relevant to the threading used inside gdb-remote and with the new port callback API, we can reliably move it there. While at it, switch from the custom Predicate to std::promise/std::future. Differential Revision: https://reviews.llvm.org/D112357 Added: Modified: lldb/include/lldb/Host/Socket.h lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h lldb/source/Host/common/Socket.cpp lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/unittests/Host/SocketTest.cpp lldb/unittests/Host/SocketTestUtilities.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h index 12ddc11be4f8..780c6e6adddd 100644 --- a/lldb/include/lldb/Host/Socket.h +++ b/lldb/include/lldb/Host/Socket.h @@ -16,7 +16,6 @@ #include "lldb/Host/SocketAddress.h" #include "lldb/Utility/IOObject.h" -#include "lldb/Utility/Predicate.h" #include "lldb/Utility/Status.h" #ifdef _WIN32 @@ -68,7 +67,7 @@ class Socket : public IOObject { // the socket after it is initialized, but before entering a blocking accept. static llvm::Expected<std::unique_ptr<TCPSocket>> TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, - Predicate<uint16_t> *predicate, int backlog = 5); + int backlog = 5); static llvm::Expected<std::unique_ptr<Socket>> TcpConnect(llvm::StringRef host_and_port, bool child_processes_inherit); diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h index 6d89fd710ab4..12eab5fc8ae8 100644 --- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h +++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h @@ -18,7 +18,6 @@ #include "lldb/Host/Pipe.h" #include "lldb/Utility/Connection.h" #include "lldb/Utility/IOObject.h" -#include "lldb/Utility/Predicate.h" namespace lldb_private { @@ -65,8 +64,6 @@ class ConnectionFileDescriptor : public Connection { lldb::IOObjectSP GetReadObject() override { return m_io_sp; } - uint16_t GetListeningPort(const Timeout<std::micro> &timeout); - bool GetChildProcessesInherit() const; void SetChildProcessesInherit(bool child_processes_inherit); @@ -123,11 +120,6 @@ class ConnectionFileDescriptor : public Connection { lldb::IOObjectSP m_io_sp; - Predicate<uint16_t> - m_port_predicate; // Used when binding to port zero to wait for the thread - // that creates the socket, binds and listens to - // resolve the port number. - Pipe m_pipe; std::recursive_mutex m_mutex; std::atomic<bool> m_shutting_down; // This marks that we are shutting down so diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index 9ca26d911c25..1eaa69532592 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -163,7 +163,7 @@ Socket::TcpConnect(llvm::StringRef host_and_port, llvm::Expected<std::unique_ptr<TCPSocket>> Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, - Predicate<uint16_t> *predicate, int backlog) { + int backlog) { Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION)); LLDB_LOG(log, "host_and_port = {0}", host_and_port); @@ -187,13 +187,6 @@ Socket::TcpListen(llvm::StringRef host_and_port, bool child_processes_inherit, if (port == 0) port = listen_socket->GetLocalPortNumber(); - // Set the port predicate since when doing a listen://<host>:<port> it - // often needs to accept the incoming connection which is a blocking system - // call. Allowing access to the bound port using a predicate allows us to - // wait for the port predicate to be set to a non-zero value from another - // thread in an efficient manor. - if (predicate) - predicate->SetValue(port, eBroadcastAlways); return std::move(listen_socket); } diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index fdbf660a1527..1c3de966916a 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -622,10 +622,9 @@ ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept( Status *error_ptr) { if (error_ptr) *error_ptr = Status(); - m_port_predicate.SetValue(0, eBroadcastNever); llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket = - Socket::TcpListen(s, m_child_processes_inherit, &m_port_predicate); + Socket::TcpListen(s, m_child_processes_inherit); if (!listening_socket) { if (error_ptr) *error_ptr = listening_socket.takeError(); @@ -827,12 +826,6 @@ ConnectionStatus ConnectionFileDescriptor::ConnectSerialPort( llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX"); } -uint16_t -ConnectionFileDescriptor::GetListeningPort(const Timeout<std::micro> &timeout) { - auto Result = m_port_predicate.WaitForValueNotEqualTo(0, timeout); - return Result ? *Result : 0; -} - bool ConnectionFileDescriptor::GetChildProcessesInherit() const { return m_child_processes_inherit; } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index ae2141d30220..4ce79da48f07 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -893,8 +893,13 @@ GDBRemoteCommunication::ListenThread(lldb::thread_arg_t arg) { if (connection) { // Do the listen on another thread so we can continue on... - if (connection->Connect(comm->m_listen_url.c_str(), &error) != - eConnectionStatusSuccess) + if (connection->Connect( + comm->m_listen_url.c_str(), [comm](llvm::StringRef port_str) { + uint16_t port = 0; + llvm::to_integer(port_str, port, 10); + comm->m_port_promise.set_value(port); + }, + &error) != eConnectionStatusSuccess) comm->SetConnection(nullptr); } return {}; @@ -1056,10 +1061,12 @@ Status GDBRemoteCommunication::StartDebugserverProcess( return error; } - ConnectionFileDescriptor *connection = - (ConnectionFileDescriptor *)GetConnection(); // Wait for 10 seconds to resolve the bound port - uint16_t port_ = connection->GetListeningPort(std::chrono::seconds(10)); + std::future<uint16_t> port_future = m_port_promise.get_future(); + uint16_t port_ = port_future.wait_for(std::chrono::seconds(10)) == + std::future_status::ready + ? port_future.get() + : 0; if (port_ > 0) { char port_cstr[32]; snprintf(port_cstr, sizeof(port_cstr), "127.0.0.1:%i", port_); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index b9a9fc1d5894..5da568e9b4d4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -12,6 +12,7 @@ #include "GDBRemoteCommunicationHistory.h" #include <condition_variable> +#include <future> #include <mutex> #include <queue> #include <string> @@ -243,6 +244,8 @@ class GDBRemoteCommunication : public Communication { std::mutex m_packet_queue_mutex; // Mutex for accessing queue std::condition_variable m_condition_queue_not_empty; // Condition variable to wait for packets + // Promise used to grab the port number from listening thread + std::promise<uint16_t> m_port_promise; HostThread m_listen_thread; std::string m_listen_url; diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp index 157fe410822e..15ccf647a2e0 100644 --- a/lldb/unittests/Host/SocketTest.cpp +++ b/lldb/unittests/Host/SocketTest.cpp @@ -154,10 +154,8 @@ TEST_P(SocketTest, UDPConnect) { TEST_P(SocketTest, TCPListen0GetPort) { if (!HostSupportsIPv4()) return; - Predicate<uint16_t> port_predicate; - port_predicate.SetValue(0, eBroadcastNever); llvm::Expected<std::unique_ptr<TCPSocket>> sock = - Socket::TcpListen("10.10.12.3:0", false, &port_predicate); + Socket::TcpListen("10.10.12.3:0", false); ASSERT_THAT_EXPECTED(sock, llvm::Succeeded()); ASSERT_TRUE(sock.get()->IsValid()); EXPECT_NE(sock.get()->GetLocalPortNumber(), 0); diff --git a/lldb/unittests/Host/SocketTestUtilities.cpp b/lldb/unittests/Host/SocketTestUtilities.cpp index 3b52a66a09eb..a37a2cd4ddf0 100644 --- a/lldb/unittests/Host/SocketTestUtilities.cpp +++ b/lldb/unittests/Host/SocketTestUtilities.cpp @@ -93,7 +93,7 @@ void lldb_private::CreateDomainConnectedSockets( static bool CheckIPSupport(llvm::StringRef Proto, llvm::StringRef Addr) { llvm::Expected<std::unique_ptr<TCPSocket>> Sock = Socket::TcpListen( - Addr, /*child_processes_inherit=*/false, /*predicate=*/nullptr); + Addr, /*child_processes_inherit=*/false); if (Sock) return true; llvm::Error Err = Sock.takeError(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
