Refactoring the socket management for lldb-dap server mode to use lldb/Host/Socket.h'.

Author: ashgti

lldb/include/lldb/Host/Socket.h

@@ -152,6 +152,9 @@ class Socket : public IOObject {
   // If this Socket is connected then return the URI used to connect.
   virtual std::string GetRemoteConnectionURI() const { return ""; };
 
+  // If the Socket is listening then return the URI for clients to connect.
+  virtual std::string GetListeningConnectionURI() const { return ""; }
+
 protected:
   Socket(SocketProtocol protocol, bool should_close,
          bool m_child_process_inherit);

lldb/include/lldb/Host/common/TCPSocket.h

@@ -53,6 +53,8 @@ class TCPSocket : public Socket {
 
   std::string GetRemoteConnectionURI() const override;
 
+  std::string GetListeningConnectionURI() const override;
+
 private:
   TCPSocket(NativeSocket socket, const TCPSocket &listen_socket);
 

lldb/include/lldb/Host/posix/DomainSocket.h

@@ -28,6 +28,8 @@ class DomainSocket : public Socket {
 
   std::string GetRemoteConnectionURI() const override;
 
+  std::string GetListeningConnectionURI() const override;
+
 protected:
   DomainSocket(SocketProtocol protocol, bool child_processes_inherit);
 

lldb/packages/Python/lldbsuite/test/lldbtest.py

@@ -251,13 +251,6 @@ def which(program):
     return None
 
 
-def pickrandomport():
-    """Returns a random open port."""
-    with socket.socket() as sock:
-        sock.bind(("", 0))
-        return sock.getsockname()[1]
-
-
 class ValueCheck:
     def __init__(
         self,

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -1172,14 +1172,13 @@ def __init__(
             )
 
         if connection:
-            print("attempting connection", connection)
-            if connection.startswith("unix://"):  # unix:///path
+            if connection.startswith("unix-connect://"):  # unix-connect:///path
                 s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-                s.connect(connection.removeprefix("unix://"))
-            elif connection.startswith("tcp://"):  # tcp://host:port
-                host, port = connection.removeprefix("tcp://").split(":", 1)
+                s.connect(connection.removeprefix("unix-connect://"))
+            elif connection.startswith("connection://"):  # connection://[host]:port
+                host, port = connection.removeprefix("connection://").rsplit(":", 1)
                 # create_connection with try both ipv4 and ipv6.
-                s = socket.create_connection((host, int(port)))
+                s = socket.create_connection((host.strip("[]"), int(port)))
             else:
                 raise ValueError("invalid connection: {}".format(connection))
             DebugCommunication.__init__(
@@ -1213,16 +1212,16 @@ def launch(
         if log_file:
             adaptor_env["LLDBDAP_LOG"] = log_file
 
-        if os.uname().sysname == "Darwin":
-            adaptor_env["NSUnbufferedIO"] = "YES"
-
         args = [executable]
+        bufsize = -1
         if connection:
+            bufsize = 0
             args.append("--connection")
             args.append(connection)
 
         proc = subprocess.Popen(
             args,
+            bufsize=bufsize,
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
             stderr=sys.stdout,
@@ -1233,29 +1232,20 @@ def launch(
             # If a conneciton is specified, lldb-dap will print the listening
             # address once the listener is made to stdout. The listener is
             # formatted like `tcp://host:port` or `unix:///path`.
-            with selectors.DefaultSelector() as sel:
-                print("Reading stdout for the listening connection")
-                os.set_blocking(proc.stdout.fileno(), False)
-                stdout_key = sel.register(proc.stdout, selectors.EVENT_READ)
-                rdy_fds = sel.select(timeout=10.0)
-                for key, _ in rdy_fds:
-                    if key != stdout_key:
-                        continue
-
-                    outs = proc.stdout.read(1024).decode()
-                    os.set_blocking(proc.stdout.fileno(), True)
-                    for line in outs.split("\n"):
-                        if not line.startswith("Listening for: "):
-                            continue
-                        # If the listener expanded into multiple addresses, use the first.
-                        connection = line.removeprefix("Listening for: ").split(",")[0]
-                        print("")
-                        return proc, connection
+            expected_prefix = "Listening for: "
+            out = proc.stdout.readline().decode()
+            if not out.startswith(expected_prefix):
                 proc.kill()
                 raise ValueError(
-                    "lldb-dap started with a connection but failed to write the listening address to stdout."
+                    "lldb-dap failed to print listening address, expected '{}', got '{}'".format(
+                        expected_prefix, out
+                    )
                 )
 
+            # If the listener expanded into multiple addresses, use the first.
+            connection = out.removeprefix(expected_prefix).rstrip("\r\n")
+            return proc, connection
+
         return proc, None
 
 

lldb/source/Host/common/TCPSocket.cpp

@@ -84,6 +84,12 @@ std::string TCPSocket::GetLocalIPAddress() const {
     socklen_t sock_addr_len = sock_addr.GetMaxLength();
     if (::getsockname(m_socket, sock_addr, &sock_addr_len) == 0)
       return sock_addr.GetIPAddress();
+  } else if (!m_listen_sockets.empty()) {
+    SocketAddress sock_addr;
+    socklen_t sock_addr_len = sock_addr.GetMaxLength();
+    if (::getsockname(m_listen_sockets.begin()->first, sock_addr,
+                      &sock_addr_len) == 0)
+      return sock_addr.GetIPAddress();
   }
   return "";
 }
@@ -118,6 +124,15 @@ std::string TCPSocket::GetRemoteConnectionURI() const {
   return "";
 }
 
+std::string TCPSocket::GetListeningConnectionURI() const {
+  if (!m_listen_sockets.empty()) {
+    return std::string(llvm::formatv(
+        "connection://[{0}]:{1}", GetLocalIPAddress(), GetLocalPortNumber()));
+  }
+
+  return "";
+}
+
 Status TCPSocket::CreateSocket(int domain) {
   Status error;
   if (IsValid())
@@ -180,8 +195,9 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
 
   if (host_port->hostname == "*")
     host_port->hostname = "0.0.0.0";
-  std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
-      host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  std::vector<SocketAddress> addresses =
+      SocketAddress::GetAddressInfo(host_port->hostname.c_str(), nullptr,
+                                    AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
     int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
                                   m_child_processes_inherit, error);
@@ -195,7 +211,7 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
     }
 
     SocketAddress listen_address = address;
-    if(!listen_address.IsLocalhost())
+    if (!listen_address.IsLocalhost())
       listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
     else
       listen_address.SetPort(host_port->port);

lldb/source/Host/posix/DomainSocket.cpp

@@ -90,7 +90,8 @@ Status DomainSocket::Connect(llvm::StringRef name) {
   if (error.Fail())
     return error;
   if (llvm::sys::RetryAfterSignal(-1, ::connect, GetNativeSocket(),
-        (struct sockaddr *)&saddr_un, saddr_un_len) < 0)
+                                  (struct sockaddr *)&saddr_un,
+                                  saddr_un_len) < 0)
     SetLastError(error);
 
   return error;
@@ -181,3 +182,17 @@ std::string DomainSocket::GetRemoteConnectionURI() const {
       "{0}://{1}",
       GetNameOffset() == 0 ? "unix-connect" : "unix-abstract-connect", name);
 }
+
+std::string DomainSocket::GetListeningConnectionURI() const {
+  if (m_socket == kInvalidSocketValue)
+    return "";
+
+  struct sockaddr_un addr;
+  bzero(&addr, sizeof(struct sockaddr_un));
+  addr.sun_family = AF_UNIX;
+  socklen_t addr_len = sizeof(struct sockaddr_un);
+  if (::getsockname(m_socket, (struct sockaddr *)&addr, &addr_len) != 0)
+    return "";
+
+  return llvm::formatv("unix-connect://{0}", addr.sun_path);
+}

lldb/tools/lldb-dap/CMakeLists.txt

@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD" )
-  list(APPEND extra_libs lldbHost)
-endif ()
-
 if (HAVE_LIBPTHREAD)
   list(APPEND extra_libs pthread)
 endif ()
@@ -37,12 +33,12 @@ add_lldb_tool(lldb-dap
   OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
-  Socket.cpp
   SourceBreakpoint.cpp
   Watchpoint.cpp
 
   LINK_LIBS
     liblldb
+    lldbHost
     ${extra_libs}
 
   LINK_COMPONENTS

lldb/tools/lldb-dap/DAP.cpp

@@ -8,6 +8,7 @@
 
 #include <chrono>
 #include <cstdarg>
+#include <fstream>
 #include <mutex>
 
 #include "DAP.h"
@@ -34,7 +35,7 @@ using namespace lldb_dap;
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode,
+DAP::DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
          std::vector<std::string> pre_init_commands)
     : debug_adaptor_path(path), broadcaster("lldb-dap"), log(log),
       exception_breakpoints(), pre_init_commands(pre_init_commands),
@@ -66,8 +67,8 @@ llvm::Error DAP::ConfigureIO(int out_fd, int err_fd) {
     return Err;
   }
 
-  out = lldb::SBFile(new_stdout_fd.get(), "w", false);
-  err = lldb::SBFile(new_stderr_fd.get(), "w", false);
+  out = lldb::SBFile(new_stdout_fd.get(), "w", true);
+  err = lldb::SBFile(new_stderr_fd.get(), "w", true);
 
   return llvm::Error::success();
 }
@@ -207,9 +208,9 @@ void DAP::SendJSON(const llvm::json::Value &json) {
   if (log) {
     auto now = std::chrono::duration<double>(
         std::chrono::system_clock::now().time_since_epoch());
-    *log << llvm::formatv("{0:f9} <-- ", now.count()).str() << "\n"
+    *log << llvm::formatv("{0:f9} <-- ", now.count()).str() << std::endl
          << "Content-Length: " << json_str.size() << "\r\n\r\n"
-         << llvm::formatv("{0:2}", json).str() << "\n";
+         << llvm::formatv("{0:2}", json).str() << std::endl;
   }
 }
 
@@ -237,8 +238,8 @@ std::string DAP::ReadJSON() {
   if (log) {
     auto now = std::chrono::duration<double>(
         std::chrono::system_clock::now().time_since_epoch());
-    *log << llvm::formatv("{0:f9} --> ", now.count()).str()
-         << "\nContent-Length: " << length << "\r\n\r\n";
+    *log << llvm::formatv("{0:f9} --> ", now.count()).str() << std::endl
+         << "Content-Length: " << length << "\r\n\r\n";
   }
   return json_str;
 }

lldb/tools/lldb-dap/DAP.h

@@ -145,7 +145,7 @@ struct DAP {
   lldb::SBBroadcaster broadcaster;
   std::thread event_thread;
   std::thread progress_event_thread;
-  llvm::raw_ostream *log;
+  std::ofstream *log;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   InstructionBreakpointMap instruction_breakpoints;
@@ -197,7 +197,7 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode,
+  DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
       std::vector<std::string> pre_init_commands);
   ~DAP();
 

lldb/tools/lldb-dap/IOStream.cpp

@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "IOStream.h"
-#include "llvm/Support/raw_ostream.h"
+#include <fstream>
 #include <string>
 
 #if defined(_WIN32)
@@ -86,7 +86,7 @@ bool OutputStream::write_full(llvm::StringRef str) {
   return true;
 }
 
-bool InputStream::read_full(llvm::raw_ostream *log, size_t length,
+bool InputStream::read_full(std::ofstream *log, size_t length,
                             std::string &text) {
   std::string data;
   data.resize(length);
@@ -130,7 +130,7 @@ bool InputStream::read_full(llvm::raw_ostream *log, size_t length,
   return true;
 }
 
-bool InputStream::read_line(llvm::raw_ostream *log, std::string &line) {
+bool InputStream::read_line(std::ofstream *log, std::string &line) {
   line.clear();
   while (true) {
     if (!read_full(log, 1, line))
@@ -143,8 +143,7 @@ bool InputStream::read_line(llvm::raw_ostream *log, std::string &line) {
   return true;
 }
 
-bool InputStream::read_expected(llvm::raw_ostream *log,
-                                llvm::StringRef expected) {
+bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
   std::string result;
   if (!read_full(log, expected.size(), result))
     return false;

lldb/tools/lldb-dap/IOStream.h

@@ -22,7 +22,7 @@ typedef int SOCKET;
 #endif
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/raw_ostream.h"
+#include <fstream>
 #include <string>
 
 // Windows requires different system calls for dealing with sockets and other
@@ -51,11 +51,11 @@ struct StreamDescriptor {
 struct InputStream {
   StreamDescriptor descriptor;
 
-  bool read_full(llvm::raw_ostream *log, size_t length, std::string &text);
+  bool read_full(std::ofstream *log, size_t length, std::string &text);
 
-  bool read_line(llvm::raw_ostream *log, std::string &line);
+  bool read_line(std::ofstream *log, std::string &line);
 
-  bool read_expected(llvm::raw_ostream *log, llvm::StringRef expected);
+  bool read_expected(std::ofstream *log, llvm::StringRef expected);
 };
 
 struct OutputStream {