[lldb-dap] Use existing lldb::IOObjectSP for IO (NFC).

This simplifies the IOStream.cpp implementation by building on top of the existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` specifier properly detect shutdown requests when the Socket is closed. Previously, the StreamDescriptor was just accessing the underyling native handle and was not aware of the `Close()` call to the Socket itself.

Author: ashgti

lldb/tools/lldb-dap/DAP.cpp

@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null";
 namespace lldb_dap {
 
 DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-         StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+         lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
          std::vector<std::string> pre_init_commands)
     : name(std::move(name)), debug_adaptor_path(path), log(log),
       input(std::move(input)), output(std::move(output)),

lldb/tools/lldb-dap/DAP.h

@@ -29,6 +29,7 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -125,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
                  lldb::SBCommandReturnObject &result) override;
 };
@@ -211,7 +212,7 @@ struct DAP {
   std::string last_nonempty_var_expression;
 
   DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-      StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+      lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
       std::vector<std::string> pre_init_commands);
   ~DAP();
   DAP(const DAP &rhs) = delete;

lldb/tools/lldb-dap/IOStream.cpp

@@ -7,125 +7,34 @@
 //===----------------------------------------------------------------------===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
+#include "lldb/Utility/Status.h"
 #include <fstream>
 #include <string>
 
-#if defined(_WIN32)
-#include <io.h>
-#else
-#include <netinet/in.h>
-#include <sys/socket.h>
-#include <unistd.h>
-#endif
-
 using namespace lldb_dap;
 
-StreamDescriptor::StreamDescriptor() = default;
-
-StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
-  *this = std::move(other);
-}
-
-StreamDescriptor::~StreamDescriptor() {
-  if (!m_close)
-    return;
-
-  if (m_is_socket)
-#if defined(_WIN32)
-    ::closesocket(m_socket);
-#else
-    ::close(m_socket);
-#endif
-  else
-    ::close(m_fd);
-}
-
-StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
-  m_close = other.m_close;
-  other.m_close = false;
-  m_is_socket = other.m_is_socket;
-  if (m_is_socket)
-    m_socket = other.m_socket;
-  else
-    m_fd = other.m_fd;
-  return *this;
-}
-
-StreamDescriptor StreamDescriptor::from_socket(SOCKET s, bool close) {
-  StreamDescriptor sd;
-  sd.m_is_socket = true;
-  sd.m_socket = s;
-  sd.m_close = close;
-  return sd;
-}
-
-StreamDescriptor StreamDescriptor::from_file(int fd, bool close) {
-  StreamDescriptor sd;
-  sd.m_is_socket = false;
-  sd.m_fd = fd;
-  sd.m_close = close;
-  return sd;
-}
-
 bool OutputStream::write_full(llvm::StringRef str) {
-  while (!str.empty()) {
-    int bytes_written = 0;
-    if (descriptor.m_is_socket)
-      bytes_written = ::send(descriptor.m_socket, str.data(), str.size(), 0);
-    else
-      bytes_written = ::write(descriptor.m_fd, str.data(), str.size());
-
-    if (bytes_written < 0) {
-      if (errno == EINTR || errno == EAGAIN)
-        continue;
-      return false;
-    }
-    str = str.drop_front(bytes_written);
-  }
+  if (!descriptor)
+    return false;
 
-  return true;
+  size_t num_bytes = str.size();
+  auto status = descriptor->Write(str.data(), num_bytes);
+  return status.Success();
 }
 
 bool InputStream::read_full(std::ofstream *log, size_t length,
                             std::string &text) {
+  if (!descriptor)
+    return false;
+
   std::string data;
   data.resize(length);
 
-  char *ptr = &data[0];
-  while (length != 0) {
-    int bytes_read = 0;
-    if (descriptor.m_is_socket)
-      bytes_read = ::recv(descriptor.m_socket, ptr, length, 0);
-    else
-      bytes_read = ::read(descriptor.m_fd, ptr, length);
-
-    if (bytes_read == 0) {
-      if (log)
-        *log << "End of file (EOF) reading from input file.\n";
-      return false;
-    }
-    if (bytes_read < 0) {
-      int reason = 0;
-#if defined(_WIN32)
-      if (descriptor.m_is_socket)
-        reason = WSAGetLastError();
-      else
-        reason = errno;
-#else
-      reason = errno;
-      if (reason == EINTR || reason == EAGAIN)
-        continue;
-#endif
-
-      if (log)
-        *log << "Error " << reason << " reading from input file.\n";
-      return false;
-    }
+  auto status = descriptor->Read(data.data(), length);
+  if (status.Fail())
+    return false;
 
-    assert(bytes_read >= 0 && (size_t)bytes_read <= length);
-    ptr += bytes_read;
-    length -= bytes_read;
-  }
   text += data;
   return true;
 }

lldb/tools/lldb-dap/IOStream.h

@@ -9,45 +9,17 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
 #define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
 
-#if defined(_WIN32)
-#include "lldb/Host/windows/windows.h"
-#include <winsock2.h>
-#else
-typedef int SOCKET;
-#endif
-
+#include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
-
 #include <fstream>
 #include <string>
 
-// Windows requires different system calls for dealing with sockets and other
-// types of files, so we can't simply have one code path that just uses read
-// and write everywhere.  So we need an abstraction in order to allow us to
-// treat them identically.
 namespace lldb_dap {
-struct StreamDescriptor {
-  StreamDescriptor();
-  ~StreamDescriptor();
-  StreamDescriptor(StreamDescriptor &&other);
-
-  StreamDescriptor &operator=(StreamDescriptor &&other);
-
-  static StreamDescriptor from_socket(SOCKET s, bool close);
-  static StreamDescriptor from_file(int fd, bool close);
-
-  bool m_is_socket = false;
-  bool m_close = false;
-  union {
-    int m_fd;
-    SOCKET m_socket;
-  };
-};
 
 struct InputStream {
-  StreamDescriptor descriptor;
+  lldb::IOObjectSP descriptor;
 
-  explicit InputStream(StreamDescriptor descriptor)
+  explicit InputStream(lldb::IOObjectSP descriptor)
       : descriptor(std::move(descriptor)) {}
 
   bool read_full(std::ofstream *log, size_t length, std::string &text);
@@ -58,9 +30,9 @@ struct InputStream {
 };
 
 struct OutputStream {
-  StreamDescriptor descriptor;
+  lldb::IOObjectSP descriptor;
 
-  explicit OutputStream(StreamDescriptor descriptor)
+  explicit OutputStream(lldb::IOObjectSP descriptor)
       : descriptor(std::move(descriptor)) {}
 
   bool write_full(llvm::StringRef str);

lldb/tools/lldb-dap/lldb-dap.cpp

@@ -15,6 +15,7 @@
 #include "RunInTerminal.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/File.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Host/Socket.h"
@@ -80,7 +81,11 @@ typedef int socklen_t;
 #endif
 
 using namespace lldb_dap;
-using lldb_private::NativeSocket;
+using lldb_private::File;
+using lldb_private::IOObject;
+using lldb_private::MainLoop;
+using lldb_private::MainLoopBase;
+using lldb_private::NativeFile;
 using lldb_private::Socket;
 using lldb_private::Status;
 
@@ -309,14 +314,14 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
   // address.
   llvm::outs().flush();
 
-  static lldb_private::MainLoop g_loop;
+  static MainLoop g_loop;
   llvm::sys::SetInterruptFunction([]() {
     g_loop.AddPendingCallback(
-        [](lldb_private::MainLoopBase &loop) { loop.RequestTermination(); });
+        [](MainLoopBase &loop) { loop.RequestTermination(); });
   });
   std::condition_variable dap_sessions_condition;
   std::mutex dap_sessions_mutex;
-  std::map<Socket *, DAP *> dap_sessions;
+  std::map<IOObject *, DAP *> dap_sessions;
   unsigned int clientCount = 0;
   auto handle = listener->Accept(g_loop, [=, &dap_sessions_condition,
                                           &dap_sessions_mutex, &dap_sessions,
@@ -330,18 +335,15 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
            << " client connected: " << name << "\n";
     }
 
+    lldb::IOObjectSP io(std::move(sock));
+
     // Move the client into a background thread to unblock accepting the next
     // client.
     std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex,
-                        &dap_sessions, sock = std::move(sock)]() {
+                        &dap_sessions]() {
       llvm::set_thread_name(name + ".runloop");
-      StreamDescriptor input =
-          StreamDescriptor::from_socket(sock->GetNativeSocket(), false);
-      // Close the output last for the best chance at error reporting.
-      StreamDescriptor output =
-          StreamDescriptor::from_socket(sock->GetNativeSocket(), false);
-      DAP dap = DAP(name, program_path, log, std::move(input),
-                    std::move(output), default_repl_mode, pre_init_commands);
+      DAP dap = DAP(name, program_path, log, io, io, default_repl_mode,
+                    pre_init_commands);
 
       if (auto Err = dap.ConfigureIO()) {
         llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
@@ -353,7 +355,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
 
       {
         std::scoped_lock<std::mutex> lock(dap_sessions_mutex);
-        dap_sessions[sock.get()] = &dap;
+        dap_sessions[io.get()] = &dap;
       }
 
       if (auto Err = dap.Loop()) {
@@ -369,7 +371,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name,
       }
 
       std::unique_lock<std::mutex> lock(dap_sessions_mutex);
-      dap_sessions.erase(sock.get());
+      dap_sessions.erase(io.get());
       std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock));
     });
     client.detach();
@@ -561,8 +563,10 @@ int main(int argc, char *argv[]) {
     return EXIT_FAILURE;
   }
 
-  StreamDescriptor input = StreamDescriptor::from_file(fileno(stdin), false);
-  StreamDescriptor output = StreamDescriptor::from_file(stdout_fd, false);
+  lldb::IOObjectSP input = std::make_shared<NativeFile>(
+      fileno(stdin), File::eOpenOptionReadOnly, true);
+  lldb::IOObjectSP output = std::make_shared<NativeFile>(
+      stdout_fd, File::eOpenOptionWriteOnly, false);
 
   DAP dap = DAP("stdin/stdout", program_path, log.get(), std::move(input),
                 std::move(output), default_repl_mode, pre_init_commands);