[lldb-vscode] Redirect stderr and stdout to DAPs console message

[wallace]

Summary

Doing this for 2 main reasons:

• Avoid stdout from corrupting DAP messages
• Make sure we can see in VSCode (or any other DAP client) the same output we see when debugging from the command line

[labath]

I don't actually use vscode, so I don't really know whether this is a good idea, but if you say it is then I can go with that.

However, I think the implementation needs a lot of work:

• it is very windows-hostile: dup, pipe, select, etc. don't exist or don't work on windows (btw, doing a blocking select before a blocking read is completely useless -- you might as well just do the read alone)
• the lack of synchronization between the (detached) forwarding threads makes it very prone to crashes if something happens to write to stderr during shutdown or early in initialization

The way I would imagine doing this is:

• redirect stdout/err as early as possible to avoid something writing to it, but don't forward anything yet
• when things are sufficiently initialized, start up the forwarding machinery
• at, an appropriate point during shutdown, stop forwarding

The stop forwarding is the trickiest part I think the most portable solution would be to just close the write end of the pipe, and join the forwarding thread (which will exit once read returns EOF).

We should also think about testing all of this. Do we have a reliable mechanism to produce stdout/err output from lldb? I guess it the worst case we could try debugging a module which produces one of these warnings.

Or, if this output is going to the same place as the SBDebuggers notion of stdout/err, then we could just stop calling SetOutputFileHandle, and have everything flow through this. That would reduce the overall complexity and would ensure this path gets better coverage, as it would be used in the common case instead of just for printing random errors.

[aadsm]

thanks for the feedback, I'll read it more carefully later. As for the tests it should be easy to set up, didn't want to spend time on that before we agreed on doing it this way.

[labath]

Btw, given that Richard has found some vscode code which already tries to send stderr to the console window https://github.com/microsoft/vscode/blob/master/src/vs/workbench/contrib/debug/node/debugAdapter.ts#L234, it would be good to first understand why doesn't that kick in. Maybe there's a vscode bug that could be fixed instead.

[aadsm]

Absolutely, I'll look into that first.

[rmaz]

In D80659#2062304, @labath wrote:

Btw, given that Richard has found some vscode code which already tries to send stderr to the console window https://github.com/microsoft/vscode/blob/master/src/vs/workbench/contrib/debug/node/debugAdapter.ts#L234, it would be good to first understand why doesn't that kick in. Maybe there's a vscode bug that could be fixed instead.

The issue is fairly clear, that listener is only added if the optional constructor arg outputService is defined, but it never is. I was planning on putting up a diff this week some time to always read from stderr regardless.

[aadsm]

This update still doesn't add windows support but I just want to get a feel that I'm going on the right direction.
It addresses the following:

• redirect stdout/err as early as possible to avoid something writing to it, and only starts or ends redirection it once it's safe.
• adds a mutex to OutputStream::write_full to make sure multiple threads don't try to write to the output fd at the same time.

We should also think about testing all of this. Do we have a reliable mechanism to produce stdout/err output from lldb?

this turned out to be harder than I thought it would be, my plan was to print to stdout in python from a thread but I never see that print anywhere (not sure why though). A sure way to test this (what I did manually) is to put a script print('foo') command in lldninit. Unfortunately I can't use a local lldbinit since that's a concept of the lldb binary and not of liblldb (unless we want to add that option to lldb-vscode as well), so not really sure about this as well.
I also tried to use a breakpoint command add -o 'script print("foo")' but same behaviour, I never saw that print. Still don't know how to tackle this, need to think more about it.

Or, if this output is going to the same place as the SBDebuggers notion of stdout/err,

There no guarantee of this, people can just use sys.stdout.write in python.

given that Richard has found some vscode code which already tries to send stderr to the console window

I would still prefer to wrap liblldb's stderr into a console message so people can see it in the client(e.g.: VSCode) console rather than outputting it through the lldb-vscode stderr which will then be up to the client to decide what to do with it.

[labath]

Sorry, I missed this patch. Modulo the inline comments and the missing windows support and tests, I think this patch is ok-ish. However, I am still not convinced of its usefulness. Why would we be doing something (particularly a thing which will be hard to do in a cross-platform manner, and will very likely border on, or downright cross into, undefined behavior territory), if we get that from vscode for free?

lldb/tools/lldb-vscode/lldb-vscode.cpp

519–520

This is very fishy. If the thread is detached in StartRedirecting, joinable will never be true.

521

Who's closing m_captured_fd[0] ?

2919–2922

Rely on the desctructor calling these? Or have the destructor assert they have been called?

[aadsm]

Why would we be doing something (particularly a thing which will be hard to do in a cross-platform manner, and will very likely border on, or downright cross into, undefined behavior territory), if we get that from vscode for free?

I'm not sure what you mean with get this for free on vscode... Right now lldb-vscode will just crash if there's any command in the lldbinit that outputs to stdout because it pollutes the protocol, I don't think think there's anything vscode can do about this. But even if it did vscode is not the only client of lldb-vscode, there are multiple others: https://microsoft.github.io/debug-adapter-protocol/implementors/tools/.

