================
@@ -24,41 +31,105 @@ using namespace llvm;
 
 namespace lldb_dap {
 
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+FifoFile::FifoFile(std::string path, FILE *f) : m_path(path), m_file(f) {}
+
+Expected<FifoFile> FifoFile::create(StringRef path) {
+  auto file = fopen(path.data(), "r+");
+  if (file == nullptr)
+    return createStringError(inconvertibleErrorCode(),
+                             "Failed to open fifo file " + path);
+  if (setvbuf(file, NULL, _IONBF, 0))
+    return createStringError(inconvertibleErrorCode(),
+                             "Failed to setvbuf on fifo file " + path);
+  return FifoFile(path, file);
+}
+
+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_path.clear();
+  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
+Error createUniqueNamedPipe(const Twine &prefix, StringRef suffix,
+                            int &result_fd,
+                            SmallVectorImpl<char> &result_path) {
+  std::error_code EC;
+#ifdef _WIN32
+  const char *middle = suffix.empty() ? "-%%%%%%" : "-%%%%%%.";
+  EC = sys::fs::getPotentiallyUniqueFileName(
+      "\\\\.\\pipe\\LOCAL\\" + prefix + middle + suffix, result_path);
+#else
+  EC = sys::fs::getPotentiallyUniqueTempFileName(prefix, suffix, result_path);
+#endif
+  if (EC)
+    return errorCodeToError(EC);
+  result_path.push_back(0);
+  const char *path = result_path.data();
+#ifdef _WIN32
+  HANDLE h = ::CreateNamedPipeA(
+      path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
----------------
SuibianP wrote:

> when its opened for reading it blocks until the file is also opened for 
> writing and vice versa

This is implemented with 
[`ConnectNamedPipe`](https://learn.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-connectnamedpipe)
 in `FifoFileIO::WaitForPeer` after creation. `WaitNamedPipe` was not needed as 
the server side would already be around by the time the client tries to connect.

> deadlock the process by having it wait on itself accidentally

Intra-process deadlock (waiting on its own pending output) should not be 
possible, since on Windows a process cannot even read back what it wrote into a 
named pipe from the same handle. The data flow is bidirectional, but can only 
be consumed by the other end.

Inter-process deadlock, with the exception of buffering, is also rather 
unlikely. Current protocol over the pipe is strictly synchronous and 
sequential, or "half-duplex" in some sense -- there are no overlapping 
transmissions; in other words, the process will not transmit before expected 
response is fully received, and will not block on read before it sends out the 
message.

Buffering can still cause deadlocks. Without the `rewind`, the process proceeds 
to wait for the response after write, but the payload has somehow not fully 
arrived at the other end, and the peer as a result does not respond yet. I 
speculated it to be due to libc buffering, but did not manage to penetrate 
through MS CRT code to verify.

> Should we instead switch to a communication channel that supports reading and 
> writing for all platforms?

Using sockets feels a bit like overkill, but would definitely make code more 
uniform across platforms I suppose. I can give it a try if you deem it more 
maintainable.

> we have some existing helpers for supporting sockets already.

As an aside, I remember I did also attempt to retrofit the use case into 
`lldb_private::Pipe`, but gave up because duplex was too hard to shoehorn into 
the existing abstraction, and would have very limited utility even if 
implemented.

> Is that something you might have a chance to look into?

I am not too familiar with that so unable to give guarantees, but let me give 
it a try.
Also unsure if the socket rewrite is going to make it for LLVM 21 feature 
freeze in July.

https://github.com/llvm/llvm-project/pull/121269
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to