Author: John Harrison Date: 2025-02-26T11:29:13-08:00 New Revision: 1e246704e23e3dcae16adbf68cc10b668a8db680
URL: https://github.com/llvm/llvm-project/commit/1e246704e23e3dcae16adbf68cc10b668a8db680 DIFF: https://github.com/llvm/llvm-project/commit/1e246704e23e3dcae16adbf68cc10b668a8db680.diff LOG: [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (#128750) 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 underlying native handle and was not aware of the `Close()` call to the Socket itself. This is both nice to have for simplifying the existing code and this unblocks an upcoming refactor to support the cancel request. --------- Co-authored-by: Jonas Devlieghere <jo...@devlieghere.com> Added: Modified: lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/IOStream.cpp lldb/tools/lldb-dap/IOStream.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: ################################################################################ diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 37bc1f68e4b08..cd53e2aca3fb6 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -18,6 +18,7 @@ #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Utility/IOObject.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" @@ -61,7 +62,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)), diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 7ceb1d114a57d..a7c7e5d9bbc19 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -30,6 +30,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" @@ -210,7 +211,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; diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp index 7d0f363440f53..c6f1bfaf3b799 100644 --- a/lldb/tools/lldb-dap/IOStream.cpp +++ b/lldb/tools/lldb-dap/IOStream.cpp @@ -7,125 +7,34 @@ //===----------------------------------------------------------------------===// #include "IOStream.h" +#include "lldb/Utility/IOObject.h" +#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; } diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h index c91b2f717893c..e9fb8e11c92da 100644 --- a/lldb/tools/lldb-dap/IOStream.h +++ b/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 diff erent 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); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 1c6bd7e903409..d8b92831d5540 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/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; @@ -308,14 +313,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, @@ -329,18 +334,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(), @@ -352,7 +354,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()) { @@ -368,7 +370,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(); @@ -560,8 +562,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); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
