[Lldb-commits] [lldb] 4373f35 - [lldb] [Host] Move port predicate-related logic to gdb-remote
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 26 04:53:21 PDT 2021
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();
More information about the lldb-commits
mailing list