[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)

[John Harrison via lldb-commits]

================
@@ -24,41 +30,95 @@ using namespace llvm;
 
 namespace lldb_dap {
 
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+std::error_code EC;
 
+FifoFile::FifoFile(StringRef path)
+    : m_path(path), m_file(fopen(path.data(), "r+")) {
+  if (m_file == nullptr) {
+    EC = std::error_code(errno, std::generic_category());
+    llvm::errs() << "Failed to open fifo file " << path << ": " << EC.message()
+                 << "\n";
+    std::terminate();
+  }
+  if (setvbuf(m_file, NULL, _IONBF, 0))
+    llvm::errs() << "Error setting unbuffered mode on C FILE\n";
+}
+FifoFile::FifoFile(StringRef path, FILE *f) : m_path(path), m_file(f) {}
+FifoFile::FifoFile(FifoFile &&other)
+    : m_path(other.m_path), m_file(other.m_file) {
+  other.m_file = nullptr;
+}
 FifoFile::~FifoFile() {
+  if (m_file)
+    fclose(m_file);
 #if !defined(_WIN32)
+  // Unreferenced named pipes are deleted automatically on Win32
   unlink(m_path.c_str());
 #endif
 }
 
-Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
-#if defined(_WIN32)
-  return createStringError(inconvertibleErrorCode(), "Unimplemented");
+// This probably belongs to llvm::sys::fs as another FSEntity type
+std::error_code createNamedPipe(const Twine &Prefix, StringRef Suffix,
+                                int &ResultFd,
+                                SmallVectorImpl<char> &ResultPath) {
+  const char *Middle = Suffix.empty() ? "-%%%%%%" : "-%%%%%%.";
+  auto EC = sys::fs::getPotentiallyUniqueFileName(
+#ifdef _WIN32
+      "\\\\.\\pipe\\LOCAL\\"
+#else
+      "/tmp/"
+#endif
+          + Prefix + Middle + Suffix,
+      ResultPath);
+  if (EC)
+    return EC;
+  ResultPath.push_back(0);
+  const char *path = ResultPath.data();
+#ifdef _WIN32
+  HANDLE h = ::CreateNamedPipeA(
+      path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 1024, 1024, 0, NULL);
+  if (h == INVALID_HANDLE_VALUE)
+    return std::error_code(::GetLastError(), std::system_category());
+  ResultFd = _open_osfhandle((intptr_t)h, _O_TEXT | _O_RDWR);
+  if (ResultFd == -1)
+    return std::error_code(::GetLastError(), std::system_category());
 #else
-  if (int err = mkfifo(path.data(), 0600))
-    return createStringError(std::error_code(err, std::generic_category()),
-                             "Couldn't create fifo file: %s", path.data());
-  return std::make_shared<FifoFile>(path);
+  if (mkfifo(path, 0600) == -1)
+    return std::error_code(errno, std::generic_category());
+  EC = openFileForWrite(ResultPath, ResultFd, sys::fs::CD_OpenExisting,
+                        sys::fs::OF_None, 0600);
+  if (EC)
+    return EC;
 #endif
+  ResultPath.pop_back();
+  return std::error_code();
 }
 
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
-    : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+FifoFileIO::FifoFileIO(FifoFile &&fifo_file, StringRef other_endpoint_name)
+    : m_fifo_file(std::move(fifo_file)),
+      m_other_endpoint_name(other_endpoint_name) {}
 
 Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
   // We use a pointer for this future, because otherwise its normal destructor
   // would wait for the getline to end, rendering the timeout useless.
   std::optional<std::string> line;
   std::future<void> *future =
       new std::future<void>(std::async(std::launch::async, [&]() {
-        std::ifstream reader(m_fifo_file, std::ifstream::in);
-        std::string buffer;
-        std::getline(reader, buffer);
-        if (!buffer.empty())
-          line = buffer;
+        rewind(m_fifo_file.m_file);
----------------
ashgti wrote:

I think the flow of the logic here is:

* `lldb-dap` (pid 1) opens the fifo
* pid 1 sends sends the `runInTerminal` command.
* pid 1 reads from the fifo (waiting for data)
* `lldb-dap --launch-target` (pid 2) starts
* pid 2 opens the fifo
* pid 2 spawns the new process
* pid 2 writes to the fifo (something like `{"pid":2}\n`) 
* pid 1 finishes the initial read from the fifo
* pid 1 lldb attaches to the process started by pid 2

This should mean that the parent side only reads from the file and the child side only writes to the file. Additionally, I think they're only each performing 1 write/read on each side so their respective file positions should still be at the beginning of the file after opening.

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