I haven't answered this yet because I was trying to figure out why this only happens when parsing the lldbinit. I was looking into the CommandInterpreter{Python} code but to be honest it's not easy to figure out because there are multiple levels of indirection when it comes to processing the IO of the python interpreter, but it seems that the stdout while executing the lldbinit script is being flushed right away to the ImmediateOutputStream but haven't figure out when this is set in the case of CommandInterpreter::IOHandlerInputComplete.

[labath]

Sorry, I confused this with the patch that was wrapping stderr into DAP messages.

Having python output break the protocol is definitely not good. Fixing it would be nice, but the solution should work on windows as well.

One possibility I'd consider is making this fix at the python level (as this is a python problem). We already are trying to do some stdin/out/err redirection there, but it seems it's not good enough. It seems it should be easy enough (though hacky, but all of this is going to be hacky anyway) to replace sys.stdin/out/err with a custom wrapper object.

[wallace]

not needed anymore because of D99974

lldb/tools/lldb-vscode/IOStream.h

 //===-- IOStream.h ----------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
struct InputStream {
 
   bool read_expected(std::ofstream *log, llvm::StringRef expected);
 };
 
 struct OutputStream {
   StreamDescriptor descriptor;
 
   bool write_full(llvm::StringRef str);
+
+  private:
+  std::mutex m_mutex;
 };
 } // namespace lldb_vscode
 
 #endif

lldb/tools/lldb-vscode/IOStream.cpp

 //===-- IOStream.cpp --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
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) {
+  std::lock_guard<std::mutex> lock(m_mutex);
   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) {

lldb/tools/lldb-vscode/lldb-vscode.cpp

 //===-- lldb-vscode.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
if (sourceMap) {
     strm << "\".\" \"" << sourcePath << "\"";
   }
   strm.flush();
   if (!sourceMapCommand.empty()) {
     g_vsc.RunLLDBCommands("Setting source map:", {sourceMapCommand});
   }
 }
 
+class FileDescriptorToConsoleOutputRedirector {
+  private:
+    int m_captured_fd[2];
+    std::thread m_thread;
+
+  public:
+    FileDescriptorToConsoleOutputRedirector(int fd) {
+      pipe(m_captured_fd);
+      dup2(m_captured_fd[1], fd);
+    };
+    ~FileDescriptorToConsoleOutputRedirector() {
+      StopRedirecting();
+    }
+    // Only call this once.
+    void StartRedirecting() {
+      int read_fd = m_captured_fd[0];
+
+      m_thread = std::thread([read_fd] {
+        char buffer[4096];
+        while (true) {
+          ssize_t bytes_count = read(read_fd, &buffer, sizeof(buffer));
+          if (bytes_count == 0)
+            return;
+          g_vsc.SendOutput(OutputType::Console, buffer);
+        }
+      });
+      m_thread.detach();
+    }
+
+    void StopRedirecting() {
+      if (!m_thread.joinable())
+        return;
+      close(m_captured_fd[1]);
+      m_thread.join();
+    }
+};
+
 // "AttachRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
 //     "description": "Attach request; value of command field is 'attach'.",
 //     "properties": {
 //       "command": {
 //         "type": "string",
 //         "enum": [ "attach" ]
EXAMPLES:
   debugger to attach to the process.
 
     lldb-vscode -g
   )___";
   llvm::outs() << examples;
 }
 
 int main(int argc, char *argv[]) {
-
+  int stdout_fd = dup(fileno(stdout));
+  FileDescriptorToConsoleOutputRedirector stdout_redirector(fileno(stdout));
+  FileDescriptorToConsoleOutputRedirector stderr_redirector(fileno(stderr));
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
 
   int portno = -1;
 
   LLDBVSCodeOptTable T;
   unsigned MAI, MAC;
   llvm::ArrayRef<const char *> ArgsArr = llvm::makeArrayRef(argv + 1, argc);
if (portno != -1) {
     if (socket_fd >= 0) {
       g_vsc.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
       g_vsc.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
     } else {
       exit(1);
     }
   } else {
     g_vsc.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-    g_vsc.output.descriptor =
-        StreamDescriptor::from_file(fileno(stdout), false);
+    g_vsc.output.descriptor = StreamDescriptor::from_file(stdout_fd, false);
   }
+  // Can start redirecting now that the output description has been properly set.
+  stdout_redirector.StartRedirecting();
+  stderr_redirector.StartRedirecting();
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
     std::string json = g_vsc.ReadJSON();
     if (json.empty())
       break;
 
     llvm::StringRef json_sref(json);
if (packet_type == "request") {
       }
     }
     ++packet_idx;
   }
 
   // We must terminate the debugger in a thread before the C++ destructor
   // chain messes everything up.
   lldb::SBDebugger::Terminate();
+  // Can now safely stop redirecting since lldb will no longer emit stdout
+  // or stderr messages.
+  stdout_redirector.StopRedirecting();
+  stderr_redirector.StopRedirecting();
   return 0;
 }