[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

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 @@ public:
   // 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/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 @@ public:
 
   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 @@ protected:
 
   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

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);
 }
 

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;
 }

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_);

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 @@ private:
   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;

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);

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();