mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor.
mgorny requested review of this revision.
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.
https://reviews.llvm.org/D112357
Files:
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
Index: lldb/unittests/Host/SocketTestUtilities.cpp
===================================================================
--- lldb/unittests/Host/SocketTestUtilities.cpp
+++ lldb/unittests/Host/SocketTestUtilities.cpp
@@ -93,7 +93,7 @@
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();
Index: lldb/unittests/Host/SocketTest.cpp
===================================================================
--- lldb/unittests/Host/SocketTest.cpp
+++ lldb/unittests/Host/SocketTest.cpp
@@ -154,10 +154,8 @@
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);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ 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 @@
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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -893,8 +893,13 @@
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 @@
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_);
Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===================================================================
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -631,10 +631,9 @@
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();
@@ -853,12 +852,6 @@
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;
}
Index: lldb/source/Host/common/Socket.cpp
===================================================================
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -163,7 +163,7 @@
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 @@
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);
}
Index: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
===================================================================
--- lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ 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 {
@@ -63,8 +62,6 @@
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);
@@ -125,11 +122,6 @@
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
Index: lldb/include/lldb/Host/Socket.h
===================================================================
--- lldb/include/lldb/Host/Socket.h
+++ 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 @@
// 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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
