[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

[Pavel Labath via lldb-commits]

================
@@ -5058,72 +5018,187 @@ int main(int argc, char *argv[]) {
   auto terminate_debugger =
       llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
 
-  StreamDescriptor input;
-  StreamDescriptor output;
-  std::FILE *redirectOut = nullptr;
-  std::FILE *redirectErr = nullptr;
-  if (portno != -1) {
-    printf("Listening on port %i...\n", portno);
-    SOCKET socket_fd = AcceptConnection(log.get(), portno);
-    if (socket_fd < 0)
+  std::vector<std::string> pre_init_commands;
+  for (const std::string &arg :
+       input_args.getAllArgValues(OPT_pre_init_command)) {
+    pre_init_commands.push_back(arg);
+  }
+
+  if (!connection.empty()) {
+    auto maybeProtoclAndName = validateConnection(connection);
+    if (auto Err = maybeProtoclAndName.takeError()) {
+      llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+                                  "Invalid connection: ");
       return EXIT_FAILURE;
+    }
 
-    input = StreamDescriptor::from_socket(socket_fd, true);
-    output = StreamDescriptor::from_socket(socket_fd, false);
-  } else {
-#if defined(_WIN32)
-    // Windows opens stdout and stdin in text mode which converts \n to 13,10
-    // while the value is just 10 on Darwin/Linux. Setting the file mode to
-    // binary fixes this.
-    int result = _setmode(fileno(stdout), _O_BINARY);
-    assert(result);
-    result = _setmode(fileno(stdin), _O_BINARY);
-    UNUSED_IF_ASSERT_DISABLED(result);
-    assert(result);
-#endif
+    Socket::SocketProtocol protocol;
+    std::string name;
+    std::tie(protocol, name) = *maybeProtoclAndName;
 
-    int stdout_fd = DuplicateFileDescriptor(fileno(stdout));
-    if (stdout_fd == -1) {
-      llvm::logAllUnhandledErrors(
-          llvm::errorCodeToError(llvm::errnoAsErrorCode()), llvm::errs(),
-          "Failed to configure stdout redirect: ");
+    Status error;
+    static std::unique_ptr<Socket> listener = Socket::Create(protocol, error);
+    if (error.Fail()) {
+      llvm::logAllUnhandledErrors(error.takeError(), llvm::errs(),
+                                  "Failed to create socket listener: ");
       return EXIT_FAILURE;
     }
 
-    redirectOut = stdout;
-    redirectErr = stderr;
+    error = listener->Listen(name, /*backlog=*/5);
+    if (error.Fail()) {
+      llvm::logAllUnhandledErrors(error.takeError(), llvm::errs(),
+                                  "Failed to listen for connections: ");
+      return EXIT_FAILURE;
+    }
+
+    std::string address =
+        llvm::join(listener->GetListeningConnectionURI(), ", ");
+    if (log)
+      *log << "started with connection listeners " << address << "\n";
+
+    llvm::outs() << "Listening for: " << address << "\n";
+    // Ensure listening address are flushed for calles to retrieve the resolve
+    // address.
+    llvm::outs().flush();
+
+    static lldb_private::MainLoop g_loop;
+    llvm::sys::SetInterruptFunction([]() {
+      g_loop.AddPendingCallback(
+          [](lldb_private::MainLoopBase &loop) { loop.RequestTermination(); });
+    });
+    std::mutex active_dap_sessions_mutext;
+    std::set<DAP *> active_dap_sessions;
+    unsigned int clientCount = 0;
+    auto handle = listener->Accept(g_loop, [=, &active_dap_sessions_mutext,
+                                            &active_dap_sessions, &clientCount,
+                                            log = log.get()](
+                                               std::unique_ptr<Socket> sock) {
+      std::string name = llvm::formatv("client_{0}", clientCount++).str();
+      if (log) {
+        auto now = std::chrono::duration<double>(
+            std::chrono::system_clock::now().time_since_epoch());
+        *log << llvm::formatv("{0:f9}", now.count()).str()
+             << " client connected: " << name << "\n";
+      }
+
+      // Move the client into a background thread to unblock accepting the next
+      // client.
+      std::thread client([=, &active_dap_sessions_mutext, &active_dap_sessions,
+                          sock = std::move(sock)]() {
+        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);
+
+        if (auto Err = dap.ConfigureIO()) {
+          llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+                                      "Failed to configure stdout redirect: ");
+          return;
+        }
+
+        RegisterRequestCallbacks(dap);
+
+        {
+          std::scoped_lock lock(active_dap_sessions_mutext);
+          active_dap_sessions.insert(&dap);
+        }
+
+        if (auto Err = dap.Loop()) {
+          llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+                                      "DAP session error: ");
+        }
+
+        {
+          std::scoped_lock lock(active_dap_sessions_mutext);
+          active_dap_sessions.erase(&dap);
+        }
+
+        if (log) {
+          auto now = std::chrono::duration<double>(
+              std::chrono::system_clock::now().time_since_epoch());
+          *log << llvm::formatv("{0:f9}", now.count()).str()
+               << " client closed: " << name << "\n";
+        }
+      });
+      client.detach();
+    });
+    if (auto Err = handle.takeError()) {
+      llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+                                  "Registering accept handler failed: ");
+      return EXIT_FAILURE;
+    }
+
+    error = g_loop.Run();
+    if (error.Fail()) {
+      llvm::logAllUnhandledErrors(error.takeError(), llvm::errs(),
+                                  "MainLoop failed: ");
+      return EXIT_FAILURE;
+    }
+
+    if (log)
+      *log << "lldb-dap server shutdown requested, disconnecting remaining "
+              "clients...\n";
+
+    bool client_failed = false;
+    std::scoped_lock lock(active_dap_sessions_mutext);
+    for (auto *dap : active_dap_sessions) {
+      auto error = dap->Disconnect();
+      if (error.Fail()) {
+        client_failed = true;
+        llvm::errs() << "DAP client " << dap->name
+                     << " disconnected failed: " << error.GetCString() << "\n";
+      }
+    }
+
+    return client_failed ? EXIT_FAILURE : EXIT_SUCCESS;
----------------
labath wrote:

There's a race here between this thread exiting (and destroying the `active_dap_sessions` list) and the now-disconnected threads trying to remove themselves from it.

I think that could be fixed by waiting until this list becomes empty (ideally using the aforementioned [std::notify_all_at_thread_exit](https://en.cppreference.com/w/cpp/thread/notify_all_at_thread_exit) to delay cond_var notification until the thread is well into shutting itself down.

https://github.com/llvm/llvm-project/pull/116392